scp-fs2open
Increase JOY_NUM_BUTTONS to 128 and add required localizations
I want to put this out as less of a "pull request" and more of a strawman or question from a drive-by contributor - is this worth doing? Are there non-obvious downsides (performance? backward compatibility with old pilot profiles [maybe the movement of the hat switch identifiers has bad implications]? etc.).
Bumping the joystick button limit to match what I understand to be the maximum allowed by DirectInput (plus the now excellent multiple-controller support) would be a big help for those of us with "fancy" HOTAS setups. Many mid- and high-end controllers support acting as multiple "virtual controllers", but for at least some this doesn't work with FS2Open due to each "virtual controller" having the same identifiers.
Have you tested whether this is sufficient to actually use buttons past 32? I assume you have such a joystick?
Yes, it's worked fine with my setup (joystick + a separate throttle with ~100-odd buttons) on "my machine" (Windows 10, build from above patch on MSVC 2022, MediaVPs, Knossos.NET) in limited playtesting. I don't have the capability at the moment to test much beyond that.
So it sounds like it fixes issues for joysticks with near the max number of buttons, so I think we just need someone to test the converse: that joysticks with fewer buttons (and mice and keyboards) still work with these changes.
Awesome, I may be able to do some testing with a Saitek Cyborg Evo (~10 buttons, IIRC) when I get back from some work travel.
@z64555 what did you have in mind for a procedural translation? I like the idea but didn't feel comfortable either going from static arrays to a lookup function or introducing some kind of metaprogramming (templates or macros) to generate the static arrays. That second option is the way I would lean but I am a total rando to this codebase!
Hey, I'm just converting this to draft until we figure out if we want to make the string generation dynamic or not, so that no one merges it by accident. I'm good with it being static.
I summoned something from the dreaming void. It appears to be valid C++20: https://gist.github.com/derrickturk/51c68fb15cbf82211dc4c812832487b3
I think there may be an even shakier, standards-wise, way to "count" using the preprocessor, but it's beyond me. At most I'd probably use an "X-macro" to simplify the generation of arrays entries, like so: https://gist.github.com/derrickturk/2e26b18de7f075b0b069bae63ec24df8. At least that keeps the "counting" simple and in one place, although it introduces the requirement to make sure that the count constants get updated concurrently with the definition list. I think keeping it as is would be just fine too and possibly the least confusing option for most programmers.
I guess I should elaborate that the above techniques are both "static" in the sense of operating at or before compile-time and generating honest-to-god constant arrays. There would be a whole other set of approaches for generating these at runtime - probably simpler to program but at the expense of a tiny bit of performance and complexity (when do we initialize them, etc etc).
I don't think we'd be able to upgrade to c++ 20 just for this. We do have some c++ 17 features added via small libraries, but we're targeted to c++11 for now. I don't think anyone would object to adding a library to manually add a c++20 feature. The choice is up to you and @z64555 on whether that's worthwhile or not.
IMO it's not worth it for the metaprogramming here - I've blown up several "C++20" compliant compilers with similar code (the core idea comes from a named-tuple implementation) so it's a bit "bleeding edge".
My personal comfort zone would be using an X-macro to keep the arrays global, static, and constant, but I don't see that pattern elsewhere in the codebase and I'd be reluctant to throw it in here as a one-off unless it's well-known to other developers.
Probably the best answers, again IMO, are to keep things as is, or perhaps introduce a singleton like io::mouse::CursorManager which generates the tables dynamically once and gets initialized "early" with a static init() function.
PR #6371 adds the procedural translation, so the localization part of this PR is obsolete. That leaves just the number bump.
We had someone recently ask about increasing the number of buttons, so just wanted to check in on this PR. Looks like @z64555 has approved it. What's the remaining blockers to move it out of draft and into review/merge readiness? :)
IIRC @z64555 built a proper implementation of procedurally-generated button names in a previous commit. I read some of the discussion around that and I believe the blocker on increasing JOY_NUM_BUTTONS was an interaction with the control presets code. I have not had a chance to investigate that further yet, unfortunately.
Chatting with z in Discord, at this point all that is needed for the control limit bump is "essentially its a one-line change thats blocked by needing to test if it would break anything"
@derrickturk I've gone ahead and made just the bump PR separately in #6479; out of curiosity, would you be able to test that PR?
@derrickturk I've gone ahead and made just the bump PR separately in #6479; out of curiosity, would you be able to test that PR?
I was able to build your branch and try it out last night. I didn't test exhaustively but I was able to bind buttons in the 50s and 60s without a problem and use them in-flight. I was also able to save and load control presets. So this change seems OK as far as I can tell!
The only issue I noticed is not related to your changes, but showed up when binding a hat switch:
I bound the shield quadrant controls to the corresponding hat directions (e.g. forward should be hat up...), but the labels in the binding UI show up incorrectly (my "up" shows as "right", etc.). I've verified with Windows joy.cpl that the directions reported by my stick's hat correspond to the physical hat directions.
Thanks for testing! Hmm and that hat direction bug only occurs on this branch and not a current nightly, correct?
Thanks for testing! Hmm and that hat direction bug only occurs on this branch and not a current nightly, correct?
The bug is present in the nightly builds as of nightly 20241222. I suspect it's nothing to do with your button limit change but rather something to do with the changes to procedural generation of button labels. If I had to guess, I'd closely check the decision around here to re-order the label arrays: https://github.com/scp-fs2open/fs2open.github.com/pull/6371/commits/0f14b61dfb96124a13e2a538d91398ad4d39b2d7.
@derrickturk I've fixed/merged the hat labeling bug and also merged the button limit bump. Thanks again for all your work and testing. I will also now close this PR :)