Skip to content

Commit b672556

Browse files
Fix cross-clock UI/sound artifacts at mixed speeds
Three composable mechanisms behind the remaining mixed-speed flicker and re-firing notification sounds: World-tick faction restore hardened: PreContext swaps every map onto the spectator faction's data (resource counter, zone/area/designation managers) and the restore was conditional on the popped context, with SetFaction silently no-oping when a map lacks the faction's data. Any leak left maps on spectator data, alternating with frames that ran a world tick - which only happens at mixed speeds. Restore now falls back to the local player faction (identical to a balanced pop), the skip warns once per map/faction, and a log-only diagnostic reports residue at UI time. Sustainer/sample cross-clock lifetime: most sustainers have no map (the constructor downgrades defs without world sub-sounds to OnCamera), so their tick math ran under the viewed map's clock while Maintain() stamped other clocks - a viewer clock ahead of the stamp ends and respawns the sustainer every frame. Sound brackets now fall back to the world clock for map-less sounds; a tolerant end-check re-stamps tick-stale sustainers that were realtime-maintained within 1s (truly abandoned ones still die, at most 1s late); the one-shot reaper runs under the world clock so a paused viewer keeps reaping. Frame-order and tween: Root_Play.Update runs RealTime/portraits/ UIRootUpdate before the frame's viewer clock was installed - install it at frame start too; PawnTweener brackets carried pawns with MapHeld instead of the null Map. All render/audio/UI-side; no simulation-visible changes.
1 parent b1fc85e commit b672556

6 files changed

Lines changed: 241 additions & 5 deletions

File tree

Source/Client/AsyncTime/AsyncWorldTimeComp.cs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,18 @@ public void PostContext()
237237
{
238238
if (Multiplayer.GameComp.multifaction)
239239
{
240-
var f = FactionExtensions.PopFaction();
240+
// Restore must be unconditional: PreContext swapped EVERY map onto
241+
// the spectator faction's data (resourceCounter, zone/area/
242+
// designation managers), and leaving any map on it corrupts every
243+
// UI read until something else swaps it back (alternating only on
244+
// frames that ran a world tick - a per-frame flicker/re-ping
245+
// shape). A balanced pop returns the pre-world-tick OfPlayer,
246+
// which is the client-local faction, so falling back to
247+
// RealPlayerFaction on an unbalanced pop restores the same thing
248+
// the balanced path would have. This is the faction half of the
249+
// same PreContext/PostContext imbalance whose speed half caused
250+
// the paused-map time-context bug.
251+
var f = FactionExtensions.PopFaction() ?? Multiplayer.RealPlayerFaction;
241252
foreach (var map in Find.Maps)
242253
map.MpComp().SetFaction(f);
243254
}

Source/Client/AsyncTime/SetMapTime.cs

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,11 @@ static class PreDrawPosCalculationMapTime
139139
static void Prefix(PawnTweener __instance, ref TimeSnapshot? __state)
140140
{
141141
if (Multiplayer.Client == null || Current.ProgramState != ProgramState.Playing) return;
142-
__state = TimeSnapshot.GetAndSetFromMap(__instance.pawn.Map);
142+
// MapHeld, not Map: a carried/transported pawn has no map of its
143+
// own, and with no install its tween ran under the viewer's clock
144+
// (stamping lastDrawTick with it), then hard-snapped when a real
145+
// clock returned. The holder's map clock is the motion's basis.
146+
__state = TimeSnapshot.GetAndSetFromMap(__instance.pawn.MapHeld);
143147
}
144148

145149
static void Postfix(TimeSnapshot? __state) => __state?.Set();
@@ -169,7 +173,16 @@ static IEnumerable<MethodBase> TargetMethods()
169173
static void Prefix(Sustainer __instance, ref TimeSnapshot? __state)
170174
{
171175
if (Multiplayer.game == null) return;
172-
__state = TimeSnapshot.GetAndSetFromMap(__instance.info.Maker.Map);
176+
// Most sustainers have no map: the Sustainer constructor
177+
// downgrades any def without world sub-sounds to OnCamera, making
178+
// Maker invalid. Falling through with no install left those
179+
// running under the viewed map's clock while Maintain() stamps
180+
// the maintainer's ambient tick - a cross-clock pair that ends
181+
// (and respawns) the sustainer every frame when the viewer's
182+
// clock is ahead. The world clock is the only session-wide
183+
// monotone basis, so install it for map-less sound code.
184+
__state = TimeSnapshot.GetAndSetFromMap(__instance.info.Maker.Map)
185+
?? TimeSnapshot.GetAndSetFromWorld();
173186
}
174187

175188
static void Postfix(TimeSnapshot? __state) => __state?.Set();
@@ -181,12 +194,36 @@ static class SampleUpdateMapTime
181194
static void Prefix(Sample __instance, ref TimeSnapshot? __state)
182195
{
183196
if (Multiplayer.game == null) return;
184-
__state = TimeSnapshot.GetAndSetFromMap(__instance.Map);
197+
// World-clock fallback for map-less samples - see
198+
// SustainerUpdateMapTime above
199+
__state = TimeSnapshot.GetAndSetFromMap(__instance.Map)
200+
?? TimeSnapshot.GetAndSetFromWorld();
185201
}
186202

187203
static void Postfix(TimeSnapshot? __state) => __state?.Set();
188204
}
189205

