From f5ec6b3f4569237569d09d27667688c70ef3926e Mon Sep 17 00:00:00 2001 From: pkp24 <37089940+pkp24@users.noreply.github.com> Date: Sat, 11 Jul 2026 11:21:21 -0400 Subject: [PATCH] Fix QualityBuilder NRE when its designator field became non-static MpCompatLoader logged "Exception loading hatti.qualitybuilder: System.NullReferenceException" on startup with QualityBuilder installed. Root cause: the patch resolved the skilled-builder designator's right-click "set min quality" lambda by hard-coding the compiler-generated closure type name "QualityBuilder._Designator_SkilledBuilder+<>c". That cached <>c closure only exists while the lambda captures nothing. A recent QualityBuilder change made the designator's quality field (curQualityCat) a per-instance field instead of static, so the lambda now captures `this` and the compiler emits it as an instance method on the designator itself -- the <>c type no longer exists. AccessTools.TypeByName then returned null, and GetFirstMethodBySignature(null, ...) dereferenced it, throwing the NRE and aborting the whole compat class (so ToggleSkilled and the gizmo menu weren't synced either). Fix: sync the designator's state instead of the choice-setting lambda. Register a SyncWorker for _Designator_SkilledBuilder that serializes curQualityCat, so MP replays each designation with the chosen quality and every client builds it at the same quality -- the same idiom other modded designators use (e.g. Dubs Bad Hygiene). This is also the only mechanism that still carries the quality after the refactor: the old approach synced the pick into a *static* field, but curQualityCat is now per-instance, so syncing the pick alone would no longer reach the designation (a latent desync even once the NRE is fixed). Each player's own menu pick stays local to their designator; the applied quality is what syncs. Also null-guard the three TypeByName lookups so a future internal rename in QualityBuilder degrades to a warning instead of taking down the compat class. Co-Authored-By: Claude Opus 4.8 (1M context) --- Source/Mods/QualityBuilder.cs | 75 ++++++++++++++++++++++++++++------- 1 file changed, 60 insertions(+), 15 deletions(-) diff --git a/Source/Mods/QualityBuilder.cs b/Source/Mods/QualityBuilder.cs index f54646b9..49fc2065 100644 --- a/Source/Mods/QualityBuilder.cs +++ b/Source/Mods/QualityBuilder.cs @@ -1,4 +1,4 @@ -using System; +using System; using HarmonyLib; using Multiplayer.API; using Verse; @@ -12,27 +12,72 @@ namespace Multiplayer.Compat [MpCompatFor("hatti.qualitybuilder")] class QualityBuilder { + // The skilled-builder designator remembers the min-quality the player picked from its + // right-click menu in an instance field (curQualityCat). This ref lets us serialize that + // choice so a designation resolves to the same quality on every client. + private static AccessTools.FieldRef desiredQualityField; + public QualityBuilder(ModContentPack mod) { - Type builderCompType = AccessTools.TypeByName("QualityBuilder.CompQualityBuilder"); - Type toggleCommandType = AccessTools.TypeByName("QualityBuilder.CompQualityBuilder+ToggleCommand+<>c"); - Type designatorType = AccessTools.TypeByName("QualityBuilder._Designator_SkilledBuilder+<>c"); - Type[] argTypes = { typeof(QualityCategory) }; - MethodInfo toggleCommandMethod = MpMethodUtil.GetFirstMethodBySignature(toggleCommandType, argTypes); - MethodInfo designatorMethod = MpMethodUtil.GetFirstMethodBySignature(designatorType, argTypes); + // --- "Skilled" toggle gizmo on the building comp --- + Type builderCompType = AccessTools.TypeByName("QualityBuilder.CompQualityBuilder"); + if (builderCompType != null) + MP.RegisterSyncMethod(builderCompType, "ToggleSkilled"); - //my decompiler shows the method name is b__3_0 - //while harmony saids it is b__2_0, weird..." + // The gizmo's right-click "set min quality" lambda is non-capturing, so it still lives in a + // cached <>c closure. Match it by signature rather than by a brittle lambda ordinal. + Type toggleCommandType = AccessTools.TypeByName("QualityBuilder.CompQualityBuilder+ToggleCommand+<>c"); + MethodInfo toggleCommandMethod = toggleCommandType != null + ? MpMethodUtil.GetFirstMethodBySignature(toggleCommandType, argTypes) + : null; + if (toggleCommandMethod != null) + { + MP.RegisterSyncWorker(SyncTypes, toggleCommandType); + MP.RegisterSyncMethod(toggleCommandMethod).SetContext(SyncContext.MapSelected); + } + else + { + Log.Warning("MPCompat :: QualityBuilder: couldn't resolve the ToggleCommand right-click sync method; the min-quality gizmo menu won't be synced."); + } - MP.RegisterSyncWorker(SyncTypes, toggleCommandType); - MP.RegisterSyncWorker(SyncTypes, designatorType); + // --- "Skilled builder" designator (right-click min-quality choice) --- + // Previously this synced the choice-setting lambda through its <>c closure. QualityBuilder made + // curQualityCat a non-static (per-instance) field, so that lambda now captures 'this' and the + // <>c closure no longer exists -- looking it up returned null and NRE'd the whole patch. Instead, + // sync the designator's state directly: MP replays the designation with this worker, so every + // client resolves the same desired quality. This mirrors how other modded designators are synced + // (e.g. Dubs Bad Hygiene) and is robust to that closure refactor. + Type designatorType = AccessTools.TypeByName("QualityBuilder._Designator_SkilledBuilder"); + if (designatorType != null) + { + desiredQualityField = AccessTools.FieldRefAccess(designatorType, "curQualityCat"); + MP.RegisterSyncWorker(SyncSkilledDesignator, designatorType, shouldConstruct: true); + } + else + { + Log.Warning("MPCompat :: QualityBuilder: couldn't find _Designator_SkilledBuilder; its designations won't be synced."); + } + } - MP.RegisterSyncMethod(builderCompType, "ToggleSkilled"); - MP.RegisterSyncMethod(toggleCommandMethod).SetContext(SyncContext.MapSelected); - MP.RegisterSyncMethod(designatorMethod); - + // Serializes the skilled-builder designator's chosen min quality (a nullable QualityCategory). + // The designator is freshly constructed (shouldConstruct: true), so curQualityCat starts null; + // only overwrite it when the writer actually had a value. + private static void SyncSkilledDesignator(SyncWorker sync, ref Designator designator) + { + if (sync.isWriting) + { + QualityCategory? chosen = desiredQualityField(designator); + sync.Write(chosen.HasValue); + if (chosen.HasValue) + sync.Write(chosen.Value); + } + else + { + if (sync.Read()) + desiredQualityField(designator) = sync.Read(); + } } void SyncTypes(SyncWorker sync, ref object c)