Systems are the answer!

[shanecelis]

ME, calling up Elm on the phone a la Back to the Future: Hey Elm, you know that new gamepad handling you've been looking for? Well, look at this!

Heh heh.

---

Before

commands.spawn(
    KeySequence::new(MyEvent, keyseq! { alt-X M })
);

After

commands.add(
    KeySequence::new(action::send_event(MyEvent) /* or any other system you like */, 
                     keyseq! { alt-X M })
);

More earnestly, I think this patch has some good stuff in it. I set out to deal with the gamepad issue, and what I came up with was this: Why don't we just fire systems when we match a key sequences? It's still remarkably easy to fire events as before we just use our helper action::send_event(MyEvent), but now you can just invoke whatever you like.

But the real cherry on top is that systems can have input, and button sequences expect a system that accepts a Gamepad as its input, which means you can send an event, populate it with your gamepad info without any GamepadEvent shenanigans; in fact GamepadEvent trait has been thrown out.

Doing things this way means that we're not specializing on the event type E anymore. That allowed me to setup us up with a InputSequencePlugin. While I was adding plugin, I decided to follow in bevy_defer's foot steps and allow the user to declare what Schedule and what SystemSet they'd like to use. I've actually managed to avoid writing a ton of code with some judicious uses of run_if()s with bevy-input-sequence.

Let me know what you think. Be well, Elm!

[not-elm]

ME, on the phone responding to shanecelis: Thankyou, I was just looking for it!

---

I haven't confirmed everything yet, but I think the concept of this PR is very good.

However, I have the following two concerns

• The systems passed to KeySequence is executed only on the main thread.
• commands::run_system_with_input could be a problem in games where response time is important, since the system is run lazily.

I feel that the key to this PR is whether this is an issue that can be ignored.

[shanecelis]

Hey Elm,

Very good point. I hadn't considered that. It'd be nice if there was some measurements we could make. At worst the deferred commands are executed before the next frame.

I suppose there are a couple alternatives:

0. Let the user run bevy-input-sequence in PreUpdate if they want to ensure events and systems are run before Update. (That's achievable as-is.)
1. Use apply_deferred when adding our systems apply_deferred.after(key_sequence_matcher).
2. Use an exclusive system for key_sequence_matcher.

I prefer solution 0. But solution 1 might be useful: I'd suggest we could take approach 1 and make it optional. Add something to the plugin like plugin.reduce_latency(true) or something.

The downside of solutions 1 and 2 are they in some sense reduce parallelized performance of the app overall to provide delivery guarantees.

Let me know what's best.

-Shane

[not-elm]

oh! I missed that key_sequence_matcher is now able work on certain schedules.
I too think solution 0 is the best solution.

I think solution 1 is necessary when the sending and receiving systems must operate on the same schedule.

For Instance, in the code below, action::send_event is the sending side and event_listener is the receiving side.

Example

use bevy::prelude::*;
use bevy_input_sequence::prelude::*;

#[derive(Clone, Debug)]
enum Direction {
    Clockwise,
    CounterClockwise,
}

#[derive(Event, Clone, Debug)]
struct MyEvent(Direction);

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_plugins(InputSequencePlugin::default()
            .run_in(Update) // run on `Update`
        )
        .add_event::<MyEvent>()
        .add_systems(Startup, setup)
        .add_systems(Update, event_listener) // run on `Update`
        .run();
}

#[rustfmt::skip]
fn setup(mut commands: Commands) {
    // Specify key codes directly.
    commands.add(
        KeySequence::new(
            action::send_event(MyEvent(Direction::Clockwise)),
            [KeyCode::KeyW,
             KeyCode::KeyD,
             KeyCode::KeyS,
             KeyCode::KeyA],
        )
        .time_limit(Duration::from_secs(1)),
    );

    // Use keyseq! macro.
    commands.add(
        KeySequence::new(
            action::send_event(MyEvent(Direction::CounterClockwise)),
            keyseq!(W A S D),
        )
        .time_limit(Duration::from_secs(1)),
    );

    println!("Press W D S A to emit clockwise event.");
    println!("Press W A S D to emit counter clockwise event.");
}

fn event_listener(mut er: EventReader<MyEvent>) {
    for e in er.read() {
        println!("{e:?} emitted.");
    }
}

However, I don't think solution 1 is necessary because I can't think of a situation where it is absolutely necessary to run on the same schedule.

[not-elm]

One more point, since KeySequence is no longer a Component, I noticed that it is becoming difficult for the user to add information.

For example, in the code below, we can cancel only a specific sequence by attaching a marker component along with KeySequence.

Example

