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

Make EditorSpinSlider display a slider for floats with a step of 1.0 #95169

Merged

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Aug 5, 2024

Integers still don't display a slider (and use up/down arrows instead), so that they can be quickly distinguished from floats in the inspector. The original issue mentioned that integers could be made to look like floats in the inspector, but it means we'd be losing this particular UX affordance, so I didn't go as far as making all integers have a slider for now.

However, this now makes floats with a step of 1.0 look different from integers in the inspector. This complements #47502 in that sense.

Preview

@export_range(0, 10, 1) var integer_step_1 := 1
@export_range(0.0, 10.0, 1.0) var float_step_1 := 1.0
@export_range(0.0, 10.0, 0.1) var float_step_0_1 := 1.0

Before

Screenshot_20240805_190042

After

Screenshot_20240805_191712

@Calinou Calinou added bug topic:editor usability cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Aug 5, 2024
@Calinou Calinou added this to the 4.4 milestone Aug 5, 2024
@Calinou Calinou requested review from a team as code owners August 5, 2024 17:24
@Calinou Calinou force-pushed the editorspinslider-float-step-1-slider branch 2 times, most recently from be04816 to 860756c Compare August 5, 2024 17:26
@Calinou Calinou force-pushed the editorspinslider-float-step-1-slider branch 2 times, most recently from fb6c1d0 to 456bc37 Compare August 20, 2024 22:55
akien-mga
akien-mga previously approved these changes Aug 26, 2024
@akien-mga
Copy link
Member

Actually I read a bit too fast, I see now that this is changing the current behavior where exported ints with a step != 1 actually show a slider. This does warrant some more discussion IMO to make sure this is the UX users will expect.

@akien-mga akien-mga dismissed their stale review August 26, 2024 21:15

Need more opinions.

@timothyqiu
Copy link
Member

To be honest, until I saw this PR, I thought spinners were for integers and sliders were for floats in the inspector 🤣

@akien-mga
Copy link
Member

akien-mga commented Sep 5, 2024

I feel this new editing_integer property sounds like what one would expect hide_slider to control - but hide_slider controls both the slider (float) and the SpinBox arrows (int).

I wonder if we shouldn't instead deprecate hide_slider, and introduce a new enum property to choose what kind of control should be shown, like edit_widget with options Slider, Arrows, None.

Then EditorPropertyFloat and other float editors can use edit_widget == EDIT_WIDGET_SLIDER, while EditorPropertyInteger would use edit_widget == EDIT_WIDGET_INTEGER.

And the hide_slider property hint would be deprecated and replaced by no_edit_widget I guess?

Better naming suggestions welcome, we don't really use "widget" in Godot speak currently.

This would also add the possibility to use a slider with integers if desired, by making a custom EditorProperty for ints that sets EDIT_WIDGET_SLIDER. There could possibly also be property hints for this (slider, arrows) if there's actual demand that warrants adding more hints.

@KoBeWi
Copy link
Member

KoBeWi commented Sep 6, 2024

introduce a new enum property to choose what kind of control should be shown

You mean in EditorSpinSlider? It would be inconvenient to use without a dedicated hint.

@akien-mga
Copy link
Member

introduce a new enum property to choose what kind of control should be shown

You mean in EditorSpinSlider? It would be inconvenient to use without a dedicated hint.

I suggest we could add dedicated hints for it, but realistically, this is mostly an implementation detail and in the vast majority of cases users wouldn't need to think about it or specify it manually. int properties would use the SpinBox and float properties would use the slider automatically.

We can drop exposing the functionality to users for now and just keep it internally (thus no hint strings needed). My main concern with this PR is how it exposes an editor implementation detail as a public property.

Integers still don't display a slider (and use up/down arrows instead),
so that they can be quickly distinguished from floats in the inspector.

However, this now makes floats with a step of 1.0 look different
from integers in the inspector.
@Calinou Calinou force-pushed the editorspinslider-float-step-1-slider branch from 456bc37 to 57700b0 Compare January 22, 2025 22:05
@Repiteo Repiteo merged commit 4f827e6 into godotengine:master Jan 31, 2025
19 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jan 31, 2025

Thanks!

@Calinou Calinou deleted the editorspinslider-float-step-1-slider branch February 6, 2025 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release topic:editor usability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants