Fixes to GUI Settings and general workflow#1840
Open
Seneral wants to merge 5 commits intoSlimeVR:mainfrom
Open
Fixes to GUI Settings and general workflow#1840Seneral wants to merge 5 commits intoSlimeVR:mainfrom
Seneral wants to merge 5 commits intoSlimeVR:mainfrom
Conversation
Affects use in command line on linux (gradlew run) when interrupted with Ctrl + C Does not seem to affect use through IntelliJ Idea Services
Clearing a selected file would navigate to "#" and thus return to the home page This was observed with the VRM model in the VMC settings
Server may send SettingsResponse which are not fully populated E.g. only "steamVrTrackers" is set, with others null These three settings pages would reset to default values whenever this occured
vrmJson setting is managed by a separate VMCFile component But it used the same change request as the rest of the settings managed by the main component So VMCFile component kept a copy of the VMC settings state, updated from SettingsResponses It then copied the other VMC settings manually to keep them (!) Conversely, the main component just didn't update the vrmJson There were two problems with this: - A possible race condition as VMCFile may overwrite past changes (rarely a problem in practice) - Manual copying of settings left out newer boolean flags This meant some settings were reset when changing vrmJson - Only reflected in GUI by leaving the settings and going back to them The best solution is to separate the change requests outright: - prevent race condition entirely, even edge cases - does not require changes to both change requests when new settings are added But to keep the server-side similar, the underlying config has not been split This is rather unorthodox, but splitting those up further - but keeping both on the same settings page - is also confusing
github-actions
Bot
added
Area: Application Protocol
Seneral
requested review from
ButterscotchV,
Erimelowo and
loucass003
as code owners
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These fixes were made while working on improving VMC support, as the OSC-related settings pages, especially VMC itself, were very unreliable prior to this.
The first commit is a bonus workflow improvement (confirmed a problem on Linux when server is run from the command line).
This requires SlimeVR/SolarXR-Protocol#212