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

sdl_input: rewrite IN_SetMouseMode to make it more readable #1141

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

illwieckz
Copy link
Member

@illwieckz illwieckz commented May 13, 2024

  • sdl_input: rewrite IN_SetMouseMode to make it easier to read
    I cannot grasp the code in the current state so I cannot debug it.

  • sdl_input: keep the mouse in current state when toggling fullscreen
    This fixes the mouse capture being lost on Alt-Enter
    .

  • sdl_input: also centers the mouse pointer when entering delta mode
    My experience with NetRadiant taught me that not centering a mouse pointer when entering delta mode may help the mouse to escape the capture. It may be possible that this is useless with SDL, but it can't do harm to do it so I prefer to be extra-precocious.

@illwieckz illwieckz added T-Bug T-Improvement Improvement for an existing feature A-Client labels May 13, 2024
@slipher
Copy link
Member

slipher commented May 14, 2024

  • sdl_input: keep the mouse in current state when toggling fullscreen
    This fixes the mouse capture being lost on Alt-Tab.

Did you mean Alt-Enter?

@slipher
Copy link
Member

slipher commented May 14, 2024

Just tried it and sdl_input: keep the mouse in current state when toggling fullscreen has the consequence (on Windows at least) that if you vid_restart while in CustomCursor mode (I did it by using a delay command in the maim menu) and move the cursor outside the window, you get two cursors displayed (custom and system) afterward. I think this is a worse bug than the cursor potentially not being captured after Alt-Enter. The existing behavior of setting the mode to SystemCursor on a restart makes the most sense to me because (a) the game may not respond to inputs for a long time at this point; and (b) both of the other modes depend on a specific cgame state to make sense, which may not exist after the restart.

@slipher
Copy link
Member

slipher commented May 14, 2024

Fullscreen toggling normally does in_restart but not vid_restart. in_restart is not used by anything else, so we could make it have a different behavior wrt mouse mode since there should not be a long delay and the cgame state doesn't change.

@slipher
Copy link
Member

slipher commented May 14, 2024

Try #1146.

I cannot grasp the code in the current state so I cannot debug it.
My experience with NetRadiant taught me that not centering a mouse pointer
when entering delta mode may help the mouse to escape the capture.

It may be possible that this is useless with SDL, but it can't do harm to
do it so I prefer to be extra-cautious.
@illwieckz illwieckz changed the title sdl_input: rewrite IN_SetMouseMode to make it more readable and fix mouse escape on fullscreen toggling sdl_input: rewrite IN_SetMouseMode to make it more readable May 15, 2024
@illwieckz
Copy link
Member Author

Even with #1146 I still have interest in this branch (once the fullscreen toggling commit removed).

I don't trust the current code because I don't understand it enough, it's too much implicit.

I experience many input issues like having the pointer centered while the game is not focused, or not being able to click on focused windows because the game continue to steal the input, etc, so I want to make sure that all I want the engine do in that function is done by the engine, to be able to know if the bugs may come from something else than the engine or not.

@slipher
Copy link
Member

slipher commented May 16, 2024

I prefer the old version. But of course I'm biased, as the last person to rewrite that function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Client T-Bug T-Improvement Improvement for an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants