Skip to content

Fix QualityBuilder NRE when its designator field became non-static #602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pkp24 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix QualityBuilder NRE when its designator field became non-static #602

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 60 additions & 15 deletions Source/Mods/QualityBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using HarmonyLib;
using Multiplayer.API;
using Verse;
Expand All @@ -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<Designator, QualityCategory?> 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)

@notfood notfood Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failing silently here. Just let the whole thing fail with NPE.

MP.RegisterSyncMethod(builderCompType, "ToggleSkilled");

//my decompiler shows the method name is <get_RightClickFloatMenuOptions>b__3_0
//while harmony saids it is <get_RightClickFloatMenuOptions>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<object>(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<object>(SyncTypes, toggleCommandType);
MP.RegisterSyncWorker<object>(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)

@notfood notfood Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assume it exists, and let it fail if it doesn't.

{
desiredQualityField = AccessTools.FieldRefAccess<QualityCategory?>(designatorType, "curQualityCat");
MP.RegisterSyncWorker<Designator>(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<bool>())
desiredQualityField(designator) = sync.Read<QualityCategory>();
}
}

void SyncTypes(SyncWorker sync, ref object c)
Expand Down