CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
10.26k stars 4.12k forks source link

Simultaneous [mouse] inputs (notably including MOUSE_MOVE) block other inputs from registering #61984

Open Acru opened 1 year ago

Acru commented 1 year ago

Describe the bug

If the mouse is moving at all, MOUSE_MOVE will register but all other mouse inputs and some keys (such as escape/enter) will be prevented from registering at all. This will occur in other edge cases as well, such as when both mouse buttons are pressed at once only one will register, but the above case is most problematic as slight mouse movements while pressing mouse buttons regularly occur.

Steps to reproduce

I tested this by unbinding inputs except MOUSE_MOVE, then moving the mouse while pressing buttons, and "unknown command" prompts do not occur while the mouse is moving.

Expected behavior

Ideally, all inputs would occur simultaneously/sequentially, but having mouse_move occur at a lower priority than all other inputs would fix the worst effects.

Versions and configuration

Version tested: windows-tiles-sounds-x64-2022-10-27-0619 (and msvc version)

PS: Registering middle mouse and the often available mouse-4 and mouse-5 (forward/back) buttons too would be nice~

ZeroInternalReflection commented 1 year ago

I can confuse the game a bit with left click + significant mouse movement, as the game starts to think that it's a click-and-drag action. However, I haven't been able to replicate this otherwise. Are there particular situations where this happens reliably for you?

PS: Registering middle mouse and the often available mouse-4 and mouse-5 (forward/back) buttons too would be nice~

SDL support for those should be relatively straightforward. Currently supported version of curses, on the other hand, are limited to a maximum of four mouse 'buttons', which in my testing was left click, middle click, right click, and the very useful scrollwheel up.

Night-Pryanik commented 1 year ago

I can confirm this, but only for pressing Esc or Enter. Pressing most other buttons, like Eat, Read, reload, activate, map, work fine.

Are there particular situations where this happens reliably for you?

Just move mouse crazy fast and try to press Esc or Enter.

Acru commented 1 year ago

I can confuse the game a bit with left click + significant mouse movement, as the game starts to think that it's a click-and-drag action. However, I haven't been able to replicate this otherwise. Are there particular situations where this happens reliably for you?

This happens constantly for me, I have to be careful not to move the mouse at all when clicking, or when using the side buttons which are mapped to pageup/down. (Also just checked, the keyboard key pgup/pgdn are blocked too while moving the mouse.) This may be worse for me due to having a high poling rate (1khz) high dpi (12000) gaming mouse?

I can confirm this, but only for pressing Esc or Enter. Pressing most other buttons, like Eat, Read, reload, activate, map, work fine.

Pageup/down and home/end are affected too, though not letter keys as far as I can tell. I haven't tested every key, but mouse button presses/releases and mouse wheel scrolling are affected.

ZeroInternalReflection commented 1 year ago

This may be worse for me due to having a high poling rate (1khz) high dpi (12000) gaming mouse?

That's probably a factor, since much of the input code is set up to be first-come first-served, so your mouse might be constantly beating you to the punch. I can now replicate this behaviour on my Cheapest Mouse Available (TM), but it requires a whole lot of movement. Rapidly swinging from one side of the screen to the other results in a ~50% hit rate on pressing Escape:

https://user-images.githubusercontent.com/89038572/198848885-e081848b-a4b8-445a-b67f-d200f64f6c1b.mp4

My thinking right now is that it's less a matter of "mouse movement should be considered last" and more "mouse movement should only be considered if it's large enough to impact the game". So, if you move your cursor to the next tile over, the game should notice and react, but if the cursor is just doing tiny random wiggles inside one tile, the game should be waiting for 'real' input. Now to see if there's any way I can test that with my crummy mouse.

Acru commented 1 year ago

So this can be labeled s (S2 - Confirmed) then? I'd like to see this fixed, and may give a go at it myself if noone else will, but I ran into something more serious to pin down first.

ZeroInternalReflection commented 1 year ago

I was working on a brute-force solution of only reporting a mouse move if it changed the ui coordinate, but I wasn't able to get it functional before I ran out of free time. In my video above, you can see from the tracking messages I added just how often MOUSE_MOVE gets reported on my crappy mouse. Presumably it's even more frequent on yours.

The idea was to add another filter on mouse movement here: https://github.com/CleverRaven/Cataclysm-DDA/blob/b804be3f68f225bca3dd09b47e6d4568e4c8f475/src/input.cpp#L1211-L1216

