chocolate-doom icon indicating copy to clipboard operation
chocolate-doom copied to clipboard

Mouse handling behavior leads to unintended side effects

Open hfc2x opened this issue 4 years ago • 6 comments

Background

Version of Chocolate Doom: 3.0.0 and onwards (including development build)

Operating System and version: Windows 10 64-bit

Game: All

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

Bug description

Observed behavior: This is probably related to issue #722, since it deals with mousewheel not behaving as expected. I recently bought a 5-button mouse, and decided to try it with Chocolate Doom, but the side buttons behave exactly as mousewheel scroll because of the way Chocolate Doom processes mousewheel events. This means mouse buttons 4 and 5 become impossible to bind to any other action, since they'll always be treated as duplicates for mousewheel scrolling motion. Besides this, mousewheel scroll is treated as multiple button presses every single time it moves, instead of a single one, causing this button setup to make weapons change multiple times whenever the mousewheel moves, which causes number keys to be much more reliable when changing weapons.

Expected behavior: Extra mouse buttons should be able to be bound to actions and not conflict with mousewheel movement. Mousewheel movement should not act like multiple button presses every time it moves.

hfc2x avatar Jan 12 '21 22:01 hfc2x

Indeed choco-doom has the MapMouseWheelToButtons() function that turns scroll down in Button 4 down/up and scroll up in Button 3 down/up. Should not be hard to fix in i_input.c for SDL2.

in SDL_mouse.h
#define SDL_BUTTON_X1       4
#define SDL_BUTTON_X2       5

as the UpdateMouseButtonState() function decrements the button bumber by one it means that there is really a mapping between wheel up/down and X buttons 1/2. this could be fixed by using bigger numbers for the MapMouseWheelToButtons function

RamonUnch avatar Apr 20 '21 13:04 RamonUnch

@hfc2x Try with those choco exes. The fix is trivial, just replace

    if (wheel->y <= 0)
    {   // scroll down
        button = 4;
    }
    else
    {   // scroll up
        button = 3;
    }

by

    if (wheel->y <= 0)
    {   // scroll down
        button = 6;
    }
    else
    {   // scroll up
        button = 5;
    }

in the static void MapMouseWheelToButtons(SDL_MouseWheelEvent *wheel) function

choco3.0.1-FIXED.zip I can not test myself I only have 3 mouse buttons. Note that you will have to re-configure the scroll wheel...

RamonUnch avatar Apr 20 '21 19:04 RamonUnch

Note that CrispyDoom seems to also have the bug, can you confirm?

RamonUnch avatar Apr 20 '21 19:04 RamonUnch

Tried these executables, with a new cfg created from scratch, and this seems to resolve the issue. However, this only after editing the mouse inputs by hand in the config file, since the Setup utility doesn't treat the side buttons and the mousewheel as separate. The problem I see with this is that it will only fix the issue for mice that have a total of 5 buttons, though. In theory, for a more permanent fix, the button numbers representing the mousewheel have to be really high and out of reach (maybe 90 or something?), to prevent mice with a higher button count to ever conflict with them.

I'm not sure if making a pull request with those changes would get rejected, however.

hfc2x avatar Apr 20 '21 23:04 hfc2x

SDL2 only supports 5 buttons, so if I fix the setup.exe it should be fine. To get access to more than 5 buttons there are no longer any documented ways under Windows other than the deprecated directinput.

EDIT: I could use button 30 and 31 in case SDL2 would map more buttons probably incrementally for future releases. note that 32 is the max value fr a button with the current code. EDIT2: Actually the MAX_MOUSE_BUTTONS is set to 8 in i_input.h and if i do not chnage it then the values have to be 6 and 7.

RamonUnch avatar Apr 21 '21 05:04 RamonUnch

@hfc2x Here is the final version of the fix. It took some time to figure where the wheel->button translation was made for the setup.exe (actually it is not in the src directory but in textscreen/txt_main.h). Tell me if all your 5 button + Wheel works fine for you and I will make the PR. I use Wheel Up = Button 7 (internally button 6) And Wheel Down = Button 8 (Internally button 7) So there is room for a X-BUTTON 3 that would map to Button 6 (5 internally) but it is just speculation because SDL2 has no documented support for more than 5 buttons... It is at least better than the current situation. All choco-exe are Zipped here. the chocolate-setup.exe is fixed as well but should not be used alongside the older exe. Note that this is also the last 'main' version of choco. and not the v3.0.1 or 3.0.0 even though it is written 3.0.0 Chocolate-doom-5buttonFix.zip

RamonUnch avatar May 02 '21 08:05 RamonUnch