-
Notifications
You must be signed in to change notification settings - Fork 181
Feature: Win Only - Ability to enable controller preallocation and alternate controller numbering #868
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
base: master
Are you sure you want to change the base?
Conversation
Core new feature of alt controller numbering and preallocation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! Looks good at first glance, does this affect other systems? It seems it only hides the config from webui, will it cause issues if user enables it directly from the config?
…tting Fix rumble/feedback_queue issue, use template in .vue and fixed formatting
|
So far, there have not been any problems enabling it directly from the sunshine.conf file. Most of the new routines are wrapped with a check on config::input.enable_alt_controller_numbering_mode so if it is not enabled, then the new routines should not be executed. The other 2 parameters have defaults which will be used if they do not get a value. /////////////////////////////// Further changes were made because the rumble/feedback_queue messages were not getting propagated correctly. Those are messages from the Apollo system to the client which go to the controller. They have a bit of a different mechanism for that as opposed to the controller to client to Apollo. |
|
I'm quite busy recently, will review later. Thanks again for your work! |
…numbering_v1' into PR868
|
The logic is nested and the var naming is hard to follow I think the ordering string format can be better and more comperhensive, what about using JSON driectly? It can be parsed pretty easily and safe with nlohmann/json which is already included in the project, and can be validated more easily on the frontend to check any errors. |
8600b87 to
11e0890
Compare
|
Ok, after some thought I think it might be better to move the controller assignment settings to each client's config section where it suits the best. And it seems you haven't called any Windows only methods here so this might work cross platform. |
|
I am assuming that these would stay in the Configuration->Input option: It is the alt_controller_order_string that you want changed. This setting would be moved to each client's config section" PIN->Pin Pairing tab->Device Management->CUSTOMDEVICENAME->"Controller Order String". Instead of just one setting in Configuration->Input, there would be a setting per CUSTOMDEVICENAME of something like "Controller Order String". This would eliminate the need to specify the DEVICENAME directly making the setting easier, but each DEVICENAME would need to be potentially gone through. Can this setting just be a text field with a list of numbers separated by spaces? Or would you want an array in JSON format? |
|
We'd better make it easier by writing a graphical configuration for it, as it's already simple enough. I think the underlaying format should be a JSON array of numbers but special flags should be strings like "auto", "never", not magic numbers. Or it's better to have a separate field storing the fallback behavior. |
I am working on the changes requested. |
|
The GUI element for the controller ordering is being added to the pin.html since that view has each of the device names. The rest of the settings are staying in the INPUT view. The buttons are user interactive to build the order list. The list order is displayed and can be reset via the CLEAR button. The special conditions like ANY preallocated-controller will show up as text for the user; the backend will still have the special numbers. Buttons will be disabled as appropriate like once a number is chosen. (4 and 3 in this picture.) |
|
It looks generally fine, the wording or instructions might need a better design. You can push the updates so we can discuss the implementation better before it's done. |
Changed controller order for GUI interface instead of text. Removed the controller list from the Configuration->Input. Added the controller list to the pin.html as part of each named device. Using alt_controller_count_temp and alt_controller_enable_temp in the pin.html which is really the settings from the Config->Input for the alternate controller number.
|
Changed controller order for GUI interface instead of text. Using alt_controller_count_temp and alt_controller_enable_temp in the pin.html which is really the settings from the Config->Input. Those settings are needed for the GUI buttons to show up (enable) and for the number of buttons(count). The backend code was also modified to ignore those 2 settings in the sunshine_state.json file as it is what is in the input setting that should be used. |
|
Thanks for continuing to work on this but the code is just way too yolo, it's essentially just patches over patches to make things "look like" the final idea, but not actually constructed in a proper way. For example, we don't need to duplicate the global config on every device's info, but we can put them on the root of the returned object. I feel this code is massively written/reconstructed by AI since the pattern of "patching" things instead of actually fixing things looks pretty alike. Also, the naming convention is not followed and variable names are ambiguous, what's "controller_list2" and "controller_list3" for? They're not comperhensive enough that will make future maintance a mess. I'm sorry but the code quality can't meet my lowest reqirement here, this PR can't be merged until it handles things properly. |
|
I am working on changing those items. |
pin.html focus: 1. The temporary values in the sunshine_state.json file were eliminated which represented the controller count and if the alternative controller mechanism is enabled. The sunshine_state.json will contain the controller list for the devicename. Instead of using the file, the values are fetched via a new "./api/altcontroller" request. Those temporary values can be moved to sunshine_state.json file (just a single copy), but this seemed like a cleaner method to keep the sunshine_state.json file without extra values. 2. The controller list variables have been reorganized. A. this.controllerListNumbersForInternal - Internal to pin.html which is a list of the numbers representing the controllers. This is the main list used when a button is pressed to add/clear values. B. this.controllerListTextForUsers - Internal to pin.html which is string that is displayed to the screen of the controller number and the special numbers displayed as text. This is for the GUI use only. This uses the controllerListNumberForInternal variable. C. client.editControllerListNumbers - Used with standard mechanisms for exchanging values between the sunshine_state.json file D. client.controller_list_numbers - - Used with standard mechanisms for exchanging values between the sunshine_state.json file
Better organization of the pin.html methods for the controller
If that is unacceptable, can the /api/clients/list API call be used to add those 2 pieces of information? The getClients() in confighttp.cpp already adds "output_tree["platform"] = "windows";" which is not part of the state file.
|
|
Moved "alt_controller_count" and "enable_alt_controller_numbering_mode" to getCLients() API and got rid of getAltController() Made similar changes to the pin.html and fixed a bug in which the "platform" was changed to blank when switching between OTP Pairing and Pin Pairing.
Potential Bug fixed/workaround:
The security and alternate controller options differ in several ways. Difference 1 - //////////////////////////////////////// Example (that I actually have been using): SystemA will get assigned 1,2. The this.controllerListTextForUsers is really needed so that the list of numbers can be translated to a string. In summary, I will try to look at eliminating the this.controllerListNumbersForInternal and just use client.editControllerListNumbers. Please let me know if I am misunderstanding or missing the point. I will also continue to look at the permission system and see if there is something more. |
Removed temp controllerlist variables
|
|
Please let me know if you want additional changes. |
The controller mode did not have a setting before. Now it does. Before the only controller mode was the strict mode. Now the strict, share, and both modes are available. Added the ability to have shared host controllers between 2 or more client controllers. This means that 2 or more people can control the same controller on the host side. ////////////////////////////////// On the configuration->input web page, there are now the following options: Enable alternative controller numbering mode (Existed in original pull request) ---Number of Controllers to preallocate (Existed in original pull request) --- Alternate Controller Mode (new) ------ Mode 1: Strict (was really the default and not an option in the original pull request) ------ Mode 2: Shared ( new ) ------ Mode 3 : Both (new) – Has ability to switch between both modes via SHIFT+CTRL+ALT+I keypress ////////////////////////////////// On the PIN web options per named device/client, there are now the following options (if the alternative controller numbering mode is enabled) --- Alternate Controller Strict Mode List (Existed in original pull request) --- Alternate Controller Shared Mode List (New) --- Alternative Controller Jitter Joystick Correction List (New) – Typically needed when sharing controllers so that an “idle” controller does not seize control. --- Alternative Controller Swap Joysticks List ( New )
|
Added the ability to have shared host controllers between 2 or more client controllers. This means that 2 or more people can control the same controller on the host side. Most of the additional code was based on the original code of the pull request. |
|
I have added a compiled version of this on https://github.com/thesystemcoder168/Apollo_MyChanges/tree/support-apollo-pull-868/beta as there appeared to be some interest in the discussions/issues at one point for further testing of shared controllers. The user would still need to copy the files over an existing Apollo directory installation as it does not contain the drivers. |
…n of controllers to be more effective Add user scripts to remove unconnected controllers for the enumeration of controllers to be more effective.
|
Added user runnable scripts to uninstall/remove unused game controllers which appears to help the controller enumeration. (if needed) |
…removal script everytime. Tweaked controller removal script for more device manager removals related to controllers and added better directions to run removal script every time and without a reboot. 2 files were changed: src_assets\common\assets\web\public\assets\locale\en.json src_assets\windows\misc\gamepad\uninstall-existing-controllers-for-alt-controller-enumerating.bat
|
Tweaked controller removal script for more device manager removals related to controllers and added better directions to run removal script every time and without a reboot. 2 files were changed: |