I should be able to do some more testing later this week.

Acru commented 1 year ago

Though I imagine this would still cause issue if the mouse cursor happened to be hovering around the border of a cell?

When using the scroll wheel to navigate a description, for instane, the targeted coordinate doesn't matter anyway and shouldn't be blocked by mouse movement even if the coordinate changes. Well, it doesn't matter now anyway though I imagine someone might eventually want to add code to scroll different text frames depending on which the mouse cursor is pointed to, but implementing such would not be simple~

It isn't clear to me yet how input is handled, and I don't know if I'm looking in the right area for this issue, but I came across these areas: https://github.com/CleverRaven/Cataclysm-DDA/blob/b804be3f68f225bca3dd09b47e6d4568e4c8f475/src/ui.cpp#L1122-L1123 https://github.com/CleverRaven/Cataclysm-DDA/blob/b804be3f68f225bca3dd09b47e6d4568e4c8f475/src/ui.cpp#L969-L971

Perhaps the mouse moving without a button press should cause should cause the function handle_mouse() to return as 'unhandled' rather than 'handled', or something like that, to allow for continued processing of other inputs, for whatever code that calls it? As I said, I don't know if I am looking in the right area, but if I was setup to compile I would just try editing it and see what happens. :3

ZeroInternalReflection commented 1 year ago

Though I imagine this would still cause issue if the mouse cursor happened to be hovering around the border of a cell?

I think what's happening with this bug is:

  1. The current screen is waiting for input
  2. It hands things off to input_context to get that input
  3. The mouse reports movement
  4. The current screen updates based on getting a "MOUSE_MOVE"
  5. The current screen redraws
  6. The current screen waits for input again
  7. Since steps 2->5 require processing time, if you're getting enough "MOUSE_MOVE" actions, it seems to flood the system, causing other inputs to be lost

You're correct that, under the right circumstances, hovering over a tile border might still cause the issue, which would be even harder to debug. On reflection, it might be better to have a (user-configurable?) maximum rate of MOUSE_MOVE actions. I'd need a very similar filter, but I imagine if you capped it at, say, 50 MOUSE_MOVE updates per second, you could still have snappy selection while being much less likely to lose other inputs. It would need a fair bit of testing, though

add code to scroll different text frames depending on which the mouse cursor is pointed to, but implementing such would not be simple~

I've actually implemented that on a couple of screens. See the results of #59856. It requires knowing the coordinates of what the user might consider a 'frame', which the code isn't always set up for, but otherwise isn't too tricky.

https://user-images.githubusercontent.com/89038572/182641147-2680732b-40bc-4c37-95be-ef28a02119b1.mp4

I came across these areas:

Which leads us to the other part of this mess. Detecting whether the mouse has moved is handled by input_context in input.cpp, with some help from other places like ncurses_def.cpp and sdltiles.cpp (depending on version). Updating the game based on that movement is done by the code of the particular ui screen. The code you've pulled up is specifically for uilists, one of the generic ui screens in the game. They look like this:

Butchery screen as a uilist example

Similar input-handling for, e.g. the crafting menu (&) is handled here:

https://github.com/CleverRaven/Cataclysm-DDA/blob/b804be3f68f225bca3dd09b47e6d4568e4c8f475/src/crafting_gui.cpp#L1605-L1610

So, whatever change is made, it should probably be in input_context or upstream of it. Otherwise, it'll require changes to dozens of different bits of ui code.

Acru commented 1 year ago

I've actually implemented that on a couple of screens.

Ah, neat, I hadn't noticed~!

Which leads us to the other part of this mess. Detecting whether the mouse has moved is handled by input_context in input.cpp, with some help from other places like ncurses_def.cpp and sdltiles.cpp (depending on version).

Okay, so the various ui elements call input_context::handle_input, which calls input_manager::get_input_event, presumably the one in wincurse.cpp for the windows version, though I wonder if the sdltiles/ncurses versions behave the same or are uneffected, due to the different implementations?

A possible solution though; input_context::handle_input could register if a mouse movement occured but continue looping until the timeout, and only then return the mouse move event rather than the timeout result. If another type of event occurs before the timeout elapses, the mouse move event is discarded in favor of immediately returning with the new event. This would effectively treat the mouse_move event as the lowest priority while quickly discarding any repetitive mouse movements? As long as the timeout period isn't too large, the delay in mouse movement related ui updates wouldn't be noticable, I'd bet, while also throttling how frequently such updates can occur.

