Add Toggle Joystick Add-on #1131
Conversation
This reverts commit 8c00d6e.
|
We closed your previous PR and you have re-opened it with a new PR. We are issuing a temporary week ban @tupleu , please do not open this PR again on our main branch. If you would like to support your own features, GP2040-CE is a open, free, MIT licensed project and you are welcome to support and maintain your own code and base. |
|
@arntsonl Hi, this is not the same pull request as the previous one. I removed half of the features, that being the tilt toggle which was the main point of contention in the last PR. I also resolved the merge conflicts that @TheTrainGoes requested. So, if you all could please take another look at this PR that would be greatly appreciated. |
|
@tupleu There are conflicts with this against main. Please update it so I can build a UF2 and I will check it out. My attempted build from your branch failed as well. |
|
@TheTrainGoes Thank you for the response. I just merged main into this branch again which might fix the issues. However, I just made a fresh build without any issues building on my side. Are there any flags I should have set? |
|
@tupleu the produced artifact for this does not have anything changed in the web-config. |
|
@TheTrainGoes I don't know if it's my code but sometimes I have to do a full reconfig of cmake for it to rebuild the web-config code. I usually do |
|
@tupleu this was built from GitHub directly so there should be no issues. Please review your code and let me know when its ready to test out. |
|
@TheTrainGoes I just flash nuked my device, downloaded the build artifact, tested it out and it seems to work fine. I then tried again, upgrading from the stable release and it still works fine for me. The only changes are two new pins in the pin configuration menu and an option for the joystick toggle add-on in the add-ons menu. Could you check one more time to see if maybe a leftover build was on your device and see if these new options are present? |
|
@TheTrainGoes Hi, if you could please try the build artifact again. I tried it on my device and everything worked as intended. |
|
Hi @tupleu , I re-ran the build and was able to get a working artifact from it. Tested it out personally and we are currently talking about it internally. I will come back to you when we have some feedback ready. |
|
@TheTrainGoes Okay, thank you for your time and effort. |
|
@tupleu , Had a chance to play around with this a bit. I understand now what you are doing with this but we need to work on how it is named / presented as the current name does not really click with me. Something like ""Direction Freeze Toggle
If the overall goal of these submissions is to mimic how the Cross|Up works then I believe the best path is to have this stand-alone addon and separately some changes made to the existing tilt addon. Through our testing it appears that this causes breaking changes to current SOCD rules. We are going to move foward with the v0.7.10 release without this included but can look at it for future inclusion if we can address the naming concerns and also fix any SOCD issues. |
|
@TheTrainGoes Sounds good to me. I am down for a name change as I wasn't quite able to find one I was completely happy with. For brevity's sake we could even just call it "Direction Toggle" and add a switch to enable or disable the freezing in the add-on configurator. The overall goal was to mimic how the Cross|Up functions but as it stand now I believe it's sufficient to play most non-action games without the tilt feature. I originally added on the tilt feature as an addendum once I realized the existing tilt add-on was a separate add-on that did its own thing. I'm okay with making this add-on standalone and making changes to the existing tilt add-on. However, I would caution that this could introduce an ordering dependency. For example if it first freezes the previous inputs, then tilts them to 50% intensity; or if it first tilts the sticks to 50% intensity, then adds the previous inputs. I think at that point it would make sense to just put the logic together into a single "Toggle" add-on rather than introducing an unnecessary dependency to the project. Let me know your thoughts though. Just to be clear, will you all be making the changes internally or do you want me to make those fixes? Let me know if there is anything needed from me. |
|
Hi @tupleu , You will need to make these changes. We are going to be doing the v0.7.10 release soon so it would be best to wait until that has rolled out and we can take another look at this. |
|
I would hold off on making changes to this PR, in the interest of time. This discussion sparked a conversation that is leading me to conclude the lock is attainable with no new addon and instead just some minor tweaks. |
|
@bsstephan Hi, could you elaborate on this? If there is going to be some logic for storing and freezing inputs, instead of being run all the time, shouldn't it be contained in an add-on? Especially if not everyone will use this add-on. |
|
There are a couple considerations here. The first one is that there are already actions for enabling JS=LS/RS, so...
...is already a feature in core. The second is that the locking described seems to boil down to the behavior of the core LS/RS emulation when the buttons are not held, which could be a simple option in the core gamepad to set them to midpoint or leave them at their old value. For example, I was able to implement a rudimentary locking just by commenting out some code. Those two points combined, I'm skeptical how much of this needs to be an addon at all, and thirdly I think addons overriding the gamepad should be an exceptional case, as the interaction of all the pieces together can be a bit complicated to understand, and the long-term maintenance of a separate addon is more expensive if its usage is minimal --- see for example the concern about what this does to SOCD. Essentially, this seems like a complicated change and I'm not sure if it warrants the complication compared to an alternative. Maybe in the end it is right to be this complicated, but I wouldn't go too much further down the road of this PR until we know more. |
|
@bsstephan Thank you for your response. I am not opposed to moving my code into the core instead of keeping it in an add-on. There would need to be a place in the configurator for setting the two toggle modes as well as a toggle for the freezing. As for the complexity, most of the logic is devoted to how the toggle occurs. The feature may seem simple at first glance but there are a couple behaviors to consider. Currently, it allows for a common use case, holding up to move forward on the left stick and then toggling to the right stick, to operate smoothly. Without extra logic for this behavior, there would be a jerk upwards every time you switched in this scenario. There's additionally more logic to allow for smooth switching between modes both from the add-on itself as well as from external triggers. I also have not been able to notice any issues with any of the SOCD cleaning modes as they seem to work fine on all the d-pad modes. So please let me know what issues there are that I need to fix. |
| @@ -57,6 +57,8 @@ export const BUTTON_ACTIONS = { | |||
| BUTTON_PRESS_E10: 52, | |||
| BUTTON_PRESS_E11: 53, | |||
| BUTTON_PRESS_E12: 54, | |||
| BUTTON_PRESS_TJ_TOGGLE_1: 55, | |||
There was a problem hiding this comment.
@tupleu I'm sorry for being some random GitHub user giving you this feedback, but it looks like this line has spaces instead of tabs.
P.S. I'd like to contribute to this repository in the future, so I was just browsing some pull requests to get familiarized with the code.
There was a problem hiding this comment.
Thank you for the feedback. Nice catch, I'll fix it in the next commit.
| @@ -79,5 +79,7 @@ export default { | |||
| BUTTON_PRESS_E10: 'Extra 10', | |||
| BUTTON_PRESS_E11: 'Extra 11', | |||
| BUTTON_PRESS_E12: 'Extra 12', | |||
| BUTTON_PRESS_TJ_TOGGLE_1: 'Joystick Toggle Dpad Primary', | |||
There was a problem hiding this comment.
It looks like this line also has spaces instead of tabs
Adds the Toggle Joystick Add-on which allows the user to switch dpad modes while held.
First, the dpad toggle buttons. When held, the dpad mode is switched to the selected dpad mode. When released, the dpad mode is switched back. Additionally, if any direction on the joystick is held, the input is frozen. For example the user can freeze the left joystick to hold up, while switching to the right joystick to move it around. This allows for a smoother experience when needing to utilize both the left and right joysticks.
The user can select two different buttons, a primary and a secondary toggle, both customizable in the add-ons configuration. For example, the primary can switch the dpad mode to right joystick and the secondary can switch to the dpad. The primary toggle will take precedence over the secondary toggle as well as freeze any inputs held by the default dpad mode and the secondary dpad mode. This allows for the simultaneous input of the left and right joysticks as well as the dpad.
The pins for the primary and secondary toggle have been implemented in the pin mapping section, which allows for the buttons to be different for different profiles.