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

Fix: volume slider behavior in music gui #7209

Closed
wants to merge 1 commit into from

Conversation

nikolas
Copy link
Member

@nikolas nikolas commented Feb 10, 2019

In the music gui, the volume sliders don't respond to mouse drags
that extend outside of their small area.

This changes the event code to give them more typical dragging behavior
that's easier to work with.

@@ -26,6 +26,7 @@
#include "string_func.h"
#include "settings_type.h"
#include "settings_gui.h"
#include "tilehighlight_func.h"
Copy link
Contributor

@glx22 glx22 Feb 10, 2019

Choose a reason for hiding this comment

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

It's strange to require tilehighlight_func when not working with tiles at all, but I know it's needed for drag stuff (used in other GUI too)

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree -_- this is required to use SetObjectToPlaceWnd(). depot_gui.cpp uses this function in a similar way, to allow for dragging vehicles to be sold in the depot window.

Maybe the window system's dragging events were only anticipated to be used with tiles? I think it may make sense to move SetObjectToPlaceWnd() out of tilehighlight_func.h, as it's useful for dragging events inside windows that don't work with tiles at all.

src/music_gui.cpp Outdated Show resolved Hide resolved
@PeterN
Copy link
Member

PeterN commented Feb 10, 2019

In master, moving the mouse away from the widget stops all further response, and you need to click and drag again to change volume.

As it stands, this PR makes the sliders continue to respond if you move the mouse away from the widget and then back.

I think it would be better if the sliders continued to respond whilst the cursor is off the widget, until dragging is stopped.

@nikolas
Copy link
Member Author

nikolas commented Feb 10, 2019

I've updated this PR to behave in the way you described, and yeah, it is much better.

src/music_gui.cpp Outdated Show resolved Hide resolved
In the music gui, the volume sliders don't respond to mouse drags
that extend outside of their small area.

This changes the event code to give them more typical dragging behavior
that's easier to work with.
@PeterN
Copy link
Member

PeterN commented Feb 12, 2019

This works but I'd like to look at how scrollbars achieve the same result.

@nikolas
Copy link
Member Author

nikolas commented Feb 12, 2019

Good point.. It looks like scrollbar dragging is handled in window.cpp's MouseLoop(), with HandleScrollbarScrolling: https://github.com/OpenTTD/OpenTTD/blob/master/src/window.cpp#L2835

It might be appropriate to add a HandleSliderScrolling() handler here, creating some consistency with any future sliders that are added to the window system.

Though, that may add some complexity, as we'll still want to do the actual volume-setting in music_gui.cpp, not window.cpp...

@PeterN
Copy link
Member

PeterN commented Feb 14, 2019

#7227 extends scrollbar handling to allow generic widget dragging without needing to use bits of tile highlight functions.

@nikolas
Copy link
Member Author

nikolas commented Feb 14, 2019

Great, that's a nicer solution, and the volume sliders are working well for me on that branch.

@nikolas nikolas closed this Feb 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants