Fix QualityBuilder NRE when its designator field became non-static#602
Open
pkp24 wants to merge 1 commit into
Open
Fix QualityBuilder NRE when its designator field became non-static#602pkp24 wants to merge 1 commit into
pkp24 wants to merge 1 commit into
Conversation
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<Designator> 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) <noreply@anthropic.com>
|
I can confirm that this commit fixes MP Compat for Qualitybuilder Unofficial 1.6 7/10 update, which otherwise does not work properly and throws NRE at launch. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Code by claude code, multiple people who use MPCompat have said this change seems to work fine. I haven't personally tested it since I don't play multiplayer.
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
thisand 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.