Replay debug actions under the acting player's world view - #976
Conversation
A debug action replayed on another client re-derives whatever it needs from
local state. DebugSync already reproduces the cursor and the selected object,
but not which view the player had open -- and vanilla's incident action picks
its entire target from exactly that:
WorldSelected ? Find.WorldSelector.SingleSelectedObject : Find.CurrentMap
So a host on the planet with a caravan selected fires at the caravan, while a
client looking at a colony fires at the map. The node's label is built from the
target's name, so the two clients also disagree about the label and
RecreateGraphAndGetNode finds nothing -- the command is accepted, acknowledged,
and silently never executed there. A synchronized command that runs on one
client and not the other is a desync, and this one leaves no trace where it
happens.
Carry the flag in the debug command and override the getter during replay,
following the MouseCellPatch and MouseTilePatch pattern already used for the
cursor. Read it before the node graph is rebuilt, since labels depend on it,
and clear it in the same finally block as the others so it cannot leak into
normal play.
The debug command's wire format gains one byte; host and clients need matching
builds, which this mod already requires.
notfood
left a comment
notfood
left a comment
There was a problem hiding this comment.
I don't buy that we can't read the state of the scene and needs a patch to WorldRendererUtility. Double check that.
| public static class WorldSelectedPatch | ||
| { | ||
| /// <summary>Non-null only while a debug command is being replayed.</summary> | ||
| public static bool? result; |
There was a problem hiding this comment.
Something is wrong here, it's never assigned. Later it asks for .HasValue but it was never set.
There was a problem hiding this comment.
You are right that this file alone does not show it. The field does get set, but in the other file.
It is set here, in DebugSync.HandleCmd:
It is read from the network at that spot on purpose. The node graph gets rebuilt a few lines
later, and node labels can depend on the view. So the value has to be in place first.
It is set back to null in the finally block, next to the two cursor overrides:
Multiplayer/Source/Client/Debug/DebugSync.cs
Lines 122 to 124 in 17df8b3
This is the same pattern as the two patches right above it in Patches.cs. All three are set
and cleared in the same places:
| Field | Declared | Set | Cleared |
|---|---|---|---|
MouseCellPatch.result |
Patches.cs#L172 |
DebugSync.cs#L31 |
#L122 |
MouseTilePatch.result |
Patches.cs#L184 |
DebugSync.cs#L33 |
#L123 |
WorldSelectedPatch.result |
Patches.cs#L209 |
DebugSync.cs#L55 |
#L124 |
Still, you had to open another file to answer this. I can update it if you want to say where the value is set and cleared, so the class makes sense on its own.
My description was not clear, so let me explain what the patch is really for. We can read the state, and we do. The sending client reads it with no patch at all: Multiplayer/Source/Client/Debug/DebugSync.cs Line 169 in 17df8b3 Reading was never the problem. The patch is for the other side, when the command is replayed Now, the other option: set the real state instead of patching the getter. I checked that too. public static bool WorldSelected => CurrentWorldRenderMode == WorldRenderMode.Planet;
public static WorldRenderMode CurrentWorldRenderMode
{
get
{
...
if (Current.ProgramState == ProgramState.Playing && Find.CurrentMap == null)
return WorldRenderMode.Planet;
if (Find.World.renderer.wantedMode == WorldRenderMode.Planet)
return WorldRenderMode.Planet;
...
}
}It comes from two things on the local client:
The patch changes only the answer, only while one command is replayed, and clears right after Multiplayer/Source/Client/Patches/Patches.cs Lines 171 to 181 in 17df8b3 To replay where the mouse was, we override I can add this reasoning to the comment on the class if you want. Right now it says what the patch does If there is a way to do this that I missed, and it does not need a patch or move anything the |
2 participants
Firing a debug action desyncs the game whenever two players are looking at
different screens.
Root cause
DebugSyncreplays a debug action on every client by re-running it from a syncednode path. It reproduces the cursor position and the selected object so that
actions depending on them behave the same everywhere — but not which view the
acting player had open.
Vanilla's incident action derives its entire target from exactly that:
WorldRendererUtility.WorldSelectedis client-local camera state. So a host onthe planet with a caravan selected resolves
Caravan, while a client looking ata colony resolves
Find.CurrentMap.That breaks the replay twice over:
(
labelGetter = () => name + " (" + GetIncidentTargetLabel() + ")..."), sothe two clients disagree about the label.
RecreateGraphAndGetNodematcheschildren by
LabelAndCategory(), finds nothing, returnsnull, and thecaller skips execution. The command is accepted, acknowledged, and quietly
never runs on that client — with no log line where it happens.
A synchronized command that runs on some clients and not others is about the most
direct way to desync a lockstep simulation, and this one leaves no trace at the
point of failure, which is why the desync surfaces later somewhere unrelated.
WorldSelectedtruefalseGetTarget()CaravanFind.CurrentMapDo incident (Caravan Rim)...Do incident (Map)...RecreateGraphAndGetNodenullEvidence
Firing
GiveQuest_EndGame_ShipEscapewith the host on the planet and the clienton a colony map. Both traces are the first entry in their capture, i.e. the
divergence itself rather than its aftermath.
Host — executing the incident inside the synced command:
Client, same trace index — never entered the command at all, just ticking:
The same signature appears with
CaravanMeeting, diverging slightly earlier —while the debug action is still computing its parameters, before the worker runs:
The fix
Carry the flag in the debug command and override the getter during replay,
following the
MouseCellPatch/MouseTilePatchpattern already used for thecursor:
WorldSelectedPatch— getter postfix with a nullableresult, placed besidethe existing cursor overrides.
SendCmdwritesWorldRendererUtility.WorldSelected.HandleCmdreads it before rebuilding the node graph (labels depend on it)and clears it in the same
finallyblock as the cursor overrides, so it cannotleak into normal play.
Verification
Two connected clients,
asyncTimeoff, multifaction off:original repro. Fires at the caravan on both. No desync.
Find.Worldonboth. No desync.
GiveQuest_EndGame_ShipEscapefrom the world view — the trace above. Questletter arrives on host and client. No desync.
Build clean, 158/158 tests (no test-visible surface; the harness cannot construct
RimWorld UI types).
Notes
matching builds, which this mod already requires.
incidents.
[NO]in the incident list means "would not fire naturally", not "cannot beforced" — the action checks
TargetAllowedbut neverCanFireNow. That isunchanged here, and is why the
GiveQuestcase above executes at all.path cannot be resolved (that silent
nullis what made this expensive tofind), and keeping path resolution out of the simulation's random stream.