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

You can't override the "escape" key firing a quit event #97

Closed
icefoxen opened this issue Jul 9, 2017 · 11 comments

Comments

@icefoxen
Copy link
Contributor

commented Jul 9, 2017

I thought you could but it turns out not. This should be fixed; you can ignore or handle the quit event, but there's no way to make pressing "escape" not fire it.

@icefoxen

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2017

We... can't actually make EventHandler::key_down_event() call Context::quit() by default because it never has access to a Context.

So far it seems the only sensible thing is to remove this feature entirely and make sure the tutorials have a key_down_event() handler to handle escape button presses.

@icefoxen

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2017

IT'S EVEN WORSE because key_down_event() doesn't get a Context either, so it can't call Context::quit().

When I wrote EventHandler I couldn't think of any use case that would need the Context: most operations you need it for are expensive things like creating resources, which are best done outside event handlers, or things like drawing which have their own event. However, we are slowly accumulating more options to mess around with the game/window state, like graphics::set_mode(), and accessing those needs the Context.

So it seems like the Right Thing might be to update the EventHandler trait to pass the Context to the event callbacks as well, however that is a breaking change so would have to wait for 0.4.

@obsoke

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2017

I agree that this is the right change to make; otherwise, you are left setting flags from the event handling methods to then act upon in update. Better to just act directly, IMO.

In a similar vein, what about arguments passed to update such as dt? Would those be useful in other methods? e.g.: From key_down_event, some other method would be called when Space is held down to do something like speed * dt to move a character.
Or perhaps being able to get a Duration object from the Context representing the current delta time would be a cleaner alternative to passing an additional argument to each of the EventHandler methods.

@icefoxen

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2017

Eeeeeeh. I'm wary of passing dt to event handlers because the Right Way is almost always to do as little as possible in event handlers besides register that events happened. Getting a Duration from the context isn't a terrible idea though...

@Manghi

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2017

@obsoke Small tidbit: the way I avoided this was to set a flag which would detect if a key was already pressed.

For a small menu system which has an option to exit the game, I had to manually call Context::quit() that way. Wasn't that big of deal, but it sounds like its not the best method??

@rofrol

This comment has been minimized.

Copy link

commented Jul 22, 2017

@icefoxen Maybe add to 0.4 milestone?

@icefoxen icefoxen added this to the 0.4 milestone Jul 22, 2017

@icefoxen

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2017

Oops, thought I had already.

@Manghi

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2017

@icefoxen You mentioned that you could ignore the Context::quit()-from-ESC-key event in your first post.

How exactly would this be done?

@icefoxen

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2017

Override the quit_event method on your EventHandler and have it check whether the escape key is pressed, exactly like you said. If the quit_event method returns true, the game does not exit.

@Manghi

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2017

quit_event, so obvious. Thanks! Works like a charm.

icefoxen added a commit that referenced this issue Sep 25, 2017

Refactored EventHandler interface
Big breaking change here!  Resolves issue #97

 * Removed dt from update; get it from timer::get_delta()
 * Added a context passed to all event types, so you can at least
   query state like timer info, number of controllers, etc.
 * Removed the "escape button quits game" behavior that was hardwired
   into the stupid event loop handler; added it to the default key_down
   handler
@icefoxen

This comment has been minimized.

Copy link
Contributor Author

commented Sep 25, 2017

DONE! \o/

@icefoxen icefoxen closed this Sep 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.