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

Enhance Godot's EditorSpinSlider #6988

Open
MewPurPur opened this issue May 31, 2023 · 6 comments
Open

Enhance Godot's EditorSpinSlider #6988

MewPurPur opened this issue May 31, 2023 · 6 comments

Comments

@MewPurPur
Copy link

MewPurPur commented May 31, 2023

Describe the project you are working on

This applies to pretty much any Godot project.

Describe the problem or limitation you are having in your project

Editor spinsliders are a little janky... This can be an annoyance when they don't do something you expected them to, and just gives Godot an impression of being unpolished.

Here's a bug report from me that lists a dozen such small janky issues: #76620

I'm making this proposal because I've started working on fixing up spinsliders a little and since it's looking like a code overhaul (although I'll be keeping the old look for the most part), I want to gather feedback on a specification I wrote.

On top of the jank, I expect some extra things from spinsliders, here are some:

  • The specific arrows to be highlighted to let me know if the value will be incremented or decremented, not both arrows at the same time. Also, I expect the visual feedback to be more noticeable.

image

  • For specific arrows to have a disabled mode showing when you're at the min/max value and they won't work (as well as actually not working, see the linked issue)
  • For the slider to remain highlighted while you're dragging it.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Rework the EditorSpinSlider. The API should be the same and I will try to avoid significant visual changes. Specifications have been posted below.

To understand the specifications, I clarify that EditorSpinSlider has two types of grabbers, which are drawn when hide_slider is false:

  • Spinner: For editing integers. Shows two arrows on the right.
  • Slider: For editing floating point numbers. Shows a slider at the bottom.

To be clear: I AM WORKING ON THIS CURRENTLY, AND TRYING TO MEET THE SPECIFICATIONS LISTED. I hope I'm not missing anything, but if you notice something missing, please do tell me as the below list, aside from theming, is supposed to be exhaustive of what I want to achieve.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

This refers to an editable spinslider with grabbers enabled. Should be obvious which things don't apply to read-only spinsliders or ones with hidden grabbers.


When hovering the box

  • Mouse idle = Show the slider handle. If no motion for a while, show a tooltip with the value and some of the tricks documented
  • Mouse click and release = Enter value editing mode and emit value_focus_entered
  • Mouse drag and move = Enter value dragging mode
  • Mouse exit = Go back to the standard look

When hovering a grabber

  • Slider + Mouse idle = Highlight the slider
  • Spinner + Mouse idle = Highlight the specific spinner arrow that's hovered (WORK NEEDED)
  • Mouse click and move = Enter grabber dragging mode and emit grabbed
  • Mouse exit = Go back to the standard look (WORK NEEDED)

Value editing mode

  • Regular text editing in a temporary LineEdit. A validated number from the text affects the editor, but the changes are not registered
  • Enter / Click away = Submit the text, validating and adjusting it. Emit value_focus_exited
  • Esc = Reject the text
  • Up/Down = Change the value up/down by 1 or by the step if the step isn't an egyptian fraction (e.g. 0.25). Multiply the change by 1/10, 10, or 100 if the corresponding Alt/Shift/Ctrl is pressed.

Value dragging mode

  • As you drag to the left or right, the value gets changed based on interface/inspector/float_drag_speed. If the value reaches minimum or maximum, don't make the user scroll all the way back. The changes show up in the editor, but are not submitted (WORK NEEDED)
  • Goes outside of the property's range if allowed.
  • Mouse release = Submit the changes and release the mouse in the position where it was confined (WORK NEEDED)
  • Right click, Esc = Reject the changes
  • Shift = More precise changes
  • Ctrl = Round to integers
  • Alt = Broader changes

Grabber dragging mode

  • The changes show up in the editor, but are not submitted (WORK NEEDED)
  • Mouse release = Submit the changes and emit ungrabbed (WORK NEEDED)
  • Slider
    • As you drag around, the value is adjusted to match the cursor's X position
    • If the mouse exits the slider area: Keep dragging and keep the slider highlighted (WORK NEEDED)
  • Spinner
    • As you click it, the value is changed by 1 or -1 based on the arrow hovered
    • If you hold down the mouse inside the arrow's area: After a 0.6sec cooldown, the value starts to change every 0.05sec. These aren't adjustable (WORK NEEDED)
    • If the mouse exits the arrow's area: Stop changing the value, submit the changes (NO NOTION YET)
    • If the arrow becomes disabled because you're at the property min/max value: Submit the changes (NO NOTION YET)

