Nautilus compatibility for GameInput keybinds#2663
Nautilus compatibility for GameInput keybinds#2663JackNytely wants to merge 9 commits intoSubnauticaNitrox:masterfrom
Conversation
|
I see now thx to @misterbubb - the new update to Nautilus broke the compatibility: NullReferenceException: Object reference not set to an instance of an object
at GameInput.GetBinding (GameInput+Device device, GameInput+Button action, GameInput+BindingSet bindingSet) [0x00000] in <c055c80802d14a9db359fbdc91eb371c>:0
at GameInput.AppendDisplayText (GameInput+Button action, System.Text.StringBuilder sb, System.Boolean allBindingSets) [0x0001c] in <c055c80802d14a9db359fbdc91eb371c>:0
at GameInput.FormatButton (GameInput+Button action, System.Boolean allBindingSets) [0x0000d] in <c055c80802d14a9db359fbdc91eb371c>:0
at uGUI_CameraCyclops.UpdateBindings () [0x00000] in <c055c80802d14a9db359fbdc91eb371c>:0
at uGUI_CameraCyclops.UpdateTexts () [0x00000] in <c055c80802d14a9db359fbdc91eb371c>:0
at uGUI_CameraCyclops.OnEnable () [0x00000] in <c055c80802d14a9db359fbdc91eb371c>:0
UnityEngine.Object:Internal_InstantiateSingle_Injected(Object, Vector3&, Quaternion&)
UnityEngine.Object:Internal_InstantiateSingle(Object, Vector3, Quaternion)
UnityEngine.Object:Instantiate(Object, Vector3, Quaternion)
UnityEngine.Object:Instantiate(GameObject, Vector3, Quaternion)
UWE.Utils:InstantiateWrap(GameObject, Vector3, Quaternion)
<InitializeAsync>d__43:MoveNext()
UnityEngine.SetupCoroutine:DMD<UnityEngine.SetupCoroutine::InvokeMoveNext>(IEnumerator, IntPtr) |
Measurity
left a comment
Measurity
left a comment
There was a problem hiding this comment.
Nitrox keybindings appears twice with these changes:
Minor: when BepInEx mods are loaded, I'm unable to change Nitrox keybindings as well.
NitroxPatcher/Patches/Persistent/GameInputSystem_Initialize_Patch.cs
Outdated
Show resolved
Hide resolved
NitroxPatcher/Patches/Persistent/GameInputSystem_Initialize_Patch.cs
Outdated
Show resolved
Hide resolved
Will look into fixing this, not sure why it appears as double, will investigate asap |
Measurity
left a comment
Measurity
left a comment
There was a problem hiding this comment.
The changes to support AsyncBarrier in net472 code is quite involved. But AsyncBarrier is not used right now except for (net10) server.
Would you mind reverting changes made for AsyncBarrier in an effort to keep complexity to the minimum?
Keeping DLLs in the output root avoids assembly load failures when AssemblyResolve is not used (e.g. some load order/contexts in Release). Fixes empty server list and related issues in Release-only builds.
I was attempting to fix an issue where the server list in the launcher itself was empty and crashing if you tried creating a server, however, I found out that the issue was in the way the app builds a release, publishing works, but if you run from direct release, it moves the dlls over instead of copying which causes it to break in some areas. I removed my changes and fixed the issue in the build script instead Kept the fixes for nautilus to work though. |
I am not getting duplicates on my side and keybinds seem to work now |
…te keybinds The game builds the input binding list from both Enum.GetValues and GameInput.AllActions. Adding Nitrox buttons in both places caused 'Open chat' and 'Focus on the Discord invite window' to appear twice when running Nitrox without Nautilus. Nitrox keys are now added only via AllActions in GameInputSystem_Initialize_Patch so each keybind shows once.
The changes I made caused the UI to build the input bindings from both Enum.GetValues and GameInput.AllActions, by adding nitrox in both it caused the duplication when running without nautilus enabled I have fixed this issue now, thanks for pointing it out <3 |
|
Thanks for working on this. I noticed the input bindings cannot be rebound when starting without mods. Please verify behavior doesn't regress when playing Nitrox without BepInEx / Nautilus. |
Server shutdown could hang indefinitely when the process didn't respond
to the quit command. Added timeout-based shutdown with force-kill fallback,
and ensured embedded servers stop when the launcher closes.
Nitrox keybinds (Open Chat, Focus Discord) could not be rebound in the
settings UI due to infinite recursion in the AddBindingOption patch.
Changed to modify the label by reference instead of recursively calling
AddBindingOption. Added duplicate checks for Nautilus compatibility when
extending both Enum.GetValues and GameInput.AllActions, registered
'Option{id}' localization entries for the binding conflict dialog, and
added default controller bindings (D-Pad Up/Down).
I fixed the issue with rebinds not working on nitrox without mods, found the issue in my code and fixed it ( nautilus with bepinex still has the binding issues, I will need to investigate this in the future) I also ran into issues with nitrox servers sometimes not starting, and the stop buttong sometimes does nothing, forcing me to close the game and nitrox and task manager the server (I also fixed this issue in my PR) |
There was a problem hiding this comment.
Job well done, thanks.
Setting input bindings works with Nitrox. And mods load now at least.
That said, I'm not getting the "server close not working" issue, but would like the GameInput keybinds fixes to be merged.
Can the server related bug fixing be split in a separate PR?
| @@ -1,4 +1,4 @@ | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
There was a problem hiding this comment.
Please revert file changes.
|
|
||
| // Stop all embedded servers when launcher closes | ||
| // Non-embedded servers are left running as they have their own console window | ||
| lock (serversLock) | ||
| { | ||
| List<Task> stopTasks = []; | ||
| foreach (ServerEntry server in servers) | ||
| { | ||
| if (server.IsOnline && server.IsEmbedded && server.Process != null) | ||
| { | ||
| try | ||
| { | ||
| Log.Info($"Stopping embedded server {server.Name} during launcher shutdown"); | ||
| stopTasks.Add(server.StopAsync()); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Log.Error(ex, $"Error stopping server {server.Name} during shutdown"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Wait for all servers to stop with a timeout | ||
| if (stopTasks.Count > 0) | ||
| { | ||
| try | ||
| { | ||
| Task.WaitAll([.. stopTasks], TimeSpan.FromSeconds(30)); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Log.Error(ex, "Error waiting for servers to stop during shutdown"); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Is this code ever called? No debugger breakpoint hits on launcher exit. I think the code can be removed.
MainWindowViewModel also already handles server exit.
| Log.Warn($"Server {Name} did not respond to quit command, attempting force terminate"); | ||
| using ProcessEx? proc = ProcessEx.GetFirstProcess(GetServerExeName(), ex => ex.Id == LastProcessId); | ||
| proc?.Terminate(); | ||
| break; |
There was a problem hiding this comment.
Terminating the server on timeout can break server saving. I'd prefer we investigate why the server doesn't exit and fix instead of terminating it.
If you know how to trigger the bug, please create a new issue with steps. Then we fix this in a separate PR.
There was a problem hiding this comment.
Yeah I agree, I think it could also help with speed of merging the prs. Here it feels like three prs in one
| logger.ZLogWarning($"An error occurred while deserializing the prefab cache. Re-creating it: {ex.Message:@Error}"); | ||
| } | ||
| if (cache.HasValue) | ||
| if (cache.HasValue && cache.Value.Version == CACHE_VERSION) |
There was a problem hiding this comment.
Thanks for noticing this bug. Fixed in #2674 in case your PR stays unmerged for a while.
| <!-- Copy (don't Move) so dependencies stay in output root. Some code paths load assemblies before/when AssemblyResolve runs or from contexts that don't use it (e.g. Release vs Debug), causing empty UI or missing types when DLLs are only in lib/. --> | ||
| <Copy SourceFiles="@(FilesToMove)" | ||
| DestinationFolder="$(OutputDirectory)" | ||
| OverwriteReadOnlyFiles="True" | ||
| SkipUnchangedFiles="True" | ||
| Retries="3" | ||
| RetryDelayMilliseconds="100" | ||
| Condition="'$(Configuration)' == 'Debug'" | ||
| ContinueOnError="ErrorAndContinue" /> | ||
|
|
||
| <!-- Move every matching files to OutputDirectory --> | ||
| <Move SourceFiles="@(FilesToMove)" | ||
| DestinationFolder="$(OutputDirectory)" | ||
| OverwriteReadOnlyFiles="True" | ||
| Condition="'$(Configuration)' == 'Release'" | ||
| ContinueOnError="ErrorAndContinue" /> |
There was a problem hiding this comment.
This is intended behavior. Avalonia doesn't use our AppDomain.CurrentDomain.AssemblyResolve. In Debug we're fine with duplicating the files so Avalonia Previewer works.
| Log.Warn($"Server {Name} did not respond to quit command, attempting force terminate"); | ||
| using ProcessEx? proc = ProcessEx.GetFirstProcess(GetServerExeName(), ex => ex.Id == LastProcessId); | ||
| proc?.Terminate(); | ||
| break; |
There was a problem hiding this comment.
Yeah I agree, I think it could also help with speed of merging the prs. Here it feels like three prs in one
| public class ChatKeyBindingAction : KeyBinding | ||
| { | ||
| public ChatKeyBindingAction() : base("Nitrox_Settings_Keybind_OpenChat", "y") { } | ||
| public ChatKeyBindingAction() : base("Nitrox_Settings_Keybind_OpenChat", "y", "dpad/up") { } |
There was a problem hiding this comment.
Are these universal across all unity controllers? Will it only work for say playstation but not Xbox?
| } | ||
|
|
||
| private static MemberInfo GetMemberInfo(LambdaExpression expression, Type implementingType = null) | ||
| private static MemberInfo GetMemberInfo(LambdaExpression expression, Type? implementingType = null) |
There was a problem hiding this comment.
Thanks for improving this code. Added in #2674 in case your PR stays unmerged for a while.
4 participants
1. Keybind compatibility with Nautilus
Problem
When Nitrox and Nautilus run together, Nitrox buttons may not get
InputActionentries. That can causeRegisterKeybindsActionsto fail becausegameInputSystem.actions[NitroxButton]does not exist.When Nitrox runs without Nautilus, the game was showing duplicate keybinds in the input settings ("Open chat" and "Focus on the Discord invite window" twice) because Nitrox buttons were added in two places:
GameInput.AllActionsandEnum.GetValues(GameInput.Button). The game builds the binding list from both, so each Nitrox keybind appeared twice.Solution
GameInput.AllActions: In theInitializeprefix, extendAllActionswith Nitrox buttons so the game builds the action map for them. A duplicate check prevents adding them again whenAllActionswas already initialized from the patchedEnum.GetValues.GameInputSystem.Deinitialize()to restore the originalAllActionsarray.AllActions),RegisterKeybindsActionscreates and registersInputActions for Nitrox buttons in the same way as Nautilus'sInitializePostfix.Enum_GetValues_Patchstill extendsEnum.GetValues(GameInput.Button)with Nitrox buttons (required for the rebinding UI to recognize them as valid), but now checks for duplicates so Nautilus or repeated calls don't add them twice. Combined with theAllActionsduplicate check, each keybind appears once in the settings UI.Changes
Deinitializeto restoreAllActions.AllActions(prevents double entries when already initialized from patchedEnum.GetValues).TryGetValuecheck inRegisterKeybindsActionswith manualInputActioncreation when an entry is missing.gameInputSystem.OnActionStarted, add bindings, and enable them to mirror Nautilus behavior."Option{buttonId}"localization entries inLanguage.mainso the binding conflict dialog shows translated labels (e.g. "Open chat") instead of "Option1000".AddBindingOption— which triggered the prefix again on the same button, recursing infinitely. Changed to avoidprefix that modifies thelabelparameter by reference, so the original method runs exactly once with the correct display text.dpad/upanddpad/down) so the game creates proper controller binding slots during initialization, enabling controller rebinding.2. Server shutdown improvements
Problem
When stopping a server through the launcher, the shutdown could hang indefinitely if the server process didn't respond to the
quitcommand. Additionally, embedded servers were left running when the launcher was closed.Solution
Add timeout-based shutdown with force-kill fallback, and stop all embedded servers on launcher dispose.
Changes
CancellationTokenSourcetimeout to the shutdown loop. If the server doesn't respond toquitwithin the timeout, force-terminate the process. Also handleOperationCanceledExceptioninStopAsyncby force-killing the process when the graceful shutdown times out.Dispose, iterate all online embedded servers and callStopAsync, waiting up to 30 seconds for all to finish. Non-embedded servers (with their own console window) are left running.3. Release build: keep dependencies in output root
Problem
In Release, dependencies were moved into
lib/(Debug only copied). Some code paths load assemblies before or outside the customAssemblyResolve(e.g. Release vs Debug), so when DLLs existed only inlib/, the launcher could fail to load them. That led to empty server list, missing types, or other assembly-load issues in Release only.Solution
Use Copy for both Debug and Release instead of Move in Release, so dependencies stay in the output root as well as in
lib/.Changes
FilesToMoveintolib/(remove the Release-only Move). Update comment to explain that keeping DLLs in the output root avoids empty UI / missing types when they would otherwise be only inlib/.4. Minor improvements
if (cache.HasValue)condition so an outdated cache falls through to the rebuild path directly. Log message now distinguishes between outdated cache rebuild and fresh build.implementingTypeparameter.Testing
Keybinds
Server shutdown
Release build