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

Controllers #3378

Merged
merged 31 commits into from
Nov 20, 2023
Merged

Controllers #3378

merged 31 commits into from
Nov 20, 2023

Conversation

briaguya-ai
Copy link
Contributor

@briaguya-ai briaguya-ai commented Nov 10, 2023

What this does

  • Finally lets us close Allow multiple controller inputs to do the same action #27
  • Lets us build our own SoH controller settings window
    • It's mostly identical to the one in LUS right now. My goal for the one in LUS is to be a "template" for ports to build their own with. If we make improvements on the SoH side that make sense to bring into LUS I'll happily handle that side of things.
  • Bumps us to latest LUS main as of now (Kenix3/libultraship@5387abf)

What this doesn't do

  • Fully consolidate the 2 controller settings windows.
    • There's still a bit of stuff to figure out for that UX wise and I didn't want to make those calls alone.

What I'm hoping for in reviews

  • Basically just a bit of quick playtesting to verify things aren't borked. In my testing things have been fine. I'd like this to be the new base to build what we want controller configuration to be off of, not the be all end all of the controller rework in SoH.

Build Artifacts

@briaguya-ai briaguya-ai marked this pull request as ready for review November 10, 2023 05:25
@briaguya-ai
Copy link
Contributor Author

briaguya-ai commented Nov 10, 2023

//todo

  • padding
  • keyboard key multi mapping is borked?
  • analog stick options expand by default (also make it look better collapsed - any advice on this would be great!)
  • rename window to be different than it was before so imgui.ini isn't an issue (handled this by using a hidden id)
  • only show link and ivan tabs when debug is disabled, show all 4 when debug enabled (and show all buttons - not just ivan - when debug)
  • slider +/- buttons
  • clear all etc. buttons not super visible

@briaguya-ai
Copy link
Contributor Author

briaguya-ai commented Nov 11, 2023

edit: I decided to just default them to open for now, this will still be a useful comment when we start adding more analog stick options

@aMannus re:

analog stick options expand by default (also make it look better collapsed - any advice on this would be great!)

when messing around with this I found a few issues:

the issues

adding

ImGui::SetNextItemOpen(true, ImGuiCond_FirstUseEver);

or

ImGui::SetNextItemOpen(true, ImGuiCond_Once);

before

if (ImGui::TreeNode(StringHelper::Sprintf("Analog Stick Options##%d", id).c_str())) {

will make it default to open, but it will always default to open.

the expanded/collapsed state isn't saved in imgui.ini. Closing and reopening the window within the same session is fine, but after exiting and reopening the application it will go back to defaults.

after looking into this a bit more, it seems to also be a problem with all the CollapsingHeader sections too, such as

if (ImGui::CollapsingHeader("Buttons", NULL, ImGuiTreeNodeFlags_DefaultOpen)) {

ideas/paths forward

my gut feeling is that "i told you to go away" is a more frustrating user experience than "i thought you were open" when it comes to those options, but i could be convinced otherwise.

taking that into account i see a few options:

  • manually save the expanded/collapsed state of every treenode/collapsingheader
    • if we did this defaulting to open would feel fine to me, closing it will keep it closed
    • this feels like it would involve adding a ton of UI cvars, which doesn't feel ideal implementation-wise
  • default to open, have it reopen on people when they restart the application, tell them that's just how it is
  • continue to default to close, make it so it is less hidden when closed

I'm leaning towards the "continue to default to close" option for a couple reasons

  • once we add stuff like sensitivity and ESS range and other stuff the options section will get bigger, having it default to open could be overwhelming for people that just want to map a stick
  • i feel like we should figure out why imgui doesn't let us persist expanded/collapsed instead of trying to work around it with cvars, that may take a bit of time/effort and i'd rather do that than build out a ton of cvars with the hope they'll be temporary

any thoughts/suggestions would be awesome!

Copy link
Contributor

@garrettjoecox garrettjoecox left a comment

Choose a reason for hiding this comment

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

Absolutely stellar job. It looks great, functions great, and it's intuitive.

  • Should we have a set defaults option for the keyboard?
  • Can we have a "Test Rumble" button?
  • Tooltip explaining axis threshhold
  • The position of some elements seems to not take scrolling X into account correctly (video attached)
  • +/- buttons on "notch snap angle" and "Large Motor Intensity" don't do anything (maybe lacking a unique ImGui ID?)

This is maybe a larger one / design flaw: When I plug my stadia controller, two virtual controllers are added, I'm not really sure if it matters which of these "instances" I'm binding inputs to, but I don't really have a way to select which one I want to bind to. I can get both virtual controller's buttons to bind if I just keep re-trying, but it seems to just pick one of them at random each attempt. Again I'm not sure this is really an issue, unless someone has a use-case in which they desire to bind to one over the other.

Screen.Recording.2023-11-12.at.10.44.56.AM.mov

@garrettjoecox
Copy link
Contributor

Verified all my feedback has been accounted for or fixed 👍

@briaguya-ai briaguya-ai mentioned this pull request Nov 15, 2023
@briaguya-ai briaguya-ai merged commit 76e90c0 into HarbourMasters:develop Nov 20, 2023
A-Green-Spoon pushed a commit to A-Green-Spoon/Shipwright that referenced this pull request Nov 30, 2023
* lay some groundwork

* use custom window (which is currently identical to the LUS window)

* start making it shippy

* start moving stuff out of gamecontroleditor

* clean up shouldrumble

* include the other way

* wii u

* latest lus main

* notch snap angle buttons

* buttons on all the sliders

* just use a hidden id

* handle debug for port 2 and rename tabs so everything fits

* button line buttons look better

* padding fixed

* clang format

* bump to latest LUS main

* big buttons

* just default the analog stick options to open for now

* fix wii u build

* bonus: make it all scale-aware

* clang format

* fix horizontal scrolling

* fix all +/- buttons

* keyboard set defaults

* axis threshold helper text

* bonus: test rumble button

* clang format

* fix otrexporter submodule

* bump to latest lus main
@briaguya-ai briaguya-ai deleted the controllers branch March 3, 2024 01:56
jamieklassen pushed a commit to jamieklassen/Shipwright that referenced this pull request Mar 10, 2024
* lay some groundwork

* use custom window (which is currently identical to the LUS window)

* start making it shippy

* start moving stuff out of gamecontroleditor

* clean up shouldrumble

* include the other way

* wii u

* latest lus main

* notch snap angle buttons

* buttons on all the sliders

* just use a hidden id

* handle debug for port 2 and rename tabs so everything fits

* button line buttons look better

* padding fixed

* clang format

* bump to latest LUS main

* big buttons

* just default the analog stick options to open for now

* fix wii u build

* bonus: make it all scale-aware

* clang format

* fix horizontal scrolling

* fix all +/- buttons

* keyboard set defaults

* axis threshold helper text

* bonus: test rumble button

* clang format

* fix otrexporter submodule

* bump to latest lus main
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.

Allow multiple controller inputs to do the same action
2 participants