Read-only spinslider

For this one, keep it mostly the same as how it's now. However, use the new disabled spinner style, and hide the slider's marker.

Specifics specifics

Grabbing a slider must not count until the mouse has been moved 4 * EDSCALE units.

RTL layout must remain unaffected.

How spinsliders work inside other controls such as Vector3i or Transform2D is mostly ignored, as the vector widgets are separate things, but nonetheless they should still work.


If this enhancement will not be used often, can it be worked around with a few lines of script?

Nah nah, also it will be used constantly by everyone.

Is there a reason why this should be core and not an add-on in the asset library?

It's an editor feature enhancement.

@MewPurPur
Copy link
Author

Will continue working on this when the clamp issues, read-only display, and validation issues are addressed.

@MewPurPur
Copy link
Author

Updated the post a bit as a few things have happened over the months, like godotengine/godot#88275. I'll still keep it around, but I think the main big interesting thing here is the idea of changing up the integer arrows to be separate and have multiple states.

@Calinou Calinou changed the title Enhance Godot's editor spinsliders. Enhance Godot's EditorSpinSlider Feb 17, 2024
@MewPurPur
Copy link
Author

MewPurPur commented Mar 8, 2024

This PR sets a precedent, but doesn't apply to editor spinsliders as I'm writing this.

@davthedev
Copy link

Thanks for pointing me out to this widget. We can do something similar to it as I proposed on SpinBox and it would greatly improve the usability of setting various properties!

For theme resources, I would put styleboxes & icon tintings for the normal/hover/pressed/disabled states of each arrow. One icon setting per arrow can be enough as this is an editor-specific widget.

Just a thing IMHO:

Grabbing a slider must not count until the mouse has been moved 4 * EDSCALE units.

I personally hate UIs that require me to move a slider for a minimum travel distance before taking my input into consideration. This is very annoying when fine-tuning stuff below the slider min change threshold; you have to go out of your way to hit the threshold, then slide back to your fine-tuning, losing your starting point in-between.

Keeping the arrows for decimal values can be helpful for such fine-tuning. In this case, the step should be adjusted to a magnitude that corresponds to the value being edited, for example 0.01 per increment.

We can also think about having steps of acceleration when the mouse button remains pressed on an arrow. For example, there is the 0.6 sec wait, then 1 tick per 0.05 second for 2 second in total, then 10 ticks per 0.05 second for the next 2 seconds, then 100, then 1000... adapted to the edited value. I have seen that in old Windows 98 spinner controls.

@MewPurPur
Copy link
Author

I personally hate UIs that require me to move a slider for a minimum travel distance before taking my input into consideration

That's already in the current implementation. And it's very important to have, actually. GUI defaults are a multilayered compromise. If you help your experience, you'll ruin that of people with a mouse or touchpad that easily moves when they click.

@davthedev
Copy link

davthedev commented Mar 8, 2024

Sure, if the click is intended to focus the text input area and not move the slider, such a safeguard avoids some mishaps.

Another way of sliding the value that is present on the SpinBox widget is to hold down the mouse button on the arrows area then immediately slide up or down. Past a similar threshold, the mouse is grabbed and up-down movements adjust the value. Although there is a threshold before starting the slide, there is no "jump" in the initial value change corresponding to the distance traveled to the threshold. The idea of the value "jumping" proportionally to the threshold would be my main pain point for this threshold system, hence my previous remark.

  • We should make something unified on the whole editor instead of having some spinners sliding up-down while others sliding left-right IMHO.
  • Consider using a couple modifier keys during the slide operation to enlarge or reduce the change step scale. For example, if moving the mouse 1px along the slide axis changes the value by 1, holding Ctrl makes a larger change like 10, while holding Shift allows fine-tuning changes such as 0.1; such scale is to adapt to the context.
  • Live applied changes while the value is being slide-edited is crucial in some contexts. For example, a slider field is used to set the rotation of a Control node. I like to see the visual changes live as I move the mouse - as it is the case today on the stable version - because the adjustments are visual, I want to align the thing with something else in the scene - and need a live feedback loop rather than having to do some blind dichotomy attempts.

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

No branches or pull requests

3 participants