Handling of enum with sub-variants

[Mazrog]

Hi,

I am not sure if this is a bug/limitation of the current implementation but I encounter some issues with the Actionlike macro being used with "complex" enums: it seems to construct them by default, thus loosing the information.

#[derive(PartialEq, Default, Eq, Clone, Debug, Hash, Copy)]
enum ActionBars { #[default] AB0, AB1 }

#[derive(Actionlike, PartialEq, Eq, Clone, Debug, Copy)]
enum GameAction { Jump, ActionBar(ActionBars), Test(i32) }

First I was "forced" to add the Default trait to my ActionBars enum, but when running, it seems that everytime an ActionBar or the Test variant of the enum was triggered, it was reset to default when passed to the ActionState.

Code that spawns the bundle

commands.spawn_bundle(PlayerInputBundle{
        player: Player,
        input_map: InputMap::new([
            (KeyCode::Key1, GameAction::ActionBar(ActionBars::AB0)),
            (KeyCode::Key2, GameAction::ActionBar(ActionBars::AB1)),
            (KeyCode::E, GameAction::Test(42)),
            (KeyCode::Space, GameAction::Jump)
        ]).build(),
        ability_action_state: ActionState::default(),
        actionbar_map: actionbars_mapping
    });

Actually reading the action:

fn print_abilities(query: Query<(&ActionState<GameAction>, &ActionBarsMap)>) {
    for (ability_state, mapping) in query.iter() {
        for ability in ability_state.get_just_pressed() {
            if let GameAction::ActionBar(ab) = ability {
                dbg!(ab);
                let spell_id = mapping.get(&ab);
                dbg!(spell_id);
            } else {
                dbg!(ability);
            }
        }
    }
}

And in the case of subtypes, it always prints either ActionBars(AB0) or Test(0), even when it should have printed ActionBars(AB1) or Test(42).

Is this a limitation or did I miss something to make this work?

If it is a limitation, I would like to have this as an enhancement if possible.

Thank you

[alice-i-cecile]

Yep, this is a limitation. The changes that we need to make this work involve modifying the macro code to recursively walk the enums.

I'm very much in favor of this change: if you or any other reader wants to tackle this I'd be happy to review and merge a PR. Otherwise, I'll likely get to this myself Eventually.

For now, you could work around it by manually implementing the Actionlike trait.

[Mazrog]

I began trying to write something that would work for my case as a first step (quite new to Rust tbh), but I stumbled upon something that may cause problem: the whole point of the Actionlike is to match an enum variant with an unique usize, right?

How would it be translated for this enum?

enum Foo {
  Foo1,
  Other(usize)
}

We would need #usize + 1 values to be identified with a usize value (same would occur with an enum with two variant struct-like with an underlying i32 for example).

One thing I can think of would be to change the identifier used to match enum-variants but that would be a lot of changes (and I do not even know if it would work in all cases).

[alice-i-cecile]

Yep, we start getting into problems like those discussed in #68 and #60. For now, I would start with only accounting for cases where the internal data is itself an enum. Overflows can be ignored: if you have usize different discrete actions you're probably not using this crate as it was intended!

[focustense]

Random question that I'm sure is massively oversimplifying the problem, but doesn't enum_map tackle fundamentally the same issue? Particularly the Enum trait/derive macro creating a mapping between the enum and usize.

If nothing else, shows it's possible to enumerate all available enum values, given some reasonable constraints that would apply equally well to input mappings (e.g. not having more than usize actions).

I just ran into the same issue trying to do the Move(Direction) thing, I'd say it's a pretty common use case because most every game is going to already have a Direction enum for any number of other reasons (animation sets, screen transitions, movement in general). Not really that big a deal to de-parameterize the enum; but definitely more ergonomic if we can reuse our existing enums.

Is there an example of manually implementing Actionlike to work around the issue?

[alice-i-cecile]

Yep, that should tackle the issue. I'm going to play around with refactoring UserInput soon; I think we should be able to fix this limitation pretty easily.