Unexpected Behavior of Regular Buttons

[Noseey]

Background

Definitions:
Regular Button: Press and get only one action on either key-down or key-release. No hold functionality. (e. g. open Menu, toggle run)
Hold Button: Press and hold for continuous action. (e. g. firing a weapon, moving)
Longpress Button: Press to get one action - delay - and then continues action. (e. g. Text Input)

Version of Crispy Doom: 7.0.0

Operating System and version: Windows 10

Game: Doom, Heretic, Hexen, Strife(?)

Any loaded WADs and mods (please include full command line): -

### Bug description

Observed behavior:

Many Regular Buttons show unexpected behavior:

• Keyboard Buttons: Holding down the button >1s re-triggers the action in rapid frequence. Try "Automap" button to reproduce.
• Mouse Buttons: Holding down the mouse-button while moving the mouse re-triggers the action based upon mouse-movement. Try "Quick Reverse" button to reproduce.

I assume the observation might differ between Operating Systems (or even Hardware), depending on how the Raw Input is generated.

In the code, this relates to if - after performing the action - the button input is invalidated by writing directly in the Input Buffer, without applying further logic to avoid unwanted Input Updates until the physical button release, e. g. interrupts by long-press or mouse-movement.

I could see this behavior with Regular Buttons in both Vanilla Doom and Crispy Doom. Here a list of examples:

• Toggle Detail (Vanilla, Crispy)

• Toggle Messages (Vanilla, Crispy)

• Toggle Automap (Vanilla, Crispy)

• Toggle Grid, Add Marker (Vanilla, Crispy)

• Toggle always Run (Crispy)

• Toggle vert. mouse (Crispy)

• Quick Reverse (Crispy)
  ...

Recommendation:
Since the behavior is also present in Vanilla for many of those Regular Buttons, I think it doesn't make sense to completely fix the issue for all of them. Triggering a key long-press requires pressing down the button for about 1s, which practically does not happen alot and makes the issue a low priority for the most part. Also, most of the regular buttons can not be bound to the mouse in Crispy.

I would only bring in additional logic to avoid this behavior for affected Regular Buttons that can be bound to the mouse and/or have impact on player actions directly. The ID guys did that e. g. for the use-cmd evaluation in p_user.c:

This means, I would recommend to add it for:

• Quick Reverse (player impact, can be bound to mouse)
• Toggle Follow (can be bound to mouse)

Proposed fix:
master...Noseey:crispy-doom:Btn_Robustness

Let me know your thoughts and if you managed to replicate it, thanks!

[fabiangreffrath]

Does this help you?

--- a/src/i_input.c
+++ b/src/i_input.c
@@ -265,7 +265,7 @@ void I_HandleKeyboardEvent(SDL_Event *sdlevent)
             event.data2 = GetLocalizedKey(&sdlevent->key.keysym);
             event.data3 = GetTypedChar(&sdlevent->key.keysym);
 
-            if (event.data1 != 0)
+            if (event.data1 != 0 && sdlevent->key.repeat == 0)
             {
                 D_PostEvent(&event);
             }

[Noseey]

Does this help you?

--- a/src/i_input.c
+++ b/src/i_input.c
@@ -265,7 +265,7 @@ void I_HandleKeyboardEvent(SDL_Event *sdlevent)
             event.data2 = GetLocalizedKey(&sdlevent->key.keysym);
             event.data3 = GetTypedChar(&sdlevent->key.keysym);
 
-            if (event.data1 != 0)
+            if (event.data1 != 0 && sdlevent->key.repeat == 0)
             {
                 D_PostEvent(&event);
             }

Thanks alot for the idea of treating it on the Event level! Didn't cross my mind before. It would be a very clean solution fixing the regular button behavior for keyboard inputs across the board. The catch would be, however, that no more long-press will be possible for Text-Inputs (like the Multiplayer Chat), thus e. g. also no fast deletion of text by holding backspace. For me personally, that would be fine, but I'm not sure if everyone will be happy?

As for the mouse-behavior, I think, this is the piece of code causing the mouse-button inputs to be refreshed upon movement (or I should say to "hold" the buttons while moving):

Seems like this is required to maintain continuous action for mouse-bound hold-buttons when moving the mouse, since there is only one event defined for both buttons and movement together.

[fabiangreffrath]

You are perfectly right. The issue is that the mouse button state is updated each time a mouse event is triggered e.g. by mouse movement. The solution would be to have separate event types for mouse buttons and mouse movement (as e.g. in Woof!). However, I am not sure if I would want to deviate from Chocolate Doom in this regard or if it would be best to implement this split upstream first.

[Noseey]

I am not sure if I would want to deviate from Chocolate Doom in this regard or if it would be best to implement this split upstream first.

Yeah, I agree, as it is a common architectural feature of both ports. Not sure, if there will be alot of icentive to change it in Chocolate however, as they currently do not have affected mouse-buttons (most are hold-buttons, and next-/prev-weapon is covered). Unless they plan new mouse-buttons to be added, the change is of little use for them, I presume.

With that in mind, I'm drawn back to the initial suggestion to just apply a fix for the affected mouse-buttons in Crispy. Currently, it would require the user to not move the mouse while using the button, which is rather tricky, especially with thumb-buttons.

This would be my proposal (now including toggle-follow, initial post updated):
master...Noseey:crispy-doom:Btn_Robustness

It will just fix the overall issue half-way, but at least the two mouse-buttons are much more useable again. If you are interested, I can create a PR. Otherwise, I can attempt to create a ticket in Chocolate about it. Let me know your thoughts!

[fabiangreffrath]

I'd prefer to have this fixed upstream in general and not apply band aid for some specific keys in Crispy, to be honest.

[Noseey]

I'd prefer to have this fixed upstream in general and not apply band aid for some specific keys in Crispy, to be honest.

Ok, I have opened this ticket upstream for the mouse-event topic.