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

Any pad can control the menu #13173

Merged
merged 4 commits into from Nov 3, 2021
Merged

Any pad can control the menu #13173

merged 4 commits into from Nov 3, 2021

Conversation

gblues
Copy link
Contributor

@gblues gblues commented Oct 31, 2021

Description

== DETAILS
I am not sure I've quite got it so that any pad can open the
menu, but I do have it so any pad can control it.

  • split out the input processing into a separate method
  • track down and squish some hairy bugs that boiled down to
    bad pointer math
  • verified that this change did not impact actual cores

Reviewers

@twinaphex

== DETAILS
I am not sure I've quite got it so that any pad can *open* the
menu, but I do have it so any pad can control it.

- split out the input processing into a separate method
- track down and squish some hairy bugs that boiled down to
  bad pointer math
- it looks like `menu_driver.c` has a mix of line endings, so I
  ran it through `dos2unix` so it has consistent line endings
  again.
- verified that this change did not impact actual cores
@jdgleaver
Copy link
Contributor

jdgleaver commented Nov 2, 2021

@gblues Thanks - this is an often requested feature!

There are a few issues, though:

  • This must be optional - i.e. users must be able to enforce menu control by player 1 only
  • In collect_system_input() you loop over MAX_USERS ports. This is needlessly inefficient - you only need to loop over all enabled ports
  • In collect_system_input(), the keyboard and AI service handling should not go inside the loop. Keyboard input is always read from port 0, and the AI code already loops over MAX_USERS
  • The merge_input_bits() function is... just wrong...

I have prepared a small patch that fixes these issues:
gblues-menu-input-v2.patch.zip
(EDIT: I just noticed a minor inefficiency in the original patch I uploaded - the v2 patch addresses this)

Please apply this on-top of your PR and check that everything still works :)

Also, feel free to set DEFAULT_ALL_USERS_CONTROL_MENU to true for your target platform (it's the WiiU, right?)

Final note: With this PR, the toggle menu gamepad hotkey will only work for player 1, because hotkeys are handled differently and it would need a rewrite... But any set Menu Toggle Controller Combo will work fine for all users, so I think that's acceptable.

Many thanks to @jdgleaver for providing these optimizations.
@inactive123 inactive123 merged commit 1ef78d3 into libretro:master Nov 3, 2021
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

3 participants