206+
// The one-shot reaper decides "finished" partly from Find.TickManager.Paused,
207+
// which under the viewer install is the VIEWED map's pause state: a paused
208+
// viewer never reaps finished tempo-affected one-shots, they accumulate, and
209+
// the voice limiter then cuts/restarts sounds on every new play. The world
210+
// clock pauses only when the whole session does, so reaping tracks actual
211+
// sim activity. Per-sample Update calls nest their own map/world snapshots
212+
// inside this bracket (SampleUpdateMapTime).
213+
[HarmonyPatch(typeof(SampleOneShotManager), nameof(SampleOneShotManager.SampleOneShotManagerUpdate))]
214+
static class OneShotReaperWorldTime
215+
{
216+
[HarmonyPriority(MpPriority.MpFirst)]
217+
static void Prefix(ref TimeSnapshot? __state)
218+
{
219+
if (Multiplayer.game == null) return;
220+
__state = TimeSnapshot.GetAndSetFromWorld();
221+
}
222+
223+
[HarmonyPriority(MpPriority.MpLast)]
224+
static void Finalizer(TimeSnapshot? __state) => __state?.Set();
225+
}
226+
190227
[HarmonyPatch(typeof(TipSignal), MethodType.Constructor, new[] { typeof(Func<string>), typeof(int) })]
191228
static class TipSignalCtor
192229
{

Source/Client/Comp/Map/MultiplayerMapComp.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Multiplayer.Client.Factions;
66
using Multiplayer.Client.Persistent;
77
using Multiplayer.Client.Saving;
8+
using Multiplayer.Client.Util;
89
using Multiplayer.Common;
910
using RimWorld;
1011
using RimWorld.Planet;
@@ -121,10 +122,22 @@ public void DoTick()
121122
}
122123
}
123124

