Skip to content

Commit

Permalink
Rework FixNullMotes patch
Browse files Browse the repository at this point in the history
This should fix rwmt#447, rendering rwmt#481 unnecessary. I've decided to still leave that code as an extra safety precaution, but it should be now safe to revert those changes.

First, FixNullMotes patch was moved from generic "Patches" to "Determinism", as the patch itself changes some non-deterministic behavior.

As fot the changes, this should fix issues with Bioferrite Harvester and Electroharvester cables not creating motes after switching maps.

The current `FixNullMotes` patch would create a single mote per mote's `ThingDef.thingClass` and then reuse it. It ensured that the call to `MoteMaker.MakeStaticMote` never produced null, however the mote itself was more of a dummy mote (not initialized with any data).

This caused issue with the cables that would periodically attempt to spawn motes. If the mote would not be possible to spawn (drawn off-screen/on a different map), the MP patch would return the cached mote. The cable connection comp would then keep that mote forever, as it can only discard it if the mote is null or destroyed (which would never happen with the dummy note). Because of this, our dummy mote never ended up being discarded by the comp making the cable never draw another mote.

This PR ensures this doesn't happen by reworking "FixNullMotes" to actually spawn the motes, rather than returning a dummy mote. This is done by:
- Setting `makeOffscreen` to true in a prefix, skipping current map check
- Adding `& false` to `MoteCounter.Saturated` calls, allowing to spawn motes even if the mote counter is saturated
The check for off-screen motes was left as-is, as that one should be deterministic and safe.

Additional change call `Initialize` for both `Building_BioferriteHarvester` and `Building_Electroharvester`. Without it, the cables for those 2 aren't created after loading into the game (they're only created when rendered or a connection is added/removed). Since the cables aren't created the motes can't be created either, causing a discrepancy between players (unless they all had the cables created at the same time).

A final note on the original patch - it seems that it was made to fix desyncs caused by motes spawned by `CompAbilityEffect_Waterskip` (and potentially other issues at the time, but I can't find any mention of those). However, this specific issue seems to no longer be relevant as the waterskip now uses flecks rather than motes.
  • Loading branch information
SokyranTheDragon committed Jul 29, 2024
1 parent 49b6f5a commit 7e3117e
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 25 deletions.
67 changes: 67 additions & 0 deletions Source/Client/Patches/Determinism.cs
Original file line number Diff line number Diff line change
Expand Up @@ -587,4 +587,71 @@ static void Postfix(UndercaveMapComponent __instance)
}
}

[HarmonyPatch(typeof(MoteMaker), nameof(MoteMaker.MakeStaticMote))]
[HarmonyPatch([typeof(Vector3), typeof(Map), typeof(ThingDef), typeof(float), typeof(bool), typeof(float)])]
static class FixNullMotes
{
// Make sure that motes will (almost) always spawn. We skip based to player-specific
// data, and instead only allow the only fully deterministic checks (location is in map bounds).
// We skip checks to current map and current mote counter saturation, as those may vary between players.

static void Prefix(ref bool makeOffscreen)
{
// Skip a call to GenView.ShouldSpawnMotesAt, forcing
// each call to check GenGrid.InBounds instead.
// ShouldSpawnMotesAt would check for bounds, but also
// include a current map check as well, which we don't want.
makeOffscreen = true;
}

static IEnumerable<CodeInstruction> Transpiler(IEnumerable<CodeInstruction> instr)
{
var targetCall = AccessTools.DeclaredPropertyGetter(typeof(MoteCounter), nameof(MoteCounter.Saturated));

foreach (var ci in instr)
{
yield return ci;

// Add "& false" to any call to MoteCounter.Saturated to get a deterministic result.
// Not a perfect solution performance-wise, but will prevent desyncs due to
// MoteMaker.MakeStaticMote being non-deterministic without this change.
if (ci.Calls(targetCall))
{
yield return new CodeInstruction(OpCodes.Ldc_I4_0);
yield return new CodeInstruction(OpCodes.And);
}
}
}
}

[HarmonyPatch(typeof(Building_BioferriteHarvester), nameof(Building_BioferriteHarvester.SpawnSetup))]
static class AlwaysRebuildBioferriteHarvesterCables
{
static void Postfix(Building_BioferriteHarvester __instance)
{
// After reloading the game the initialize method will generally be called
// during rendering, ensure it's called on respawning as well.
if (!__instance.initalized)
{
// We need to call ExecuteWhenFinished, as it will crash otherwise.
LongEventHandler.ExecuteWhenFinished(__instance.Initialize);
}
}
}

[HarmonyPatch(typeof(Building_Electroharvester), nameof(Building_Electroharvester.SpawnSetup))]
static class AlwaysRebuildElectroharvesterCables
{
static void Postfix(Building_Electroharvester __instance)
{
// After reloading the game the initialize method will generally be called
// during rendering, ensure it's called on respawning as well.
if (!__instance.initalized)
{
// We need to call ExecuteWhenFinished, as it will crash otherwise.
LongEventHandler.ExecuteWhenFinished(__instance.Initialize);
}
}
}

}
25 changes: 0 additions & 25 deletions Source/Client/Patches/Patches.cs
Original file line number Diff line number Diff line change
Expand Up @@ -426,31 +426,6 @@ internal static void Choose(QuestPart_Choice part, int index)
}
}

[HarmonyPatch(typeof(MoteMaker), nameof(MoteMaker.MakeStaticMote))]
[HarmonyPatch(new[] {typeof(Vector3), typeof(Map), typeof(ThingDef), typeof(float), typeof(bool), typeof(float)})]
static class FixNullMotes
{
static Dictionary<Type, Mote> cache = new();

static void Postfix(ThingDef moteDef, ref Mote __result) {
if (__result != null) return;

if (moteDef.mote.needsMaintenance) return;

var thingClass = moteDef.thingClass;

if (cache.TryGetValue(thingClass, out Mote value)) {
__result = value;
} else {
__result = (Mote) Activator.CreateInstance(thingClass);

cache.Add(thingClass, __result);
}

__result.def = moteDef;
}
}

[HarmonyPatch(typeof(DiaOption), nameof(DiaOption.Activate))]
static class NodeTreeDialogSync
{
Expand Down

0 comments on commit 7e3117e

Please sign in to comment.