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

[Editor] Remove redundant code from EditorSpinSlider #89518

Merged
1 commit merged into from
Mar 24, 2024

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Mar 15, 2024

While working on some other code I ran into some issues with the range when using up/down with the inspector, the underlying range already does clamping, which is configurable with greater/less, so all this code does is ignore those flags, otherwise the code is redundant

This makes the behavior of the spin up/down consistent with the key input (which is especially relevant for float exports as they don't show the up/down)

Also moved some of the code into the cases where it's used, and cleaned up some minor details

Tested and confirmed the code without clamping works correctly, not reintroducing the issues described in:

@KoBeWi
Copy link
Member

KoBeWi commented Mar 17, 2024

This makes the behavior of the spin up/down consistent with the key input (which is especially relevant for float exports as they don't show the up/down)

Doesn't seem like inspector EditorSpinSliders property handle key input.

How this can be tested?

@AThousandShips
Copy link
Member Author

Doesn't seem like inspector EditorSpinSliders property handle key input.

How do you mean? Select the text input and use the up/down keys

@AThousandShips
Copy link
Member Author

AThousandShips commented Mar 17, 2024

To illustrate how this is extra confusing and nasty:

  • Create a script exporting a float, just plain export, no range needed (the default configuration is equivalent to "-99999,99999,1,or_greater,or_less,hide_slider")
  • Enter 200000 into the property
  • Select the property and push the arrow up key

It will jump back to 99999, the default upper range value, the fact you can't go from 99999 to 100000 alone is a bit annoying, but that side was what made me really confused when I was working on another PR, and what made me investigate this code

akien-mga added a commit that referenced this pull request Mar 24, 2024
[Editor] Remove redundant code from `EditorSpinSlider`
@akien-mga akien-mga closed this pull request by merging all changes into godotengine:master in 06abc86 Mar 24, 2024
@akien-mga
Copy link
Member

Thanks!

@AThousandShips AThousandShips deleted the spin_slider_clean branch March 24, 2024 08:45
@AThousandShips
Copy link
Member Author

Thank you!

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

Successfully merging this pull request may close these issues.

3 participants