Skip to content

Fix UI ID drift, shuttle departure loop, and stack tracing overflow - #988

Open
Manaether wants to merge 4 commits into
rwmt:devfrom
Manaether:fix/core-engine-desync-guards
Open

Manaether wants to merge 4 commits into
rwmt:devfrom
Manaether:fix/core-engine-desync-guards

Conversation

@Manaether

Copy link
Copy Markdown

Summary

This pull request addresses three critical core and engine stability issues:

  1. Prevents simulation ID desyncs caused by off-tick UI passes (UIRoot_Play.UIRootOnGUI).
  2. Fixes an IndexOutOfRangeException in the desync stack tracing subsystem (DeferredStackTracingImpl.TraceImpl).
  3. Prevents an infinite NullReferenceException tick-loop during shuttle departures in Royalty.

🛠️ Changes

1. UniqueIDsManagerGuard (Source/Client/Patches/UniqueIDsManagerGuard.cs)

  • Problem: UI rendering passes (UIRoot_Play.UIRootOnGUI) often instantiate temporary items, jobs, or preview pawns off-tick. This consumes sequential IDs from Find.UniqueIDsManager on the local client only, leaving ID counters divergent between host and peers and causing immediate desyncs on subsequent ticks.
  • Solution: Takes a snapshot of all next* integer fields in Find.UniqueIDsManager before UIRootOnGUI and restores them via [HarmonyFinalizer] when the pass completes.
  • Zero Allocation: Field getters and setters are precompiled at startup via System.Linq.Expressions delegates, generating 0 heap allocations / 0 boxing per frame in OnGUI. Active simulation ticks and synchronized commands bypass the rollback.

2. Stack Tracing Bounds Check (Source/Common/DeferredStackTracingImpl.cs & Source/Client/Desyncs/DeferredStackTracing.cs)

  • Problem: In DeferredStackTracingImpl.TraceImpl, traceIn has a fixed capacity of 32. In heavily modded stacks or when encountering dynamic Harmony wrappers (nameHash == 0), depth was not incremented while index continued incrementing, causing traceIn[index] to throw System.IndexOutOfRangeException inside the postfix on Rand.Int / Rand.Value.
  • Solution: Injects a bounds check if (index >= traceIn.Length) break; in TraceImpl, and guards DeferredStackTracing.Postfix with a try-catch to isolate core RNG from any tracing errors.

3. Shuttle Departure Loop Fix (Source/Client/Patches/ShuttlePatches.cs)

  • Problem: In vanilla Royalty, if requiredPawns or requiredItems deserialize as null during save transfer or despawn, CompShuttle.SendLaunchedSignals throws an NRE inside ShipJob_FlyAway.TryStart. Because TryStart returns false, the shuttle re-attempts launch every tick, locking the game in an infinite exception loop.
  • Solution: Null-coalesces required collections in CompShuttle.PostExposeData and suppresses departure exceptions with a finalizer so the shuttle can take off cleanly.

✅ Verification

  • Built solution: dotnet build Source/Multiplayer.sln completes with 0 errors.
  • Verified zero ID drift across clients during UI interaction passes.

- UniqueIDsManagerGuard: Snapshot and restore UniqueIDsManager counters across UIRootOnGUI via compiled expression delegates to prevent off-tick UI passes from desyncing simulation IDs
- ShuttlePatches: Null-coalesce CompShuttle required collections and guard SendLaunchedSignals to prevent infinite takeoff NRE loop
- DeferredStackTracingImpl: Add bounds check in TraceImpl to prevent IndexOutOfRangeException on deep or inlined stack traces
- DeferredStackTracing: Guard Postfix with try-catch to prevent tracing exceptions from breaking Rand getters
- SourceGen: Align Microsoft.CodeAnalysis version with SDK compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant