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

Dragging mouse to select text in script editor may randomly change selection range and trigger drag #93078

Closed
alvinhochun opened this issue Jun 12, 2024 · 5 comments · Fixed by #93105

Comments

@alvinhochun
Copy link
Contributor

Tested versions

  • Reproducible in: 4.3 (5241d30 GHA Windows editor), 4.3 (2fb296a GHA Windows editor)
  • Not reproducible in: 4.3 beta1 (Windows editor), 4.3 (a4f2ea9 GHA Windows editor)

System information

Windows 10 - Godot v4.3.beta (5241d30)

Issue description

When dragging the mouse cursor in the script editor from outside selected text (or when there isn't a text selection), sometimes (seemingly randomly) the selection range will suddenly extend to the mouse down location and initiates an unwanted drag operation. This happens without touching any keyboard keys.

capture.webm

Steps to reproduce

  1. Open a script
  2. Keep using the mouse to select ranges of text
  3. Sometimes also include clicking to cancel text selection

Minimal reproduction project (MRP)

N/A

@AeioMuch
Copy link
Contributor

Bisected #92424 as the first bad commit.

@kitbdev
Copy link
Contributor

kitbdev commented Jun 12, 2024

I can't reproduce (Godot v4.3.beta (475248d) - Windows 10.0.22631 - Vulkan (Forward+)).
But it looks like TextEdit is receiving mouse unpressed and pressed notifications when you didn't actually release the mouse button.
Does this also affect actions like panning or selecting in the scene view, or using ui sliders, or is it only the script editor?

@alvinhochun
Copy link
Contributor Author

Bisected #92424 as the first bad commit.

Thanks for bisecting. I suspect it is because the patch uses GetAsyncKeyState instead of GetKeyState. Hypothetically, if one press the mouse button while moving the cursor, the window could be receiving these messages (pseudo code):

  1. MouseMove, buttons=None, xy=(100, 100)
  2. MousePress(Left), buttons=LeftMouse, xy=(101, 101)
  3. MouseMove, buttons=LeftMouse, xy=(102, 102)

When the application processes the first message, it is possible that the user has already pressed the mouse button so GetAsyncKeyState(VK_LBUTTON) may indicate the mouse button is pressed, different from what the message represents. More importantly it would indicate the mouse button being held down before the MousePress message had been processed. If the control relies on the MousePress message to initialize states related to text selection, it could explain the bug. (This is just a hypothesis -- I haven't checked what is happening within Godot.)

I think it really should use GetKeyState instead.


I can't reproduce [...]. But it looks like TextEdit is receiving mouse unpressed and pressed notifications when you didn't actually release the mouse button. [...]

In my capture, when the selection stops changing I definitely released the left mouse button. If my hypothesis above is correct, then the key point is that the mouse must be moving when pressing the mouse button to reproduce the bug.

If you really need to verify, I can make a capture with an OSD showing the mouse buttons (though I don't currently have a tool for this so I'll need to hunt for one).

@KoBeWi
Copy link
Member

KoBeWi commented Jun 12, 2024

Can confirm in v4.3.beta.custom_build [475248d]
Windows 10.0.19045

@kitbdev
Copy link
Contributor

kitbdev commented Jun 12, 2024

I misunderstood earlier, I can reproduce it now while moving the mouse.

Adding logpoints show that a mouse motion event with the left mouse button down is being sent before the mouse pressed event.

image

Created a PR #93105 that uses GetKeyState as suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Immediate Blocker
Development

Successfully merging a pull request may close this issue.

5 participants