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

Add Controller Settings to menu #459

Merged
merged 3 commits into from
Jul 31, 2021
Merged

Add Controller Settings to menu #459

merged 3 commits into from
Jul 31, 2021

Conversation

KrahJohlito
Copy link
Member

Pull Request checklist

Note: these are not necessarily requirements

  • I reformatted the code with clang-format
  • I checked to make sure my submission worked
  • I am the author of submission or have permission from the original author
  • Requires update of the PS2SDK
  • Requires update of the gsKit
  • Others (please specify below)

Pull Request description

Solves #401
Moved a couple pad specific settings to 'Controller Settings' and added a global option for pademu in there so people can pair their devices while using USB for games.. Seeing as pademu can be used in OPL GUI I have no issue having it here but I left per game also because I'm not sure if people disable it per game for games that don't work? I don't use pademu so idk..

Unlike other menus we need to reload the cfg upon entry since it can be changed in multiple locations.. so if changes are made, not saved and then re-enter the pademu menu the cfg will be reloaded and changes wiped..so make sure to save changes if you wanna keep em..

This would be a good place for PADEMU also
@KrahJohlito KrahJohlito requested a review from uyjulian July 4, 2021 00:03
Copy link
Member

@uyjulian uyjulian 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 to me, code-wise

@KrahJohlito
Copy link
Member Author

I'll just rebase the codacy commit

@J013k
Copy link
Contributor

J013k commented Jul 4, 2021

Finally we will have Controller Settings at the main menu: https://i.postimg.cc/RhtM16Yd/hurra.gif
https://i.postimg.cc/v8LYcR58/menu-opl-1.png

For now I have two problems.

  1. The first problem has been written at the second part of this pull request:
    Unlike other menus we need to reload the cfg upon entry since it can be changed in multiple locations.. so if changes are made, not saved and then re-enter the pademu menu the cfg will be reloaded and changes wiped..so make sure to save changes if you wanna keep em.

https://i.postimg.cc/MTH7X0Fq/menu-opl-2.png
https://i.postimg.cc/Y0gbqfvj/menu-opl-3.png
After I will return without saving setting everything resets:
https://i.postimg.cc/MTH7X0Fq/menu-opl-2.png

  1. I have an issue with getting to work via Bluetooth.
    Everything is fine when I have my DS3 controller connected via USB.
    When I pair a USB Bluetooth adapter with a DS3 controller mac address seems to match:
    https://i.postimg.cc/MTs8mw51/menu-opl-4.png
    Once I disconnect a USB cable from a PS2, DS3 does not seem to work (it only flashes like it was looking for a paired device (receiver)).
    I have also tried to pair, save settings and disconnect a cable but it also does not work:
    https://i.postimg.cc/26z0zSKR/menu-opl-5.png

Here is what my USB Bluetooth adapter supports:
https://i.postimg.cc/L4qmR4KZ/menu-opl-6.png

@KrahJohlito
Copy link
Member Author

KrahJohlito commented Jul 4, 2021

  1. is.. tricky, both per game and global actually use the same variables in the code which aren't always in scope within the gui.. so we can't rely on just getting the values from the gui like we can with other menus because we don't know whether they are for per game or global.. that's why I've got it reloading the config upon entry that way we can be sure the values are for global.. Possible solutions I can currently think of I don't really like and would rather keep it as is in this PR.

  2. Isn't related to this PR but if I recall correctly Maximus32 has a fix for this but held it back due to modular pademu.. doesn't look like that is being worked on anymore (at least atm) so just give him some time and maybe he'll open a PR for it when he can, if he doesn't have time I'm sure he wouldn't mind a cherry-pick in the furture.

@AKuHAK
Copy link
Member

AKuHAK commented Jul 20, 2021

Not quite sure what is currently wrong with this PR? It uses only global settings and ignores per game settings? If it is the only issue, then this PR can be merged, @KrahJohlito ?

@KrahJohlito
Copy link
Member Author

I’m not actually sure tbh, as with my other pr I think it’s fine as is :-p .. @J013k am I correct that 2) isn’t a new issue brought on by this pull request?

Other than that I don’t think he likes that the config is reloaded upon entering the menu, this is because the menu is in two places at once one using pergame & global and the other only global but it all uses the same code to avoid duplication.. I suppose some variables could possibly be duplicated as global but idk I really haven’t thought about it since I opened this..

I could possibly revisit it when I get back but that won’t be anytime soon, using laptop and having no ps2 to test is too much hassle.. personally I think it’s fine though.

@AKuHAK
Copy link
Member

AKuHAK commented Jul 22, 2021

Now this PR has merge conflicts

@uyjulian
Copy link
Member

I'll rebase this myself later

@J013k
Copy link
Contributor

J013k commented Jul 28, 2021

@KrahJohlito I have tested recent OPL-v1.1.0-Beta-1701-d0e816f.ELF & OPL-v1.1.0-Beta-1703-4c44ccd.ELF
and I have the same problem (number 2) with Bluetooth, so this issue was not brought by this PR.

As for number 1, if something can be done in this matter it will be great.
If not, I think that at least in the future (a new PR maybe?) it can be fixed...

So to sum up:
I really like that finally we can also have this option in a separate menu.

Best regards.

@KrahJohlito KrahJohlito requested a review from AKuHAK July 31, 2021 00:20
@AKuHAK AKuHAK merged commit c2c0f53 into ps2homebrew:master Jul 31, 2021
@KrahJohlito KrahJohlito deleted the settings branch August 8, 2021 23:30
AKuHAK added a commit that referenced this pull request Sep 30, 2021
Add Controller Settings to menu
citronalco pushed a commit to citronalco/OPL-Daily-Builds that referenced this pull request Sep 10, 2023
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.

4 participants