ZeroInternalReflection commented 1 year ago

I finally got back to this and figured out how to a) test it reliably, and b) implement that loop within handle_input(). Unfortunately, I don't think I can satisfyingly 'solve' this.

So, the test is an xdotool script that makes a minor mouse movement ~1000 times/second. This results in abysmal keyboard response. I get a menu response somewhere around 20% of the time I hit Escape. This is probably a worst-case scenario for mouse input, but it's probably within the realm of possibility for a fancy mouse.

https://user-images.githubusercontent.com/89038572/206781041-c80a06fb-5439-4a85-9831-ddf1c0af86b9.mp4

The most successful implementation I've gotten is inserting some code into handle_input() that, similar to Acru's description above, recursively calls handle_input() if mouse movement is noted, prioritizing any other input. This improves the result, but does not solve it. My test is probably a worst-case situation, but here's how it looks when mouse movement is only reported every 10ms:

https://user-images.githubusercontent.com/89038572/206781959-94e87eaa-fa65-4309-a59b-1484e02987ea.mp4

Increasing the filter time to 17ms (~60/second) or higher further improves the likelihood of keyboard input getting through, but I was shocked at how big an impact in mouse responsiveness it had. Here's moving the mouse through a uilist with the 10ms filter:

https://user-images.githubusercontent.com/89038572/206782316-288a8c9d-57a2-4ddd-ac3e-7ce616faecc3.mp4

I have not been able to find a way to balance mouse responsiveness with keyboard input actually getting through. I think the options remaining are:

  1. Move the check further upstream into the code that talks to SDL (I imagine the curses implementation could also run into this issue, but I think you'd need to play around with most terminal settings to get it to report mouse movement often enough). I might play around with this, but I don't really expect success
  2. Have some sort of separate thread for queuing up inputs that stacks up mouse movement and only reports it when relevant. That sort of change is well beyond my abilities.
dannirook commented 1 year ago

Using cdda-windows-tiles-x64-msvc-2023-06-07-1828 . Desktop, using a gaming mouse and with the sensitivity cranked up.

I just wanted to jump in and conform that if you're using a gaming mouse with sensitivity cranked up, this bug basically breaks the game by making user input incredibly unreliable and frustrating.

I don't usually contribute to bug reports b/c I'm fairly tolerant of bugs (I play on experimental and have encountered a few, lol), but this one is really bad for me because it changes my user input in a way that feels "on the fly" and "random" and out of my control.

It's like if you had a ball IRL and were about to go throw it--and something invisible keeps slapping the ball out of your hands at the very last millisecond. Instant rage.

Last fall, with the same mouse, I played fine. This week, I actually have to turn my mouse over when I use the keyboard, so the sensor faces up. Otherwise the tiniest wiggle of my desk as I type, the tiniest vibration, steals keyboard focus and gives it to the mouse.

And that can have repercussions on playing the game.

Example: Due to this bug, I actually ended up stripping my character's pants off pre-fight instead of dropping the bags I was trying to select and drop, b/c the tiny, tiny vibration of me typing as I went through the drop screen caused the mouse cursor activate itself 10 rows down to select the pants, and I had already hit the key to drop w/out realizing my focus had been stolen by the frickin' mouse being moved half the width of a hair.

I mean, that's hilarious for all of 2 seconds. (Oh nos! I drop trou to intimidate the zombies!) But when similar things happen over and over and over again...it stops being funny.

The bug mostly affects me whenever I use inventory (by pressing d). The tiny vibrations completely mess with cursor focus and my cursor will keep jumping up/down/sideways/wherever to be where the mouse cursor was last left, instead of the cursor remaining on the selection screen where it should be going based on my keyboard input.

And because it responds to the tiniest of vibrations, such as the vibrations of me hitting a down or up arrow key to select something, the result is that the MOST likely time for it to happen is EXACTLY as I'm using the keyboard to do something b/c that produces vibrations.

Extremely frustrating.

**** If fixing this bug isn't possible--can we get a config option to disable the mouse entirely? I'm fine with turning mouse input off, I learned the game without it and don't need it to play. I just haven't been able to find any existing option for it.

Thanks.

NetSysFire commented 2 weeks ago

I reset the confirmed status. Please retest if this is still happening. There have been major changes to the UI.

Night-Pryanik commented 2 weeks ago

Tested right now. This is still an issue. Mouse movements prevent Esc and Enter from working.