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

EditorSpinSlider: Change input via ´down´ or ´up´ arrow key #81272

Closed
Alex2782 opened this issue Sep 3, 2023 · 16 comments · Fixed by #81278
Closed

EditorSpinSlider: Change input via ´down´ or ´up´ arrow key #81272

Alex2782 opened this issue Sep 3, 2023 · 16 comments · Fixed by #81278

Comments

@Alex2782
Copy link
Contributor

Alex2782 commented Sep 3, 2023

Godot version

v4.1.1.stable.official [bd6af8e] and v4.2.dev4.official [549fcce]

System information

macOS 13.4.1 - Vulkan (Forward+) - integrated Apple M1 - Apple M1 (8 Threads)

Issue description

If the input is set to -1 and you want to change the value back to 0 by pressing the arrow key up, the value is increased by 0.001 only. This looks like a rounding error (see video below). In version 3.5.2 it was still ok.

Steps to reproduce

  1. create Node2D
  2. Inspector -> Transform -> x - Position
  3. enter -1
  4. press the arrow keys up several times, then down again.
Screen-2023-09-03-160653.mp4

Minimal reproduction project

N/A

@AThousandShips
Copy link
Member

AThousandShips commented Sep 3, 2023

Can't replicate, and the video doesn't show the same behavior as your description

Are you sure you don't press Alt while doing this? This would cause a change of 0.1, though it does seem a bit sporadic

@Alex2782
Copy link
Contributor Author

Alex2782 commented Sep 3, 2023

now? new video and Steps to reproduce

@AThousandShips
Copy link
Member

I cannot replicate this no, not without pressing other keys, like Alt

@Alex2782
Copy link
Contributor Author

Alex2782 commented Sep 3, 2023

no, only up and down, on my mac keyboard

@AThousandShips
Copy link
Member

Looks like you are accidentally dragging when selecting the edit, which modifies the value

Can you confirm this by having the mouse outside the area, and select it with tab instead of clicking on it?

@Alex2782
Copy link
Contributor Author

Alex2782 commented Sep 3, 2023

  • new video, now with keyboard events (bottom left)
  • mouse outside the area, and select it with tab
  • alt (option) changes the value by 0.1, but from -1 and alt + arrow up also from time to time by 0.001
Screen-2023-09-03-162152.mp4

@AThousandShips
Copy link
Member

Probably something either hardware specific on your end or Mac specific, as I cannot replicate this on Windows

@Alex2782
Copy link
Contributor Author

Alex2782 commented Sep 3, 2023

I'll also check it later under WindowsOS.

I wanted to try to catch mouse wheel events and change the values. I could not yet find the logic for arrow keys (up / down). Does anyone know in which file? Via debugging I found out that the variable text is only adjusted much later after _process. Not yet in the function LineEdit::gui_input.

void LineEdit::gui_input(const Ref<InputEvent> &p_event) {

@AThousandShips
Copy link
Member

It's not a LineEdit it's an EditorSpinSlider under editor/gui

@AThousandShips AThousandShips changed the title Bug? LineEdit: Change input via ´down´ or ´up´ arrow key Bug? EditorSpinSlider: Change input via ´down´ or ´up´ arrow key Sep 3, 2023
@Alex2782
Copy link
Contributor Author

Alex2782 commented Sep 3, 2023

partially reproduced under Windows v4.1.1.stable

  1. leave input at 0
  2. alt + key down reduced by -0.001
  3. alt + key down reduced by -0.001 to -0.002
  4. alt + key down reduced by -0.1 to -0.102
  5. alt + key down reduced by -0.1 to -0.202
  6. alt + key down reduced by -0.001 to -0.203

that looks strange, or is it intentional?

@AThousandShips
Copy link
Member

AThousandShips commented Sep 3, 2023

That part is probably a floating point issue and unsure if it can be resolved, possibly related to the way it parses text, but could just be down to how precision is handled

But that should be a separate bug report IMO as the original bug isn't reproducible on Windows and therefore unrelated

@Alex2782
Copy link
Contributor Author

Alex2782 commented Sep 3, 2023

ok thx, I will check it more closely. I think macOS also has a "floating point problem", even with -1

@Alex2782
Copy link
Contributor Author

Alex2782 commented Sep 3, 2023

I found out that the CLAMP condition is true even though the value is within min / max limit.
When I comment out the lines, I no longer see the "floating point problem". I think this is a real error set_value(last_value + real_step) and set_value(last_value - real_step) are executed unnecessarily. With double, the floating point problem should only occur after the 10th decimal place and not after 0.001.

Screen-2023-09-03-180444.mp4

@AThousandShips
Copy link
Member

AThousandShips commented Sep 3, 2023

The floating point problem I'm talking about isn't related to floating point rounding, but to representing as text, as you can see it only represents to a fixed number of digits

Feel free to open a PR changing this if you feel it's a good solution to the problem

@Alex2782
Copy link
Contributor Author

Alex2782 commented Sep 3, 2023

I may already have a solution.

more questions:
get_min() returns -99999 and get_max() +99999

but is it possible to enter larger values, should I still execute CLAMP after the normal input or remove it from key up / key down?
Note: if the values are too large, the input can no longer be changed via "key up" / "key down".
image

@AThousandShips
Copy link
Member

AThousandShips commented Sep 3, 2023

It is based on the range provided, for Node2D the range is -99999 to 99999, it depends, other properties have different ranges

it allows wider ranges in this case, it just is used for handling the normal range for the sliders etc.

@akien-mga akien-mga added this to the 4.2 milestone Sep 25, 2023
@akien-mga akien-mga changed the title Bug? EditorSpinSlider: Change input via ´down´ or ´up´ arrow key EditorSpinSlider: Change input via ´down´ or ´up´ arrow key Sep 26, 2023
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.

3 participants