"While held" set change sometimes returns to wrong set when switching between multiple sets (RCA included)

[rogerhub]

Is there an existing issue for this?

• I searched the existing issues and did not find anything similar.

Current Behavior

In set 1, I have:

• L trigger: switch to set 2 while held
• R trigger: switch to set 3 while held

Sometimes, I press L trigger and R trigger together, then release them both. Usually I return to set 1 after releasing both triggers, but sometimes I end up in set 2 or set 3.

Expected Behavior

Releasing both L and R triggers should always return to the original set.

Steps To Reproduce

1. Configure L trigger and R trigger as described above
2. Press & release L trigger and R trigger simultaneously until you end up returning to set 2 or set 3 instead of set 1

Environment

Program Version 3.4.0
Compiled from packaging: GitHub Windows Release
Built Against SDL 2.30.1
Running With SDL 2.30.1
Using Qt 5.15.2
Using Event Handler: SendInput
Host OS: windows Version: 10 Architecture: x86_64

Anything else?

No response

---

Upvote & Fund

• If you find this issue important, mark it with 👍. It lets us see which fixes and features are demanded by the most users.
• We're using Polar.sh so you can upvote and help fund this issue. It may incentivize some developers to contribute to this project and fix some bugs.
• Funded developer receives the funding once the issue is completed & confirmed by you.
• Thank you in advance for helping prioritize & fund our backlog.

[rogerhub]

So, I believe this is a race condition, and I think I know why it occurs.

The JoyTabWidget is responsible for displaying the different sets in the user interface. It has a method JoyTabWidget::changeCurrentSet which is called in two places:

• First, it is connected to InputDevice::setChangeActivated here: https://github.com/AntiMicroX/antimicrox/blob/2f7b66be195d8b3131bce9a250149ff3e7a62db9/src/gui/joytabwidget.cpp#L588C26-L588C57
• Also, it is connected to the "Set 1", "Set 2", etc buttons in the user interface

  antimicrox/src/gui/joytabwidget.cpp

  Line 1321 in 2f7b66b

  void JoyTabWidget::changeSetOne() { changeCurrentSet(0); }

Interestingly, the changeCurrentSet method also calls m_joystick->setActiveSetNumber(index);. This is useful in the context of clicking "Set 1", but it is actually problematic when performed in response to InputDevice::setChangeActivated.

Every time InputDevice::setChangeActivated is emitted, we directly call InputDevice::setActiveSetNumber, but we also asynchronously schedule a call to JoyTabWidget::changeCurrentSet which calls InputDevice::setActiveSetNumber again. So, there are two calls to InputDevice::setActiveSetNumber every time we perform a set change using the controller.

The second call is unfortunately not running on the input daemon thread, so it is not synchronized with any other input event. That means it can overwrite a simultaneous set change, thus causing the race condition.

[rogerhub]

I recompiled the program with some fixes to joytabwidget.cpp to avoid calling m_joystick->setActiveSetNumber(index); whenever the JoyTabWidget is reacting to a controller button. My version of the program will only call m_joystick->setActiveSetNumber when reacting to a GUI button click. After I made this change, the bug no longer occurs for me.

So, I have fixed the bug for myself, but I'm reporting it here in case you want to fix it in the upstream too. Let me know if you need more details.

[pktiuk]

Hi @rogerhub ,

Thank you for help with some issues on this repo. :)

in case you want to fix it in the upstream too

Of course I want this fix upstream. PR-s are welcome.
If you don't want to deal with pull request, then please at least publish code with your changes. It would be a good starting point

[rogerhub]

My employer has some policies for open source contributions, so I don’t know if I can provide code without applying for some kind of approval :) but hopefully I can describe the fix clearly enough that anyone should be able to contribute the code.