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.

Sign up for GitHub

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

Jump to bottom

Add slider menu item, configuring on-screen controls size #3021

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Add slider menu item, configuring on-screen controls size #3021

wants to merge 12 commits into from
+280 −178

Conversation

Vankata453
Copy link
Member

@Vankata453 Vankata453 commented Jul 15, 2024 •
edited
Loading

Adds the ItemSlider menu item. It allows selecting an integer value between a minimum and a maximum value. It can be used with the left mouse button, left/right arrow keys or scroll wheel. The slider step (interval between values when using the arrow keys or scroll wheel) is configurable from "Options" -> "Advanced".

The sound volume, music volume and flash intensity options now utilize a slider. The on-screen controls scale option was added back in "Options" -> "Controls" and utilizes a slider as well.

Supertux.Menu.Sliders-1.mp4

Additionally:

  • The on-screen controls toggle has also been re-added in "Options" -> "Controls".
  • The on-screen controls configuration values were changed to allow for broader scale customization.

Closes #3010.
Closes #3033.

@Vankata453
Add slider menu item, configuring on-screen controls size
09678db 
Adds the `ItemSlider` menu item. It allows selecting an integer value between a minimum and a maximum value. It can be used with the left mouse button and the left/right arrow keys.

The sound volume, music volume and flash intensity options now utilize a slider. The on-screen controls scale option was added back in "Options" -> "Controls" and utilizes a slider as well.

Additionally:

* The on-screen controls toggle has also been re-added in "Options" -> "Controls".
* The on-screen controls configuration values were changed to allow for broader scale customization.

Closes SuperTux#3010.
@Vankata453
Perform assertion only once [ci skip]
19584dd
MatusGuy
MatusGuy reviewed Jul 15, 2024
View reviewed changes
Copy link
Member

@MatusGuy MatusGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ideal for the volume sliders! Just make sure there's a way to specify the step.

@Vankata453
Copy link
Member Author

Just make sure there's a way to specify the step.

What step?

@MatusGuy
Copy link
Member

The step is the interval between values.
For example, if the step is 5, each time you press the right key, instead of increasing by 1 it increases by 5. And when using a cursor it snaps to the right place.

@Vankata453
Copy link
Member Author

Sure, will make an option for it.

@tobbi
Copy link
Member

tobbi commented Jul 15, 2024

I haven't tested it yet but... Many of the sliders in UI Frameworks have the option of increasing / decreasing when the user hovers over the slider and uses the mouse wheel. Any plans on implementing that?

@Vankata453
Copy link
Member Author

Will do, I forgot they had that.

@Vankata453
Copy link
Member Author

Added customizable slider steps and scroll wheel support.

@Thatstinkyguy
Copy link

Thatstinkyguy commented Jul 16, 2024 •
edited
Loading

@Vankata453 Awesome work! Will these features be added to the Android version?

@Vankata453
Copy link
Member Author

Will these features be added to the Android version?

After this is merged, they will.

@tobbi
Copy link
Member

tobbi commented Jul 16, 2024 

/home/runner/work/supertux/supertux/src/gui/item_slider.cpp:90:96: error: implicit conversion from 'int' to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        *m_value = static_cast<int>(((mouse_pos.x - m_slider_x) / SLIDER_WIDTH) * (m_max_value - m_min_value)) + m_min_value;
                                                                                ~  ~~~~~~~~~~~~^~~~~~~~~~~~~
/home/runner/work/supertux/supertux/src/gui/item_slider.cpp:114:96: error: implicit conversion from 'int' to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        *m_value = static_cast<int>(((mouse_pos.x - m_slider_x) / SLIDER_WIDTH) * (m_max_value - m_min_value)) + m_min_value;
                                                                                ~  ~~~~~~~~~~~~^~~~~~~~~~~~~
2 errors generated.

MatusGuy
MatusGuy reviewed Jul 30, 2024
View reviewed changes
Copy link
Member

@MatusGuy MatusGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will test this pretty soon.

src/control/mobile_controller.hpp Show resolved Hide resolved
src/supertux/gameconfig.cpp Outdated Show resolved Hide resolved
MatusGuy
MatusGuy requested changes Jul 30, 2024
View reviewed changes
Copy link
Member

@MatusGuy MatusGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't like the step option in the config menu. First time mobile port users will be extremely confused when they see the slider going from min to max instantly when using the left/right buttons because they don't know what a "slider step" is (and they're not supposed to).

When using the slider with the mouse on the volume controls, it lags the game. If you want I will investigate this.

I gotta say. I really like how the mouse can be used for precision and the arrows can be used for bringing up quick values. But I think the roles should be switched. When using the mouse, the value should be snapped to the defined step, and the arrow keys should be used for precision values, because if the mouse is somewhat defective, that "precision" is lost.

I'm excited for this!

@Vankata453 Vankata453 mentioned this pull request Aug 4, 2024
Open
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MatusGuy MatusGuy MatusGuy requested changes

Assignees
No one assigned
Labels
category:code involves:functionality status:needs-review Work needs to be reviewed by other people type:feature
Projects
None yet
Milestone
No milestone
4 participants
@Vankata453 @MatusGuy @tobbi @Thatstinkyguy