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.

Sign up for GitHub

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

Fix lost click event when opening popup #5788

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+5 −2
Open

Fix lost click event when opening popup #5788

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/engine/input/RunOnKeyDownAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ public RunOnKeyDownAttribute(string inputName) : base(inputName)

public override bool OnInput(InputEvent @event)
{
var before = HeldDown;
if (base.OnInput(@event) && !before && HeldDown)
// Only trigger if the input was pressed this frame. It isn't possible to use !HeldDown since its value is
// incorrect if a mouse release event is marked as handled e.g. when opening a popup.
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't the actual fix be to make sure that up events are always handled? Because that's how I tried to design the input system in the first place. I'm sure I put in at least some code that should feed handled inputs that are release type events into the system so that the held down statuses wouldn't get stuck.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can see, HeldDown is only set to false in the OnInput and FocusLost methods. OnInput is only called if the input is unhandled (or the attribute has OnlyUnhandled). 'FocusLost' is only called when the window has focus lost.

Could you please point me towards the existing code? Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

That's probably exactly the bug, OnInput only passes events it sees but it has code that further key handlers are given all events if they are up events. So this situation likely needs a bit of refactoring so that handled up events would still be fed as up events into all the listeners (so I'd split OnInput into two parts, one that does the normal stuff and one that has just the event forwarding, and then I'd pass handled events that are key up events to the second part of the method).

// https://github.com/Revolutionary-Games/Thrive/issues/5217
var justPressed = Input.IsActionJustPressed(InputName);
if (base.OnInput(@event) && justPressed && HeldDown)
{
if (TrackInputMethod)
{
Expand Down