-
Notifications
You must be signed in to change notification settings - Fork 47
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
Fix vjoy activation condition handling #407
base: develop
Are you sure you want to change the base?
Fix vjoy activation condition handling #407
Conversation
While the fix will solve one issue I'm unclear whether it will cause another one. The true underlying issue is that the modify function is used both for initialization, where the defaulting of variable states is important, and updating whenever a vJoy value changes, where this defaulting is not intended. Looking a bit more at the code, outright removal of the input type based checks will cause bugs. The reason for that code is to enforce the Also, the Lastly, it is not likely that any of these fixes will be merged as R14 does not reduce any of the UI, and most if not all of the action logic has to be redone as well. While fixing some of the remaining common bugs would be nice, integrating, testing, etc. of those would eat whatever little time I have to work on R14 which is why I'm unlikely to do any of that. |
ba96c09
to
7a6e28a
Compare
Check if current comparison condition is valid for the type of VJoy condition under `_modify_vjoy`. Only reset comparison condition if it is not valid for the current input type. Previously comparison condition would reset with any call to `_modify_vjoy`.
7a6e28a
to
d52c2d6
Compare
You are right. I did not consider the I understand that you don't have the time to integrate these changes yourself, but I'd still hope this fix is useful to folks in the community that are willing to build their own versions (few of us though there may be!). Thanks for your efforts all the same. |
This looks good, there are probably some ways to make it less clunky but really it would need proper enums for the various conditions and inputs, things that don't exist in that code base, at least not for the conditions. So this is a good middle ground. Yeah, that is why I comment and leave the pull requests open so that people actually can see them for their own use. Also helps me to remember things that I need to keep in mind when implementing those functions. This kind of bug and annoyance with condition types for example doesn't exist anymore in R14 conditions. |
This has come up in the discord a couple of times (and mentioned here as well #361). Decided to take a look myself to fix the issue. This fixed the problem on my end. Happy to provide a compiled win x64 binary if that is of use too.
Here are the changes from the commit itself:
Previously all vjoy activation conditions would be reset to the default value when the UI was refreshed. This was due to several lines in
VJoyConditionWidget._modify_vjoy
which were intended to reset to a default value if the VJoy input was modified. Unfortunately,_modify_vjoy
is called every time the UI is reset, thus resetting the condition trigger in addition to the intendedeffect of updating the selected VJoy input.
To avoid the reset, we simply removed the lines that change the activation condition within
_modify_vjoy
.