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

Exported int variables only display as sliders if their stepsize is set to something other that 1 #42809

Open
Arnklit opened this issue Oct 15, 2020 · 17 comments

Comments

@Arnklit
Copy link
Contributor

Arnklit commented Oct 15, 2020

Godot version:
3.2.3.stable

OS/device including version:
Windows 10 Pro v.2004 / Inspiron 7373 Core i7-8550U / Intel(R) UHD Graphics 620

Issue description:
Exported int variables don't show up as sliders. If you set the step size to something else that 1, they do. For some reason it works to set the step size to a float under 1 and then it will work as an int slider.
image

Steps to reproduce:
Open attached project to see issue

Minimal reproduction project:
export_int_issue.zip

@rsubtil
Copy link
Contributor

rsubtil commented Oct 17, 2020

Building upon this, why not simply add both options to those type of export'ed vars, like this?

dwdw
It gives both a fine-grained control with the arrows, and more wide-grained control with the slider.

@aterlamia
Copy link

If i look at the documentation it it only says that the range is limited only for the last example it also says it shows a handy slider.

So in my expectations the first one should not show a slider.

The second one is debatable as a setting of 1 for slider is the same as not setting a step, but that might need to be documented.

And for the last case with the float i guess it is cast to a int, do we need to throw an error there or document that a float will be cast to a int in the step in case of a int slider?

Or do we just want uniform functionality for all these cases?

@Calinou
Copy link
Member

Calinou commented Oct 17, 2020

Sliders are currently only displayed for floats, not integers. I think we should change this so integers also have sliders.

However, if we do that, we lose the "immediate" visual hint that some numbers are integers since they don't have an associated slider. I'm not sure if it's always worth losing that visual hint.

@aterlamia
Copy link

in the example project and screenshot above the exports on line 6 and 7 behave like integer sliders. Just tested that myself. In that case var 3 would be the error and var 4 just strange behavior if understand correctly?

But in that case i would say indeed keep sliders for floats only as only those need the ctrll to round / shift to be more precise functionality that the slider provides

@Arnklit
Copy link
Contributor Author

Arnklit commented Oct 17, 2020

@Calinou how about always showing a single decimal on floats even if they are whole numbers to indicate they are floats not ints? I know that would not be for this bug, but that might be a good proposal and then we could make ints have sliders consistently.

@aterlamia
Copy link

What does exactly need to be done here now?

If sliders should not show for integers none of the above should show a slider, am i correct?

Which does not work at this moment because in editor_spin_slider.cpp we first check

get_step() == 1

And if that is not true we check hide_slider, which is default set to false and not set to true for integers. So a slider is shown for all integer values with a step larger than 1

Does that need to be fixed or should i right away implement the changes as suggested?

An another question the example on line 7 should not be possible i think do we need a warning or an error there. Or is the functionality that it implicitly is cast to an integer how we want it?

@Bhu1-V
Copy link
Contributor

Bhu1-V commented Nov 6, 2020

Hello guys,
where can i find the logic for the export keyword.
I tried looking in godot/modules/gdscript/ path but found nothing.
image

Please Help me to find something about it.

@akien-mga
Copy link
Member

@pWNn1sher you got the path wrong:
https://github.com/godotengine/godot/search?q=export+path%3Amodules%2Fgdscript

@zak-grumbles
Copy link
Contributor

Interested in picking this issue up. Was there ever a consensus on the desired behavior for integers?

@jcommander
Copy link

However, if we do that, we lose the "immediate" visual hint that some numbers are integers since they don't have an associated slider. I'm not sure if it's always worth losing that visual hint.

Agreed, but wouldn't it be possible to add an optional export hint for @export_range with a step = 1.0 so you could give both floats and ints the default float slider representation by choice.
This would preserve visual identity by default and give a readable syntax if you decide a slider fits your variable better.

Example:
@export_range(0, 10, 1, "decimal_style") var x: int
or
@export_range(0, 10, 1.0, "decimal_style") var y: float

@ulvido

This comment was marked as off-topic.

@zell2002
Copy link

zell2002 commented Apr 6, 2024

Hey all, whats going on with this then?
It's 2024 and Godot 4 is excellent, but why can't I have a Slider for an increment of 1?
I think I agree with the above: if a min/max has been set, it should display?

More than anything, clicking the Up/Down button repeatedly to change a number is tedious. Cant seem to click and hold?

@Calinou
Copy link
Member

Calinou commented Apr 8, 2024

More than anything, clicking the Up/Down button repeatedly to change a number is tedious. Cant seem to click and hold?

EditorSpinSliders with the slider hidden in the editor can be clicked and held, but you need to drag the mouse directly on the number value. In other words, you must start the mouse dragging operation on the number directly, not the arrows.

In projects, this also works on SpinBox nodes, although you must start dragging from the arrow icons and dragging must be performed vertically instead of horizontally. The SpinBox node is also missing support for the Ctrl and Shift modifiers which are used when dragging.

@fauxhaus
Copy link

as a quick+dirty workaround, casting the value out as an int (with a non-1 range step) works in my 4.3.dev5 (and presumably earlier):

@export_range(0.0, 64.0, 0.1) var height: int = 0

this creates an integer slider that behaves as anticipated:
image

now, this shouldn't be the notation for integer sliders IMO, but this seems useful for anyone who wants them in their project today

@FishOfTheNorthStar
Copy link

Anything with ranges specified that can show a slider should show a slider in my opinion. Perhaps there could be an optional property/decorator to the hint_range string, like: 'no_slider', to retain the int format in place now for those that prefer it.

"Visual queue of integer versus float", as mentioned above, may legitimately be of some benefit, but it's nothing compared to 'the docs say this command will add a slider, I added that command and it will not show a slider'. Either the docs need clarified on this or the unexpected behavior needs corrected.

@hsandt
Copy link
Contributor

hsandt commented Apr 26, 2024

The thing is that it also behaves the same way for floats (step != 1.0 will show the slider, step = 1.0 will not), I just got this for a float angle and it was really confusing. I thought I had mistyped something or broken the inspector, as the other similar field just below had the slider. In addition, I found about the annotation "hide_slider", so I assumed the slider should be displayed when not using this annotation (that should also answer @FishOfTheNorthStar 's need for a "no_slider" hint).

(btw we could rename the issue title to include "float", I almost missed that issue because I thought it was unrelated)

In this case I added the @export_range + annotations precisely because I wanted a slider to change the angle smoothly (while looking at the result in the scene thx to some custom tool code showing the line corresponding to the angle). So I'd rather have it work out-of-the-box, and otherwise have an extra annotation to force show_slider, like the suggested decimal_style... But since hide_slider already exists, it makes more sense to me to just use that.

So that's for floats. I don't have a strong opinion on int as I haven't stumbled on the issue yet. But I do know that in Unity, I like have that little slider for integer values that are within a specific range like difficulty level between 1 & 10.

And why not have both arrows and sliders if people really fancy them depending on the situation?

EDIT: for now I'll try the workaround of setting step to 2.0... I don't need super precise angles anyway.

@FishOfTheNorthStar
Copy link

FishOfTheNorthStar commented Apr 27, 2024

To clarify, I personally have no need for a 'no slider' hint. That was said for the benefit of other people who apparently don't want the slider to show up. I'm sure they have their reasons.

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