Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flixel Actions #1741

Closed
wants to merge 18 commits into from
Closed

Flixel Actions #1741

wants to merge 18 commits into from

Conversation

larsiusprime
Copy link
Member

This is a big commit, so for a broad-strokes overview, see this:
https://github.com/larsiusprime/haxeflixel/blob/actions_/flixel/input/actions/FlxActionManager.hx#L24

(That giant info dump probably needs to go somewhere other than the code itself, I just put it there for now, probably would eventually just go in the flixel docs).

This has been tested and works on my machine, but I don't currently have a demo for it. The syntax to use it is this:

1: Simple Digital Action

var JUMP = new FlxActionDigital("jump", onJump);
JUMP.addInput(new FlxActionInputDigitalMouse(FlxMouseButtonID.LEFT, JUST_PRESSED);
JUMP.addInput(new FlxActionInputDigitalKeyboard(FlxKey.SPACE, JUST_PRESSED);
JUMP.addInput(new FlxActionInputDigitalGamepad(FlxGamepadInputID.A, JUST_PRESSED);

This will trigger "JUMP" on left mouse click, spacebar press, or pressing the bottom face button on the (first active) gamepad.

In your update loop:

JUMP.update();                 //if you're relying on callbacks
if(JUMP.check()) { doJump(); } //to do the check directly yourself

2: Simple Analog Action

var MOVE = new FlxActionAnalog("move",onMove);
MOVE.addInput(new FlxActionInputAnalogMouse(FlxAnalogState.MOVED, FlxAnalogAxis.EITHER);
MOVE.addInput(new FlxActionInputAnalogGamepad(FlxGamepadInputID.LEFT_ANALOG_STICK, FlxAnalogState.MOVED, FlxAnalogAxis.EITHER);

In your update loop:

MOVE.update();  //mandatory for analog if you rely on callbacks or want JUST_MOVED / JUST_STOPPED to behave correctly
if(MOVE.check()) { doMove(MOVE.x, MOVE.y);} //to do the check directly yourself

3: Using the Action Manager

var actionManager = new FlxActionManager();
FlxG.inputs.add(actionManager);
var menuActions = new FlxActionSet("menuActions", someDigitalActions, someAnalogActions);
var battleActions = new FlxActionSet("battleActions", otherDigitalActions, otherAnalogActions);
var menuIndex = actionManager.addSet(menuActions);
var battleIndex = actionManager.addSet(battleActions);

if(inMenu)
{
    //activate menu action set for all devices
    actionManager.activate(menuIndex, FlxInputDevice.ALL, FlxInputDeviceID.ALL);
}
else if(inBattle)
{
    //activate battle action set for all devices
    actionManager.activate(battleIndex, FlxInputDevice.ALL, FlxInputDeviceID.ALL);
}

4: Using the Action Manager with Steam Controllers

First, install the steamwrap library, and follow all the steps in the SteamWorks documentation to get the basics set up.

Then:

var actionManager = new FlxActionManager();
FlxG.inputs.add(actionManager);

var vdfText:String = Assets.getText("assets/xml/game_actions_XYZ.vdf");
actionManager.initSteam(steamwrap.data.ControllerConfig.fromVDF(vdfText)), 
    function(ad:FlxActionDigital):Void
    {
        trace("digital " + ad.name);
    },
    function(aa:FlxActionAnalog):Void
    {
        trace("analog " + aa.name);
    }
);

var menuIndex = actionManager.getSetIndex("MenuControls");  //"MenuControls" is an action set defined in the .vdf file

//get a list of connected steam controllers from the Steam API:
var controllers = Steam.controllers.getConnectedControllers();

//we must activate an action set for a steam controller to use:
actionManager.activateSet(menuIndex, FlxInputDevice.SteamController, controllers[0]);

//Let's add some non-steam inputs to the action sets we generated from the steam vdf file.
//This block is TOTALLY optional, I'm just doing it to show off how to add regular
//inputs to action set structure generated from Steam VDF data:

var menuSet = actionManager.getSet(menuIndex);
for (action in menuSet.digitalActions)
{
    switch(action.name)
    {
        case "menu_down":  action.addInput(new FlxActionInputDigitalKeyboard(FlxKey.DOWN, JUST_PRESSED));
                           action.addInput(new FlxActionInputDigitalGamepad(FlxGamepadInputID.DPAD_DOWN, JUST_PRESSED));

        case "menu_left":  action.addInput(new FlxActionInputDigitalKeyboard(FlxKey.LEFT, JUST_PRESSED));
                           action.addInput(new FlxActionInputDigitalGamepad(FlxGamepadInputID.DPAD_LEFT, JUST_PRESSED));

        case "menu_right": action.addInput(new FlxActionInputDigitalKeyboard(FlxKey.RIGHT, JUST_PRESSED));
                           action.addInput(new FlxActionInputDigitalGamepad(FlxGamepadInputID.DPAD_RIGHT, JUST_PRESSED));

        case "menu_up":    action.addInput(new FlxActionInputDigitalKeyboard(FlxKey.UP,    JUST_PRESSED));
                           action.addInput(new FlxActionInputDigitalGamepad(FlxGamepadInputID.DPAD_UP, JUST_PRESSED));

        case "menu_select": action.addInput(new FlxActionInputDigitalKeyboard(FlxKey.ENTER, JUST_PRESSED));
                           action.addInput(new FlxActionInputDigitalGamepad(FlxGamepadInputID.A, JUST_PRESSED));

        case "menu_cancel": action.addInput(new FlxActionInputDigitalKeyboard(FlxKey.ESCAPE, JUST_PRESSED));
                           action.addInput(new FlxActionInputDigitalGamepad(FlxGamepadInputID.B, JUST_PRESSED));
    }
}

Squashed commit:

[fb923a4] Getting close to final version of FlxAction stuff

[58fcfba] more cleanup

[750f177] cleanup

[f15193f] Flixel Actions -- first commit (+2 squashed commit)

Squashed commit:

[9025228] cleanup

[a516cfd] Flixel Actions -- first commit
@Gama11 Gama11 added this to the 4.1.0 milestone Feb 23, 2016
@larsiusprime
Copy link
Member Author

I'll fix travis when i can, 99% sure its missing #if steamwrap blocks

@larsiusprime
Copy link
Member Author

Okay, Travis is finally satisfied, if @Gama11 or anybody else has any comments on the PR itself.

@MSGhero
Copy link
Member

MSGhero commented Feb 24, 2016

Does this preclude the current input system(s), or is this purely an addition to make life easier/enable steam controller?

Also, needs a crossinput demo.

@IBwWG
Copy link
Contributor

IBwWG commented Feb 24, 2016

This looks nice even if you're not using steam controllers :)

@larsiusprime
Copy link
Member Author

@MSGhero: This builds directly on top of existing input stuff, so all that still works. You can use this instead of, or in combination with, existing input methods. It's the only sensible way to enable the steam controller, but it's still useful for regular input styles.

@Tiago-Ling
Copy link
Contributor

Awesome work - this will be a time saver and a allow a better organization
for all projects!

Are the update function calls mandatory (e.g. action.update() ) ? Or are
they needed only if you're using single actions (i.e. when using the
manager you don't need to update actions directly)?

Tiago Ling Alexandre
Tel: +55 41 8819-3191

2016-02-24 11:07 GMT-03:00 Lars Doucet [email protected]:

@MSGhero https://github.com/msghero: This builds directly on top of
existing input stuff, so all that still works. You can use this instead of,
or in combination with, existing input methods.


Reply to this email directly or view it on GitHub
#1741 (comment).

@larsiusprime
Copy link
Member Author

@Tiago-Ling: it will need a bit of testing to verify, but I'm 99% sure:

update() needs to be called on an action (or the action set it belongs to if you're using an action set), when:

  • you're relying on callbacks
  • you are using analog actions and need accurate JUST_MOVED / JUST_STOPPED logic
  • you are using steam controllers

Otherwise you can just use a naked FlxAction and call check() on it whenever you want to check. Digital actions have their JUST_PRESSED / JUST_RELEASED updated properly by flixel itself, but analog & steam controllers need constant update() logic to do that properly.

If I KNEW the user was always calling check() every frame, then I could make it behave like update and check, which would make those guarantees implicitly -- but it's hard to know if that's how they're using it.

But yes, as designed you can use actions by themselves, actions as parts of sets, or the manager. And if you use the manager, and add it to FlxG.inputs, you don't have to call update on anything, it's automatic.

@larsiusprime
Copy link
Member Author

Update -- I just looked at things, and it seems that calling check() also implicitly calls update(), and also fires callbacks if they are present!

This means that if the user is using a "naked" FlxAction (not used as part of a FlxActionSet or a FlxActionmanager) and calls check() every flixel update, there's literally no reason to also call FlxAction.update() -- in fact doing so could cause multiple erroneous callbacks, given the current implementation.

So a "safe" pattern could very well be to just call check() every frame. There's a few problems though:

  1. The user might NOT call check() every frame, in which case your JUST_* states won't be accurate and callbacks won't be called even when the physical input conditions have been met, if a check() wasn't run on that frame
  2. The user might call check() multiple times per frame, which would be wasteful (perhaps not important, I haven't profiled), but would also trigger multiple callbacks in one frame, which would be a problem.

Issue 1. might not be a real problem -- it's the "user's fault". But 2 seems like bad implementation. Is there a way in flixel to get a timestamp for the current frame or some other way to check that we're still at this point in time, so I can put some internal logic to make the internal updating logic short-circuit if it tries to run more than once on the same update frame?

@larsiusprime
Copy link
Member Author

Okay, now I'm fairly confident two usage patterns should suffice:

  1. call check() every frame

    On actions, sets, or even the manager. Will return true/false and fire callbacks if they exist. Will properly update JUST_* state information so long as its called every frame.

  2. use the manager, add to FlxG.inputs

    Then updating & callbacks happen automatically

@larsiusprime
Copy link
Member Author

Question: I could make FlxActions (and FlxActionSets) extend FlxBasic, which would let you add() them to the current state, automating update()'ing and ensuring they get destroy()'ed on state switch. Good idea or no?

@MSGhero
Copy link
Member

MSGhero commented Feb 27, 2016

I mean everything else extends FlxBasic. Destroying key inputs is a bit awkward, but I guess it kinda makes sense given that your state should have differently named actions, even if they use the same keys.

@MSGhero
Copy link
Member

MSGhero commented Feb 27, 2016

I would also like the demo to use the relative mouse stuff just to see how it works.

@Gama11
Copy link
Member

Gama11 commented Feb 27, 2016

I brought that FlxBasic idea up in the context of timers / tweens (#1087). It was pretty much decided against, and we should probably be consistent about that..

@larsiusprime
Copy link
Member Author

@Gama11 : I get this is a huge PR, and it'll take the time it takes to review, so no rush, but is there anything I can do in the meantime? Also maybe we should bring in some other people to review to lighten the load?

@Gama11
Copy link
Member

Gama11 commented Feb 28, 2016

There are some code style issues I've noticed but not commented on since I wanted to have a look at the high-level structure of this whole thing first. I haven't gotten to that though.

A few general things:

  • I don't like the usage of @:noCompletion - IDEs like FlashDevelop don't care about that, and technically, those classes are still public API that have to be considered for breaking changes etc. Why not make the classes private instead?
  • Variables should be declared at the top of the class, not the bottom.
  • There shouldn't be any empty lines between the doc comment of a field and the field itself.
/**
 *
 */

var example:Int;

vs

/**
 *
 */
var example:Int;

@larsiusprime
Copy link
Member Author

@Gama11: style fix in.

@larsiusprime
Copy link
Member Author

Travis says neko is failing but I crawled all over the log and can't seem to find the failure?

@Gama11
Copy link
Member

Gama11 commented Mar 5, 2016

This happens at the end of the munit tests:

pure virtual method called
terminate called without an active exception
Aborted (core dumped)

@larsiusprime
Copy link
Member Author

What does that mean?

@Gama11
Copy link
Member

Gama11 commented Mar 5, 2016

I have no idea, but I restarted that particular job and now it's green.

@larsiusprime
Copy link
Member Author

Weird :)

@MSGhero
Copy link
Member

MSGhero commented Apr 2, 2016

@larsiusprime
Copy link
Member Author

Heh, well I have to fix all the codeclimate checks now, thanks pointing that out @MSGhero :)

@larsiusprime
Copy link
Member Author

Squashed, see here:
#1805

@larsiusprime larsiusprime deleted the actions_ branch April 8, 2016 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants