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

Merge controller menus #3860

Merged

Conversation

Pepe20129
Copy link
Contributor

@Pepe20129 Pepe20129 commented Jan 15, 2024

Merge the Additional Controller Options menu into the Controller Mapping menu.
It's worth considering renaming Controller Mapping to Controller Options.
I made some small changes to the contents of the menu to bring it more in line with the rest of the UI (change the question mark in a box in the right for a tooltip for example).

Build Artifacts

@Malkierian
Copy link
Contributor

From the logs for Mac:
/Users/runner/work/Shipwright/Shipwright/soh/soh/Enhancements/controls/SohInputEditorWindow.h:64:10: error: no template named 'list' in namespace 'std'
Probably need to add an #include <list> or something to SohInputEditorWindow.h. Windows might find it automagically, but I'm guessing the other platforms don't.

Copy link
Contributor

@leggettc18 leggettc18 left a comment

Choose a reason for hiding this comment

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

I'm in favor for sure. I kinda always felt like it was weird that these were separate. In fact I think the only reason they were ever separate was because the original controller menu was built in to LUS and these were OoT specific, but now the actual input editor menu lives in SoH so it makes sense to merge them now.

@Malkierian
Copy link
Contributor

The merging isn't really the issue, it's whether everything that was in Additional Controller Options really belongs in Controller Config.

@Malkierian
Copy link
Contributor

OK, having looked at the combined window, here are my opinions: Dpad and Misc Controls settings definitely should be moved to enhancements. Ocarina controls is kind of a mixed bag, the button assignment probably should stay in controller config, but I'm not as sure about the "play with" options for it. I would lean toward enhancements for those, though.

Camera controls definitely should stay in Controller Config, except for "Disable auto-centering" in First-person section, I think that should be in Enhancements.

@Malkierian
Copy link
Contributor

All that being said, I seem to remember someone saying they were working on a menu overhaul, and if that's the case, I would be OK with merging this as is and save the location changes here for that, as long as it was handled before the next major release.

@Pepe20129
Copy link
Contributor Author

I'm not sure if the menu overhaul will be done soon, and I agree that some settings should be in the enhancements but I do think that having both menus in one is better than having them apart and I wouldn't mind if the overhaul is not done by the next release and these things remain here.
If the overhaul hasn't been started yet, I think it'd be a good idea to move them now but if it already started I think it'd make more sense to move them there to avoid merge conflicts.

@briaguya-ai briaguya-ai force-pushed the merge_controller_menus branch from 4f8160a to 49d1312 Compare February 2, 2024 02:39
garrettjoecox and others added 2 commits February 1, 2024 20:50
Co-authored-by: briaguya <70942617+briaguya-ai@users.noreply.github.com>
@garrettjoecox garrettjoecox merged commit 961b262 into HarbourMasters:develop Feb 2, 2024
8 checks passed
@Pepe20129 Pepe20129 deleted the merge_controller_menus branch February 2, 2024 20:13
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.

5 participants