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

Refactor WindowOtherWheelInput to be safer and script window compatible #22060

Merged
merged 2 commits into from
May 25, 2024

Conversation

AaronVanGeffen
Copy link
Member

@AaronVanGeffen AaronVanGeffen commented May 18, 2024

We've been able to use the mouse scroll wheel to manipulate spinner widgets for years (#7627). However, the function that was added in to support this had not been updated to account for custom (script) windows.

This PR ports the algorithm from OpenLoco/OpenLoco#2019, which is both (memory) safer as well as more accurate, making it compatible with spinner widgets in custom (script) windows.

Fixes Basssiiie/OpenRCT2-RideVehicleEditor#111

@AaronVanGeffen AaronVanGeffen added this to the v0.4.12 milestone May 18, 2024
@AaronVanGeffen AaronVanGeffen added the changelog This issue/PR deserves a changelog entry. label May 18, 2024
@Basssiiie
Copy link
Member

Basssiiie commented May 20, 2024

I've had a test and ran into a few issues with both plugin spinners and vanilla spinners.

For plugin spinners:

  • It works perfect on the text/number part of the spinner, both up and down scroll, except for the fact that the scroll direction is inverted compared vanilla spinners:
Plugin Vanilla
Scroll up Decrement Increment
Scroll down Increment Decrement
  • On the [-] button, only scroll up works, not scroll down.
  • On the [+] button, neither scroll up or down works.
  • When scrolling nearby a dropdown, sometimes glitching occurs, or weird interactions with nearby dropdowns (see pictures below).

For vanilla spinners:

  • Scrolling on the text/number part works perfect, both up and down scroll.
  • On the [-] button, neither scroll up or down works.
  • On the [+] button, only scroll up works to decrement (instead of increment).

(For testing the vanilla spinner I used the park entrance price spinner.)

In this image I tried scrolling on the [+] marked with the red arrow, but it suddenly opened the dropdown below. The dropped down part even opens in the wrong place.
image

Scrolling this [+] causes a weird vertical bar to show, and sometimes also opens the dropdown.
image

(Note these dropdown spinners are still a separate dropdown and a separate spinner in full, just overlayed on top of each other.)

@AaronVanGeffen AaronVanGeffen marked this pull request as draft May 20, 2024 19:52
@AaronVanGeffen
Copy link
Member Author

@Basssiiie I believe I've addressed the issues with the two latest commits.

Due to how the FlexUI's DropdownSpinnerControl works, the label does not support scrolling, unfortunately. However, the [-][+] spinner buttons work as expected for me.

Adding scrollwheel support to dropdown widgets should be possible as well, though. That will require a new codepath, though, and is therefore best left to another PR.

Copy link
Member

@Basssiiie Basssiiie left a comment

Choose a reason for hiding this comment

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

I've tested it again and I have found no further issues. It all looks good to me now. Thanks!

And yeah don't worry, I don't expect the dropdowns to work here. It's a custom set up anyway and I'm already happy the [+]/[-] now work as expected. 😃

@AaronVanGeffen AaronVanGeffen force-pushed the scroll-spinners branch 2 times, most recently from 020c1bf to 10c97c6 Compare May 25, 2024 14:36
@AaronVanGeffen AaronVanGeffen merged commit 15837e7 into OpenRCT2:develop May 25, 2024
22 checks passed
@AaronVanGeffen AaronVanGeffen deleted the scroll-spinners branch May 25, 2024 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog This issue/PR deserves a changelog entry.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use mousewheel to increase/decrease values
3 participants