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

Add InputEvent<T> to StateEvent #1148

Open
jojolepro opened this Issue Nov 15, 2018 · 4 comments

Comments

Projects
None yet
4 participants
@jojolepro
Copy link
Member

jojolepro commented Nov 15, 2018

By default, users and amethyst examples are using the winit VirtualKeyCode to handle keyboard events. This should be changed to show goos pratices by using the events coming out of the EventChannel<InputEvent> in State::handle_event. To be able to do this, there will need to be a variant Input(InputEvent) added to StateData. The generic type on StateData should default to String.

@udoprog

This comment has been minimized.

Copy link
Member

udoprog commented Nov 16, 2018

I'll rephrase what I raised in another issue here.

If this is implemented we would have two places where we handle input events. For documentation purposes, and to reduce confusion, I'd like clarification on what is expected to be handled where. Or that we have a clear signal what the preferred way of handling input is.

I prefer handling all input in systems. There is a clearly defined data dependency, and the handler is managed by the ECS like almost everything else. I'm curious what others think about this.

@jojolepro

This comment has been minimized.

Copy link
Member

jojolepro commented Nov 16, 2018

I only handle inputs in states if they are directly triggering a state Trans.
Otherwise, I put everything else into systems too. (There's no harm in handling input in multiple places)

@torkleyy

This comment has been minimized.

Copy link
Member

torkleyy commented Nov 22, 2018

This discussion seems more about the general event handling story. Maybe this should be discussed in the forum or in another thread (to later be merged as an RFC). We should try to agree on a consistent event handling story, update the engine accordingly and document it.

@torkleyy

This comment has been minimized.

Copy link
Member

torkleyy commented Nov 22, 2018

Request by @jojolepro, let's implement this for now and discuss alternatives or improvements later given this is a big issue that would block progress on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment