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

[HTML5] Fix input not working when buffered. #52604

Merged
merged 2 commits into from
Sep 14, 2021

Conversation

Faless
Copy link
Collaborator

@Faless Faless commented Sep 12, 2021

After input buffering was reworked, input accumulation is now handled outside of OS, and the JavaScript plaform never implemented that.
Additionally, the JavaScript platform is quite obnoxious about calling specific APIs outside specific user triggered events.

This commit adds event flushing during the main iteration, and forces it during keydown/keyup/mousedown/mouseup/touchstart/touchend/touchcanel events (effectively only accumulating only "move" events).

Fixes #52468.
3.x version: #52603

After input buffering was reworked, input accumulation is now handled
outside of OS, and the JavaScript plaform never implemented that.
Additionally, the JavaScript platform is quite obnoxious about calling
specific APIs outside specific user triggered events.

This commit adds event flushing during the main iteration, and forces it
during keydown/keyup/mousedown/mouseup/touchstart/touchend/touchcanel
events (effectively only accumulating only "move" events).
@mhilbrunner
Copy link
Member

mhilbrunner commented Sep 13, 2021

Would it make sense to do this only in process_events() and additionally add a conditional that does this inside input->parse_input_event()? Or add a setting to input like "flush after every event" that is enabled for JS, and then input->parse_input_event() flushes on each call?

Reason being
1.) this seems messy to have in 3 places that generate input events,
2.) this seeming error prone if we add more places that generate input events, because the implementer always needs to remember to also add the flush.

Then again, that might be overkill. Besides this, this looks good and to be the correct fix.

@Faless
Copy link
Collaborator Author

Faless commented Sep 14, 2021

Or add a setting to input like "flush after every event" that is enabled for JS, and then input->parse_input_event() flushes on each call?

The main benefit from accumulation is in the handling of move and drag events. Which can be fired very rapidly.

I understand having the flush in the callbacks is a bit messy, but it's unlikely we'd add more events of that kind, because those really are the only relevant actions. I.e. we don't want to flush on every event, we only want to flush on clicks/press so we can interact with the APIs that requires direct user input.

EDIT: I also have a PR that unifies the key callbacks, which will save some more duplication.

Copy link
Member

@mhilbrunner mhilbrunner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

Godot Web Editor does not respond to keyboard or mouse input
2 participants