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

feat: allow configuring menu toggle button #181

Merged
merged 3 commits into from
May 26, 2024

Conversation

briaguya-ai
Copy link
Contributor

@briaguya-ai briaguya-ai commented May 18, 2024

what this does

what this doesn't do

  • anything that lets users configure what keyboard key is used to toggle the menu

@Mr-Wiseguy
Copy link
Member

Mr-Wiseguy commented May 24, 2024

I think this PR is in a good state, if you change it to not be a draft PR I'll try to get some testing done and would be happy to merge it if that goes well. I didn't see anything in the PR to hide the input in the keyboard mapping UI, but I can help take care of that if it's not already the case.

@briaguya-ai briaguya-ai marked this pull request as ready for review May 24, 2024 18:05
@Mr-Wiseguy
Copy link
Member

Looks like CI failed due to needing a rebase, would you be able to rebase the PR on top of the current dev branch?

@briaguya-ai
Copy link
Contributor Author

I didn't see anything in the PR to hide the input in the keyboard mapping UI

yeah, i hadn't looked into that because i wasn't sure if that was a desired solution (or if actually implementing keyboard key configuration for toggle would be a similar lift)

@Mr-Wiseguy
Copy link
Member

I think it'd make sense to leave escape immutable as the menu key, it prevents you from ever getting locked into a binding since it also acts as the button to cancel an input binding. I don't think not being able to bind escape is a dealbreaker for anyone.

@briaguya-ai briaguya-ai changed the title proof of concept: allow configuring menu toggle button feat: allow configuring menu toggle button May 25, 2024
@briaguya-ai
Copy link
Contributor Author

i implemented logic to hide the keyboard binding by using a data-if in controls.rml. this felt like the least invasive way to do it, but it means the binding is still defined, just hidden.

@Mr-Wiseguy
Copy link
Member

That's a good solution. I did notice an issue if you have an existing controls.json file (as in you've played on a previous version), when you run this version the menu button is unbound. The fix is pretty straightforward, change the continue in load_input_device_from_json to load the default binding instead. I'd pass the default bindings for the current device in as an extra argument to that function and make some sort of switch case to get the default binding for a given GameInput.

@briaguya-ai
Copy link
Contributor Author

i updated load_input_device_from_json to add defaults. i needed a way to get from a GameInput enum value to the member on the DefaultN64Mappings struct so i added get_default_mapping_for_input

@Mr-Wiseguy Mr-Wiseguy merged commit d4898f2 into Zelda64Recomp:dev May 26, 2024
5 checks passed
@Mr-Wiseguy
Copy link
Member

Thanks for the contribution, this is a great feature to have!

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

2 participants