125+
private static readonly HashSet<long> warnedMissingFactionData = new();
126+
124127
public void SetFaction(Faction faction)
125128
{
126129
if (!factionData.TryGetValue(faction.loadID, out FactionMapData data))
130+
{
131+
// Skipping the swap leaves the map on whatever faction's data
132+
// is currently installed - if that was a transient context
133+
// (e.g. the world tick's spectator swap), the map stays wrong
134+
// until the next successful SetFaction. Never fail this
135+
// silently: name the map and faction once so a leak is
136+
// attributable.
137+
if (warnedMissingFactionData.Add(((long)map.uniqueID << 32) | (uint)faction.loadID))
138+
MpLog.Warn($"SetFaction skipped: map {map.uniqueID} has no FactionMapData for faction {faction.loadID} ({faction.Name}) - map keeps the previously installed faction data");
127139
return;
140+
}
128141

129142
map.designationManager = data.designationManager;
130143
map.areaManager = data.areaManager;
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
using HarmonyLib;
2+
using Multiplayer.Client.Util;
3+
using RimWorld;
4+
using UnityEngine;
5+
using Verse;
6+
7+
namespace Multiplayer.Client.Patches
8+
{
9+
// Diagnostic only. The world tick swaps every map onto the spectator
10+
// faction's data and restores afterwards; if any path leaves a map on the
11+
// wrong faction's managers, UI reads (resource readout, zones, areas,
12+
// low-food alert nutrition) alternate between factions' data on frames
13+
// that ran a world tick - a flicker/re-ping shape that only exists at
14+
// mixed speeds. The restore is hardened now, but this check catches ANY
15+
// residue source red-handed at UI time, named and counted. Log-only.
16+
[HarmonyPatch(typeof(UIRoot_Play), nameof(UIRoot_Play.UIRootUpdate))]
17+
static class FactionResidueDiag
18+
{
19+
private static int residueFrames;
20+
private static float lastReport;
21+
private const float ReportIntervalSeconds = 60f;
22+
23+
static void Postfix()
24+
{
25+
if (Multiplayer.Client == null || !Multiplayer.GameComp.multifaction) return;
26+
27+
var map = Find.CurrentMap;
28+
var realFaction = Multiplayer.RealPlayerFaction;
29+
if (map == null || realFaction == null) return;
30+
31+
var comp = map.MpComp();
32+
if (comp == null || !comp.factionData.TryGetValue(realFaction.loadID, out var ownData)) return;
33+
34+
if (ReferenceEquals(map.resourceCounter, ownData.resourceCounter)) return;
35+
36+
residueFrames++;
37+
var now = Time.realtimeSinceStartup;
38+
if (now - lastReport < ReportIntervalSeconds) return;
39+
lastReport = now;
40+
41+
var installedFactionId = -1;
42+
foreach (var kv in comp.factionData)
43+
if (ReferenceEquals(map.resourceCounter, kv.Value.resourceCounter))
44+
installedFactionId = kv.Key;
45+
46+
MpLog.Warn(
47+
$"Faction residue: map {map.uniqueID} has faction {installedFactionId}'s data installed at UI time " +
48+
$"(local faction {realFaction.loadID}, OfPlayer {Faction.OfPlayer?.loadID}); " +
49+
$"{residueFrames} affected frames since last report");
50+
residueFrames = 0;
51+
}
52+
}
53+
}
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
using System.Collections.Generic;
2+
using HarmonyLib;
3+
using Multiplayer.Client.Util;
4+
using UnityEngine;
5+
using Verse;
6+
using Verse.Sound;
7+
8+
namespace Multiplayer.Client.Patches
9+
{
10+
// Sustainers with PerTick/PerTickRare maintenance compare an ambient tick
11+
// against lastMaintainTick, but under per-map clocks the stamp and the
12+
// check can come from different clocks: Maintain() runs under whatever
13+
// context the maintainer had (often a map's tick loop), while
14+
// SustainerUpdate runs under the sound bracket's clock (owner map, or the
15+
// world clock for map-less sustainers). A stamp clock behind the check
16+
// clock reads as "unmaintained" and ends the sustainer even though its
17+
// owner maintained it microseconds ago - and owners respawn ended
18+
// sustainers, so the sound restarts every frame.
19+
//
20+
// Fix: before the vanilla staleness check runs, if the sustainer LOOKS
21+
// tick-stale but was really maintained within the grace window (measured
22+
// in realtime, which no clock swap can touch), re-stamp it under the
23+
// clock the check will use. Truly abandoned sustainers still die, at most
24+
// GraceSeconds late - audio-only, imperceptible. PerFrame maintenance is
25+
// untouched (frame counts are global). Vanilla paused behavior is
26+
// untouched (a frozen clock never reads as stale).
27+
public static class SustainerRealtimeMaintenance
28+
{
29+
public static readonly Dictionary<Sustainer, float> lastMaintainRealTime = new();
30+
31+
public const float GraceSeconds = 1f;
32+
33+
private static float lastRescueReport;
34+
private static int rescueCount;
35+
36+
public static void NoteRescue(Sustainer sustainer, int ambientTicks, int staleStamp)
37+
{
38+
rescueCount++;
39+
var now = Time.realtimeSinceStartup;
40+
if (now - lastRescueReport < 60f) return;
41+
lastRescueReport = now;
42+
MpLog.Debug($"Sustainer cross-clock rescue: {sustainer.def} ambient={ambientTicks} stamp={staleStamp} ({rescueCount} rescues since last report)");
43+
rescueCount = 0;
44+
}
45+
}
46+
47+
[HarmonyPatch(typeof(Sustainer), MethodType.Constructor, typeof(SoundDef), typeof(SoundInfo))]
48+
static class SustainerCtorRealtimeStamp
49+
{
50+
static void Postfix(Sustainer __instance)
51+
{
52+
if (Multiplayer.Client == null) return;
53+
SustainerRealtimeMaintenance.lastMaintainRealTime[__instance] = Time.realtimeSinceStartup;
54+
}
55+
}
56+
57+
[HarmonyPatch(typeof(Sustainer), nameof(Sustainer.Maintain))]
58+
static class SustainerMaintainRealtimeStamp
59+
{
60+
static void Postfix(Sustainer __instance)
61+
{
62+
if (Multiplayer.Client == null) return;
63+
SustainerRealtimeMaintenance.lastMaintainRealTime[__instance] = Time.realtimeSinceStartup;
64+
}
65+
}
66+
67+
[HarmonyPatch(typeof(Sustainer), nameof(Sustainer.End))]
68+
static class SustainerEndCleanup
69+
{
70+
static void Postfix(Sustainer __instance)
71+
=> SustainerRealtimeMaintenance.lastMaintainRealTime.Remove(__instance);
72+
}
73+
74+
[HarmonyPatch(typeof(Sustainer), nameof(Sustainer.SustainerUpdate))]
75+
static class SustainerTolerantEndCheck
76+
{
77+
// Runs after SustainerUpdateMapTime's prefix so the re-stamp uses the
78+
// same clock the vanilla staleness check is about to read
79+
[HarmonyPriority(Priority.Low)]
80+
static void Prefix(Sustainer __instance)
81+
{
82+
if (Multiplayer.Client == null || __instance.Ended) return;
83+
84+
var maintenance = __instance.info.Maintenance;
85+
int staleAfter;
86+
if (maintenance == MaintenanceType.PerTick)
87+
staleAfter = 1;
88+
else if (maintenance == MaintenanceType.PerTickRare)
89+
staleAfter = 250;
90+
else
91+
return;
92+
93+
var ambientTicks = Find.TickManager.TicksGame;
94+
if (ambientTicks <= __instance.lastMaintainTick + staleAfter)
95+
return; // not stale, vanilla keeps it
96+
97+
if (!SustainerRealtimeMaintenance.lastMaintainRealTime.TryGetValue(__instance, out var maintainedAt) ||
98+
Time.realtimeSinceStartup - maintainedAt > SustainerRealtimeMaintenance.GraceSeconds)
99+
return; // truly abandoned, let vanilla end it
100+
101+
SustainerRealtimeMaintenance.NoteRescue(__instance, ambientTicks, __instance.lastMaintainTick);
102+
__instance.lastMaintainTick = ambientTicks;
103+
}
104+
}
105+
}

Source/Client/Patches/TickPatch.cs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ static void Postfix()
198198
// and every sim path installs its own context on entry (the world
199199
// tick installs its own count), so this value is never a simulation
200200
// input.
201-
private static void InstallViewerTimeContext()
201+
internal static void InstallViewerTimeContext()
202202
{
203203
// Null checks cover the join/load window where Multiplayer.game
204204
// (and with it the async comps) lags Multiplayer.Client
@@ -371,6 +371,23 @@ public static void Reset()
371371
public static ITickable TickableById(int tickableId) => AllTickables.FirstOrDefault(t => t.TickableId == tickableId);
372372
}
373373

374+
// Root_Play.Update runs RealTime.Update, PortraitsCache and UIRootUpdate
375+
// BEFORE TickManagerUpdate, where the frame's viewer context is normally
376+
// installed - so those consumers read the PREVIOUS frame's residual
377+
// ambient (e.g. unpausedTime advances by a stale TickRateMultiplier,
378+
// making pausable-animated materials move in bursts). Install the viewer
379+
// context at the top of the frame too; the sim still installs its own
380+
// contexts on entry, so this is render/UI-only like the post-tick install.
381+
[HarmonyPatch(typeof(Root_Play), nameof(Root_Play.Update))]
382+
static class FrameStartViewerContext
383+
{
384+
static void Prefix()
385+
{
386+
if (Multiplayer.Client == null) return;
387+
TickPatch.InstallViewerTimeContext();
388+
}
389+
}
390+
374391
public class SimulatingData
375392
{
376393
public int? target;

0 commit comments

Comments
 (0)