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

The pointer skips away by dragging over an integer value in the Inspector #25046

Closed
capnm opened this issue Jan 16, 2019 · 11 comments
Closed

The pointer skips away by dragging over an integer value in the Inspector #25046

capnm opened this issue Jan 16, 2019 · 11 comments

Comments

@capnm
Copy link
Contributor

capnm commented Jan 16, 2019

OS
Linux Ubuntu 18.04 LTS

Godot version:

07e2046

Issue description:

While testing the issue #22652 (comment)
I discovered an another issue:

  • open the project
  • drag the LMB above the Custom Fonts -> Size [42] spinbox
    the pointer jumps away 8-/

image

@capnm capnm changed the title Mouse cursor jumps away after adjusting a int value in inpector Mouse cursor jumps away by adjusting a int value in inpector Jan 16, 2019
@capnm
Copy link
Contributor Author

capnm commented Jan 16, 2019

Not sure if this is already known, by trying to set the outline size below 0:

ERROR: set_outline_size: Condition ' p_size < 0 || p_size > (255) ' is true.
   At: scene/resources/dynamic_font.cpp:727.

@akien-mga akien-mga added this to the 3.1 milestone Jan 16, 2019
dragmz added a commit to dragmz/godot that referenced this issue Jan 23, 2019
slapin pushed a commit to slapin/godot that referenced this issue Jan 28, 2019
@akien-mga
Copy link
Member

Confirmed in 88a36e2. It's not critical though so moving to next milestone.

@akien-mga akien-mga modified the milestones: 3.1, 3.2 Feb 25, 2019
@bojidar-bg
Copy link
Contributor

Is this still valid?

@capnm
Copy link
Contributor Author

capnm commented Sep 16, 2019

Is this still valid?

AFAIK nobody has even tried to fix this issue.
You can check that with the mentioned project:
https://github.com/godotengine/godot/files/2646416/mouse_cursor_bug.zip

@capnm capnm changed the title Mouse cursor jumps away by adjusting a int value in inpector The pointer skips away by dragging over an integer value in the Inspector Sep 16, 2019
@akien-mga
Copy link
Member

Still reproducible in the current master branch.

Might be Linux specific though and linked to their way the cursor is captured. It's possible that dragging the custom font size value triggers an automatic update to refresh the font which interferes with the captured cursor.

@akien-mga akien-mga removed this from the 3.2 milestone Jan 13, 2020
@Windsdon
Copy link

I can confirm that this bug happens on Windows.

Also, I should add that this bug seems to only happen on the font size slider. Other sliders work fine.

Current setup:

  • Godot Engine v3.2.2.stable.mono.official
  • Windows 10 Enterprise 1903

@pouleyKetchoupp
Copy link
Contributor

pouleyKetchoupp commented Jul 25, 2020

The same issue actually happens with any property that uses _change_notify() without an argument on a property with range.

When you search in the codebase, there are lots of other potential cases. I've tried one randomly (hframes & vframes in Sprite) and it does occur too.

I wonder what would be a possible approach to fix it all over the engine:

  1. Search through the codebase for similar cases and change all _change_notify() into specific property notifications like Fix DynamicFont breaking mouse grab in inspector spinners #40701 does. That would work but it's a lot of manual changes.

  2. Maybe there could be a way to fix it on the spinner itself, or on the inspector? The issue is that EditorSpinnerSlider makes the mouse invisible, and sets it back on mouse release, while warping it to its previous position. In case the inspector is entirely refreshed, the mouse is set back to visible without being warped back, which causes it to end up at the center of the screen. Warping it back when the spinner is refreshed would cause the cursor to flicker while changing the edited value, so I'm not sure how it could be fixed.

@opl-
Copy link
Contributor

opl- commented Jul 25, 2020

While this could possibly be fixed everywhere, for example by somehow storing the control focus while rebuilding the tree or changing the strategy used when _change_notify is called without specifying a property, I'm not sure what a good way to approach that would even be. Overall calling _change_notify with the names of the properties that changed should be more efficient, as instead of recreating the whole inspector tree only the relevant property inspectors get updated. (Relevant code.)

Storing the control focus sounds like a lot of effort for what is ultimately a workaround for missing code. Changing the strategy could be an improvement but I believe would require having access to the previous property value. This is technically already stored in the nodes of each EditorProperty, at least for the primitive types, so it might be possible by implementing a new EditorProperty method, but ultimately _change_notify calls should probably be as specific as possible anyway to improve performance (which could be especially useful when dealing with large arrays, for example).

opl- added a commit to opl-/godot that referenced this issue Jul 25, 2020
This was caused by DynamicFont not specifying which property was edited, resulting in the whole inspector property tree being recreated.

Fixes godotengine#25046
@akien-mga akien-mga added this to the 4.0 milestone Mar 23, 2021
@akien-mga
Copy link
Member

Fixed in 3.x with #47235, though AFAIU it's still valid in master.

@opl-
Copy link
Contributor

opl- commented Sep 16, 2021

From what I remember this issue existed because _change_notify wasn't being called with the name of the property which was changed, causing the inspector to rebuild the UI the user was interacting with. Since #45879, _change_notify no longer exists, meaning that unless the new method of detecting changes is somehow broken, it should work on master. I can't test this myself at the moment.

@akien-mga
Copy link
Member

Thanks, I've tested quickly and I can't seem to reproduce the issue in master, so it might indeed be solved too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants