Repository navigation
Bundle Valheim Endgame and tablet compatibility in PraetorisClient 0.1.102 - #145
Conversation
|
Now I have a clear picture of the diff. Let me write the review. Blocking issues found.
|
| Arg slot | Value |
|---|---|
Ldarg_0 |
this — the EnchantingUIController instance |
Ldarg_1 |
item — ItemDrop.ItemData |
Ldarg_2 |
rarity — ItemRarity |
The transpiler inserts Ldarg_0 immediately before the redirected Call PreviewSocketWeights(int rarity, ItemDrop.ItemData item). The full stack at the call site becomes [rarity_enum, this], so the controller object is handed to the item parameter. At runtime the CLR will throw an InvalidCastException (or Mono will produce undefined behavior) because EnchantingUIController is not an ItemDrop.ItemData. This would crash or corrupt the enchanting preview dialog for every item, not just tablets.
For comparison, the analogous transpiler PreventRolledSockets correctly uses Ldarg_1 because RollMagicItem is a static method on LootRoller where slot 0 is rarity and slot 1 is item. The HideSocketPreview case needs Ldarg_1 for the same reason—the item occupies the second argument slot.
The test harness (tests/EndgameCompatibility/Program.cs lines 380–393) only verifies that PreviewSocketWeights appears once in the rewritten instruction stream; it does not execute the patched method with real arguments in a Unity context, so this type mismatch is not caught by the 51 passing checks.
Fix: Change new(OpCodes.Ldarg_0) to new(OpCodes.Ldarg_1) in HideSocketPreview.
Other observations (non-blocking)
PreventPlacement checks __0 (first item parameter) — CanSocket and CanCoexist each receive two ItemDrop.ItemData parameters. The patch checks __0. Whether __0 is the receiving item or the socket shard depends on Epic Loot's parameter convention, which is not verified by the test (lines 419–422 only confirm the reason ref-string parameter exists). If __0 is the shard/stone and __1 is the target item, tablets could still receive socket operations. This is worth confirming against Epic Loot's source before the live proof step.
Live validation was not completed — SSH key agent failure prevented baseline reproduction, candidate deployment, gameplay, and saved-item migration proof (PR description). The README's "Remaining live proof" checklist (steps 1–7) is fully outstanding. This PR should not be treated as production-ready, as stated.
|
I have everything I need. Here is my review: Blocking issues found.
|
|
No blocking issues found. Reviewed areas: Transpiler stack reasoning
The
Checks both field presence (
Applied in both the native-support path and the legacy-hash path; correctly withheld for unsupported binaries (the explicit warning log communicates this). The Packaging
Nit: |
|
I have enough context now to write the review. No blocking issues found.
The gating logic is structurally sound and deliberately mirrors The sequence in The
The hash guard at line 366–372 correctly gates all legacy IL patches to the exact known binary ( The
The |
|
Now I have enough context. Let me write the review. Blocking issues found.
const string prefix = "ve_tablet_tier";
return name != null && name.Length == prefix.Length + 2 && // requires exactly 16 chars
name.StartsWith(prefix, StringComparison.Ordinal) &&
int.TryParse(name.Substring(prefix.Length), NumberStyles.None, ..., out int tier) &&
tier >= 1 && tier <= 15;
The sub-check If
The PR's stated goal is "prevent new tablet sockets and shardstones." Whether this gap manifests today depends on the item names inside the bundled Fix: accept both one- and two-character suffixes, or accept the prefix as a minimum rather than an exact length: return name != null && name.Length > prefix.Length &&
name.StartsWith(prefix, StringComparison.Ordinal) &&
int.TryParse(name.Substring(prefix.Length), NumberStyles.None, CultureInfo.InvariantCulture, out int tier) &&
tier >= 1 && tier <= 15;Everything else reviewed without blocking findings
|
Bundle Valheim Endgame 0.5.0 with PraetorisClient 0.1.102, add compatibility with released Epic Loot 0.14.13, and restrict Map Room entry to approved beta players.
[EndgameBeta] AllowedSteamIdson the server before the exteriorMWL_MapRoom1gateway teleports a player. Send requests and replies through Jötunn CustomRPC registered during plugin startup; resolve the peer and read its Steam connection identity. Empty or missing lists deny entry, including admins. Membership changes apply without a restart; exit stays unrestricted. The server-synced[EndgameBeta] Enabledsetting can bypass the beta gate before any gateway lookup. Server-side file polling supports edits through the profile symlink.63c2c05on Valdev and Valnet01 with the Season 8 8.0.46 mod set: empty/wrong/missing membership, client-only spoofed membership, allowed entry, removal and blocked re-entry, unrestricted exit, andSteam_IDs. No new feature exceptions. Existing shader and Trailership warnings remain. The automatic collision-trigger test was blocked by solid gateway geometry; normal gateway interaction was tested. Non-Steam transport, listen-server hosting, and approval timeout/disconnect remain untested.Latest validation at
864fb50: disabling and re-enabling the beta gate applied to the connected Valnet client without restarting Valdev (same process ID). A vanilla Crypt2 Burial Chamber test fixture allowed entry and exit with the gate enabled and an empty beta list; these interactions sent no beta approval requests. No new feature exceptions appeared after the fresh candidate startup. An earlier setup startup failed before PraetorisClient loaded with CultureNotFoundException; the fresh startup with verified candidate files did not repeat it.