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

Improve mouse capture handling in menus #520

Merged
merged 20 commits into from
Nov 10, 2022

Conversation

ASpoonPlaysGames
Copy link
Contributor

This both allows a menu to have multiple mouse capture callbacks, as well as passing the menu element that captured the mouse input as a parameter, allowing for multiple scrollbars per menu

Copy link
Contributor

@uniboi uniboi left a comment

Choose a reason for hiding this comment

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

yes please holy shit

@ASpoonPlaysGames
Copy link
Contributor Author

This actually i think would break backwards compatibility for mods that use scrollbars, In theory I could solve that, but it wouldn't be nice code

@uniboi
Copy link
Contributor

uniboi commented Oct 17, 2022

works in testing btw
I don't think there are a lot of mods that use scrollbars tbh.

Copy link
Member

@F1F7Y F1F7Y left a comment

Choose a reason for hiding this comment

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

As someone who wrote the original system I approve these changes

@F1F7Y
Copy link
Member

F1F7Y commented Oct 17, 2022

Also regarding breaking mods, as far as I know mod settings is the only public mod which uses this system and it's getting merged ( right ? ) anyways so it shouldn't matter much

@uniboi
Copy link
Contributor

uniboi commented Oct 17, 2022

@GeckoEidechse this pr cured my dogs cancer can we set a new merge record? 😳😳😳

@uniboi
Copy link
Contributor

uniboi commented Oct 18, 2022

uhh all mods that use a custom scrollbar like Better Server Browser (ok grandpa), Better Modlist (deprecated by thunderstore guys), Modsettings and the LAN browser mod from that one polish guy will compile error unless updated.
For modders it's an easy fix, they just need to change their movementcapture callback function from void functionref(int,int) to void functionref(var,int,int) and that's it.

btw gecko can you please tell people in the 1.10 announcement to uninstall better modlist since it's merged thank

@BobTheBob9
Copy link
Member

this is great, but should probably be moved into its own file instead of sh_menu_models if we're api-ing it

@ASpoonPlaysGames
Copy link
Contributor Author

good point, will do

@ASpoonPlaysGames
Copy link
Contributor Author

this is great, but should probably be moved into its own file instead of sh_menu_models if we're api-ing it

Finally got around to doing this

@uniboi
Copy link
Contributor

uniboi commented Oct 29, 2022

as of now, mods that are using mouse movement captures (only mods that use the custom scrollbar afaik) will break since they register the callback for the menu instead of the capture element. However they will not compile error but scrollbars will just stop working.
We could make a legacy kind of function that doesn't break old mods, but I don't think that's worth it since there are barely any mods that use the mousemovement capture.

@ASpoonPlaysGames
Copy link
Contributor Author

This should now preserve backwards compatibility, you can now pass either a capturePanel or a menu to AddMouseMovementCaptureHandler

I'm keeping the changes to other files within NorthstarMods itself as we should prefer passing the capturePanel for use with multiple scrollbars on the same menu

@ASpoonPlaysGames
Copy link
Contributor Author

Aight this is I think in it's final state, unless anyone has any changes they want made.

From my own personal testing, backwards compatibility is preserved

@uniboi
Copy link
Contributor

uniboi commented Nov 7, 2022

what's blocking this?

Copy link
Member

@BobTheBob9 BobTheBob9 left a comment

Choose a reason for hiding this comment

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

looks good, will probably double check through everything and merge shortly, though i imagine it's fine

@uniboi
Copy link
Contributor

uniboi commented Nov 8, 2022

Titanfall_2_2022-11-08_23-15-57.mp4

@ASpoonPlaysGames
Copy link
Contributor Author

(just so it's known, the scrolling boxes being weird is not my fault, they are simply unfinished)

@BobTheBob9 BobTheBob9 merged commit f5e4a7b into R2Northstar:main Nov 10, 2022
JMM889901 pushed a commit to JMM889901/NorthstarMods that referenced this pull request Aug 12, 2024
* rework mouse capture handling

* small improvement to comment

* move to new script file

* refactor to use only the capturePanel

* refactor part 2

* github please commit everything

* remove non-implemented global function

* cleanup

* rename variable

* formatting and a comment

* run callbacks for menus

* update comment accordingly

* small formatting change

* slight refactor to avoid duplicate code

* improve comment

* pass correct parameters

* newline at end of file :)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants