Skip to content

Isolate RNG in PowerNet battery energy distribution (correct branch) - #987

Open
GetParanoid wants to merge 1 commit into
rwmt:devfrom
GetParanoid:fix/powernet-battery-shuffle-desync
Open

Isolate RNG in PowerNet battery energy distribution (correct branch)#987
GetParanoid wants to merge 1 commit into
rwmt:devfrom
GetParanoid:fix/powernet-battery-shuffle-desync

Conversation

@GetParanoid

Copy link
Copy Markdown

Originally merged into the wrong branch (#986), my apologies.

Problem

PowerNet.DistributeEnergyAmongBatteries shuffles the net's battery list (batteriesShuffled.Shuffle()), consuming calls from the map's shared RNG.

Whether it runs on a given tick is gated by a float comparison in PowerNet.ChangeStoredEnergy (extra > 0f), fed by CurrentEnergyGainRate() / CurrentStoredEnergy() summing floats across every power component and battery on the net. Those sums drift slightly between machines, so the comparison can flip a tick earlier on one peer than another: one peer runs the shuffle on a tick where the other doesn't, Rand.iterations ends the tick at a different value on each, and the map desyncs with Wrong random state on map 0.

Evidence

From a 2-player session (MP 0.11.5, RimWorld 1.6.4871, no async time, single map, no mods patching PowerNet): the client's and host's recorded Rand stack traces match call for call — same ticks, same entities, same states — through tick 6781265, then diverge.

Client — stays on tick 6781265 and makes 7 RNG calls the host never makes, all from the same stack:

Rand.RangeInclusive -> GenList.Shuffle -> PowerNet.DistributeEnergyAmongBatteries
  -> PowerNet.ChangeStoredEnergy -> PowerNet.PowerNetTick -> Map.MapPostTick

Host — makes none of those calls, advancing straight from tick 6781265 to 6781266 (an unrelated ChildcareUtility.ShouldWakeUpToAutofeedUrgent check).

The same signature recurred 20+ times across the session, typically every few thousand ticks, and reappeared after each automatic rejoin. DistributeEnergyAmongBatteries shows up as a real stack frame in the traces, so it isn't being inlined.

Happy to attach the full desync logs if that's useful.

Fix

Push/pop the RNG state around the method while in MP, matching the approach in #968.

This targets exactly what the desync check compares. AsyncTimeComp.Tick records randState once per map tick in its finally, and TryAddMapRandomState stores (uint)(state >> 32) — the high half of Rand.StateCompressed, i.e. Rand.iterations. So ClientSyncOpinion.CheckForDesync's map comparison is effectively "did both peers make the same number of RNG calls this tick?"

Rand.PushState() saves (seed, iterations) and PopState() restores both, so whatever the shuffle consumes is discarded before the tick boundary is recorded. A peer that runs the method and a peer that doesn't record the identical value.

Why isolating the RNG is safe here

The shuffled order cannot affect the outcome, so discarding the RNG it draws changes nothing:

  1. Every battery in a pass receives the same amount — either the smallest AmountCanAccept across the list, or an even share of the energy remaining. Never a per-position amount.
  2. AmountCanAccept depends only on the battery itself, never on the others, so no battery's share is influenced by what was processed before it.
  3. The removal predicate is position-independent (amountCanAccept <= 0f || amountCanAccept == num2), so the same batteries drop out of the list under any ordering.
  4. energy is decremented by the same value the same number of times, so the float arithmetic is order-independent too.

Every permutation therefore leaves the batteries holding identical energy.

Two further notes:

  • PushState() with no seed continues from the current state rather than reseeding, so in-sync peers still produce the same shuffle as each other. Behaviour is unchanged beyond the isolation.
  • Singleplayer is untouched — the patch returns early when Multiplayer.Client == null.

Scope

PowerNet.PowerNetTick has two other float-gated RNG consumers — partsWantingPowerOn.RandomElement() and potentialShutdownParts.RandomElement() — reachable depending on how num2 + num >= -1E-07f lands. Both are left alone: their results decide which building powers on or shuts down, so isolating that RNG would make peers pick different buildings and diverge for real. The battery shuffle is the one consumer in this path whose result provably doesn't affect the outcome, which is what makes it safe to isolate. The other two also sit behind tick-interval and non-empty-list conditions, so they fire far more rarely than the shuffle, which runs whenever a net has surplus energy.

This doesn't eliminate the underlying float drift — that's inherent to float math across machines, and the desync check doesn't look at it. It removes the drift's ability to become a random-state mismatch.

Testing

  • Compiles clean against dev, no new warnings.
  • The diagnosis is confirmed in-game. Patching DistributeEnergyAmongBatteries no longer draws from the shared RNG, took a 2-player colony from desyncing and rejoining every few thousand ticks to roughly 3 hours with no desync at all.

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