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

Avoid double-firing of keyPressed events #2757

Merged
merged 1 commit into from Apr 23, 2018

Conversation

FedericoStra
Copy link
Contributor

The IngameKeyListener seems to double fire keyPressed events.
The net effect is that it is impossible to use hotkeys such as 'B'
because the dropdown construction menu just flashes for an instant
and then closes back down immediately.

I suspect this is a deeper issue inside fifengine, but I am not
familiar with that so here I offer a quick fix to avoid the problem.

Simply put, the IngameKeyListener keeps track of the last event
and, if the current one is identical (same key, same press/release
status), it directly discards it without further processing.

Furthermore, I also cache an instance of KeyConfig inside
IngameKeyListener instead of instantiating one every time
keyPressed or keyReleased are called. This just popped to my eyes
as an obvious optimization to implement, but feel free to discard if
it gives raise to some problems.

The `IngameKeyListener` seems to double fire `keyPressed` events.
The net effect is that it is impossible to use hotkeys such as 'B'
because the dropdown construction menu just flashes for an instant
and then closes back down immediately.

I suspect this is a deeper issue inside `fifengine`, but I am not
familiar with that so here I offer a quick fix to avoid the problem.

Simply put, the `IngameKeyListener` keeps track of the last event
and, if the current one is identical (same key, same press/release
status), it directly discards it without further processing.

Furthermore, I also cache an instance of `KeyConfig` inside
`IngameKeyListener` instead of instantiating one every time
`keyPressed` or `keyReleased` are called. This just popped to my eyes
as an obvious optimization to implement, but feel free to discard if
it gives raise to some problems.
@FedericoStra
Copy link
Contributor Author

Here is the associated issue: #2758

@LinuxDonald
Copy link
Member

@helios2000 could it be an fife/fifechan problem?

@FedericoStra Thank you for this pr :) First we will took on the issue whats going on.

@LinuxDonald
Copy link
Member

@AndyMender could you maybe review this please? :)

@AndyMender
Copy link
Member

Looks alright to me, but I too lack discrete knowledge on how Unknown Horizons handles key presses. @FedericoStra if you don't mind, we'll hold back with merging for now as our test server is in a shamble and many of the tests fail even locally. I would like to address that first.

@AndyMender
Copy link
Member

Bumping. @FedericoStra did you test that your patch addresses the issue you experience? I tried reproducing your problem, but for me the build menu works fine (appears after pressing 'B' once and disappears after pressing 'B' a second time).

In principle, I can merge this, though sadly I cannot reproduce your bug :(.

@LinuxDonald LinuxDonald merged commit 017a074 into unknown-horizons:master Apr 23, 2018
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.

None yet

3 participants