// Marker Component
#[derive(Component)]
struct KonamiCommand;

#[rustfmt::skip]
fn setup(mut commands: Commands) {
     commands.spawn((
         KonamiCommand,
         KeySequence::new(
            action::send_event(MyEvent(Direction::CounterClockwise)),
            keyseq!(ArrowUp ArrowUp ArrowDown ArrowDown ArrowLeft ArrowRight ArrowLeft ArrowRight B A),
        )
        .time_limit(Duration::from_secs(1))
         ));

    println!("Press W D S A to emit clockwise event.");
    println!("Press W A S D to emit counter clockwise event.");
}

fn event_listener(
    mut commands: Commands,
    sequences: Query<Entity, With<KonamiCommand>>
) {
    for entity in sequences.iter() {
        // cancel
        commands.entity(entity).despawn();
    }
}

However, would it require a slightly more extensive modification to solve this?
The solution I quickly came up with is to divide the KeySequence into a part that is visible user and a part that is used inner by the library.
Roughly, it is as follows.

proposed amendment

mod user {
    use bevy::prelude::Commands;
    use crate::input_sequence::lib::Sequence;

    fn spawn_sequence(
        mut commands: Commands
    ) {
        commands.spawn(Sequence::new(
            action::send_event(MyEvent(Direction::CounterClockwise)),
            keyseq!(ArrowUp ArrowUp ArrowDown ArrowDown ArrowLeft ArrowRight ArrowLeft ArrowRight B A),
        ));
    }
}

mod lib {
    use bevy::prelude::{Added, Commands, Component, Entity, Query};
    use crate::input_sequence::InputSequenceBuilder;

    #[derive(Component)]
    pub struct Sequence(Option<InputSequenceBuilder>);
    
    #[derive(Component)]
    struct SequenceInner(InputSequence);
    
    fn initialize(
        mut commands: Commands,
        mut sequences: Query<(Entity, &mut Sequence), Added<Sequence>>
    ){
        for (entity, mut sequence) in sequences.iter(){
            if let Some(builder) = sequence.0.take(){
                commands.add(move |world|{
                    world.entity_mut(entity).insert(SequenceInner(builder.build(world)));
                })
            }
            
        }
    }
}

[shanecelis]

Beggin' your pardon, Elm, but KeySequences are still components. I mean, it's a type alias for InputSequence<KeyChord, ()> but you can query the alias same as anything else. I wrote this code in the "keycode.rs" example that works just to double check myself:

fn event_listener(mut er: EventReader<MyEvent>, query: Query<&KeySequence>) {
    for e in er.read() {
        println!("{e:?} emitted.");
        println!("there are {} key sequences", query.iter().count());
    }
}

You're right there is a slight bit of obfuscation now because we're no longer spawning them directly. We're using the Command trait. That's because under the hood there's a InputSequenceBuilder that requires &mut World to turn that into an InputSequence.

Create a KeySequence entity and component.

commands.add(KeySequence::new(…));

Create a KeySequence component and add it to an entity.

commands.entity(id).add(KeySequence::new(…));
// OR
commands.spawn(…)
    .add(KeySequence::new(…));

Create a KeySequence component but don't add it to anything

commands.add(|&mut world| {
    let builder = KeySequence::new(…);
    let key_sequence = builder.build(world);
    // And then put it somewhere?
});

Perhaps I should add a few of these details into an advanced section of the README.

[not-elm]

Sorry, I made a mistake in my statement.
The correct statement is "KeySequence itself is a component, but what returned by KeySequence::new is not".
However, this problem can be solved by Create a KeySequence component and add it to an entity. as suggested above.

As for the documentation, I am thinking of adding it myself after merging.

The only remaining concern is The systems passed to KeySequence is executed only on the main thread. but I think this issue can be ignored.

If there are no particular problems, I will also verify the operation (I cannot check the operation of the gamepad because my Playstation Controller is broken) and merge it after I get home.

Thank you!

---

P.S.

Also, I would like to add or transfer you as the authority owner of this repository.What do you think?
(I've never done it before, so it might take some time.)

[shanecelis]

Admittedly returning a builder from new() isn't conventional but I liked the look of KeySequence::new(…) more than KeySequenceBuilder::new(…) which works now but seemed like an implementation detail we didn't need to draw attention to.

---

Sure thing. Happy to take on some maintainership duties if you like. You can add me as a collaborator to this repo. That'll give me push access to the repo. You can also add me as a owner on crates.io if you're comfortable with me publishing releases there.

[not-elm]

Merged after checking operation and no problems were found!
I also sent the collaborator invites as you told me. Please check it out.