Use Case:
When hosting a game with 4 players connecting via Moonlight to Apollo, there is the need to control which player gets which controller number on the host system.
Feature Details:
Through the Configuration->Input options, one can enable the alternate controller numbering. By default, this feature is NOT ENABLED. This feature will enable Apollo to preallocate the desired number of controllers when the first controller is added. There is also the ability to control which device name gets which controller or which set of controllers depending on what is available.
This will only assign the controllers that are preallocated by Apollo.
Alternate Controller Number String --- Example:
DeviceName1="host1" Order1="4 2 999" DeviceName2="win10" Order2="3 999 1000" DeviceName3="NOMATCH" Order3="90"
Syntax: There is the DeviceNameX="<DEVICENAME_STRING>" and the OrderX="[NUMBER_SEPARATED_BY_SPACE]" parameters.
The DeviceNameX needs to have the string assigned to the device (at PIN time). The NOMATCH device name is special in this context to represent all of the devicenames that do not have this rule.
The OrderX needs to have the enumerated number of the controller as Apollo preallocated those controllers. This starts at 1. Special numbers are 999 and 1000 which are explained later. Each number needs to be separated by a space. If the number cannot be used, the logic goes to the next number to see if there is a match.
Meaning: host1 will give controller number 4 in the host system (as was preallocated by Apollo) first, next in line is controller number 2.
The 999 means that any controller preallocated by Apollo can be used.
The 1000 means that a new controller will be added to the system (This is not recommended especially over 4 controllers.)
The 90 means that there will be no controllers match at that number. This is useful for the special device name of NOMATCH which is the rule that will be used if none of the device names are found.
Overall:
DeviceName of host1 will try to get controller 4 first, if not successful, it will try for controller 2, and if still not successful, it will try for any of the remaining controllers preallocated.
win10 will try to get controller 3, then any controller preallocated (999), then add a new controller (1000)
All other devicenames will go to the NOMATCH rule. This will not give a controller to the user since the number 90 is larger than the controller limit
If this field blank, then the following command will be used:
DeviceName1="NOMATCH" Order1="999"
This will use any of the controllers which have been preallocated for all device names.
#### End Description for the alt_controller_order_string parameter ####
Other Recommendations/Limitations: