Skip to content

Bundle Valheim Endgame and tablet compatibility in PraetorisClient 0.1.102 - #145

Merged
jneb802 merged 7 commits into
mainfrom
feature/endgame-epicloot-compat
Oct 19, 2026
Merged

jneb802 merged 7 commits into
mainfrom
feature/endgame-epicloot-compat

Conversation

@jneb802

@jneb802 jneb802 commented Oct 18, 2026 •

Copy link
Copy Markdown
Owner

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.

  • Include both DLLs, tablet config patches, required dependencies, and separate all-rights-reserved licenses with third-party notices.
  • Allow tablet enchanting, restore the result display, and block new tablet sockets, shardstone operations, chisels, and crafting socket transfers.
  • Check [EndgameBeta] AllowedSteamIds on the server before the exterior MWL_MapRoom1 gateway 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] Enabled setting can bypass the beta gate before any gateway lookup. Server-side file polling supports edits through the profile symlink.
  • Build passes with zero errors. No test project or test documentation is included.
  • Live beta checks passed again with the Jötunn transport at 63c2c05 on 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, and Steam_ 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.
  • Earlier Epic Loot and multiplayer checks passed enchanting, result display, saved tablet data, socket/chisel rejection, shared attempts, room transitions, reconnect, and encounter cancellation. Crafting transfers in gameplay, existing socket contents, bridge removal, simultaneous actions, and complete attempt exhaustion remain untested.
  • Existing encounter travel issue: https://github.com/jneb802/ValheimEndgame/issues/58. This PR does not claim full production validation. No production deployment or Thunderstore publication.

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.

@github-actions

Copy link
Copy Markdown

Now I have a clear picture of the diff. Let me write the review.


Blocking issues found.

HideSocketPreview transpiler loads this instead of the item parameter

File: Source/EpicLoot/EndgameCompatibility.cs, lines 227–229

CodeInstruction load = new(OpCodes.Ldarg_0);
load.labels.AddRange(instruction.labels);
...
yield return load;
instruction.opcode = OpCodes.Call;
instruction.operand = AccessTools.Method(typeof(EndgameCompatibility), nameof(PreviewSocketWeights));

The transpiler patches GetEnchantInfo on EpicLoot.CraftingV2.EnchantingUIController. That class is a MonoBehaviour/UI controller, so GetEnchantInfo(ItemDrop.ItemData item, ItemRarity rarity) is an instance method. In that case the IL argument slots are:

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.

@jneb802 jneb802 changed the title Add temporary Endgame tablet compatibility for Epic Loot Bundle Valheim Endgame and tablet compatibility in PraetorisClient 0.1.102 Oct 19, 2026
@github-actions

Copy link
Copy Markdown

I have everything I need. Here is my review:


Blocking issues found.

HideSocketPreview injects Ldarg_0 but GetEnchantInfo is almost certainly an instance method

File: Source/EpicLoot/EndgameCompatibility.cs, line 181

The HideSocketPreview transpiler injects OpCodes.Ldarg_0 immediately before the redirected call to PreviewSocketWeights(int rarity, ItemDrop.ItemData item):

CodeInstruction load = new(OpCodes.Ldarg_0);
// ...
yield return load;
instruction.opcode = OpCodes.Call;
instruction.operand = AccessTools.Method(typeof(EndgameCompatibility), nameof(PreviewSocketWeights));

The intent is to push the item argument onto the stack so PreviewSocketWeights can call IsTablet(item). This is correct only if GetEnchantInfo is a static method, where Ldarg_0 is the first declared parameter (ItemDrop.ItemData item). However, EnchantingUIController is a MonoBehaviour-based Unity component; its GetEnchantInfo method is almost certainly an instance method, which means:

  • Ldarg_0 = this (the EnchantingUIController instance)
  • Ldarg_1 = the ItemDrop.ItemData item

When the rewritten call executes, PreviewSocketWeights receives this as its item parameter. this as ItemDrop.ItemData is null, so IsTablet(null) returns false, and the method falls through to socketWeights.Invoke(...) — returning the real socket weight table. Tablets will still display socket probabilities in the enchanting preview, contradicting the stated PR goal ("remove socket probabilities from their enchanting preview").

Contrast with PreventRolledSockets: that transpiler correctly uses Ldarg_1 (line 160) because RollMagicItem is a static method on LootRoller where Ldarg_1 is the item parameter.

The harness does not catch this. The test at tests/EndgameCompatibility/Program.cs:67–69 only counts how many times PreviewSocketWeights appears in the rewritten IL. That count is still 1 regardless of whether Ldarg_0 refers to this or to the item. No harness check verifies the semantic argument identity.

The live validation does not confirm this path. The PR description confirms "zero sockets" on rolled items (from PreventRolledSockets) and rejection by the socket guards, but does not explicitly state that the enchanting-table UI preview showed zero socket probability for tablets.

Fix: if GetEnchantInfo is an instance method, change line 181 from OpCodes.Ldarg_0 to OpCodes.Ldarg_1. If it is static, document that assumption (e.g., a harness check that asserts method.IsStatic) so the behavior is not silently broken by a future refactor.


Non-blocking observation

HasNativeSupport checks all RollMagicItem overloads rather than the specific one patched (Source/EpicLoot/EndgameCompatibility.cs, line 124). AccessTools.GetDeclaredMethods(type).Where(m => m.Name == "RollMagicItem") matches every overload; if a partially-native build adds the IsSocketable guard only to a different overload, native detection returns true and the socket-rolling patch is skipped even though the targeted overload has no native guard. This does not affect the production hash-verified binary today, but it would silently leave tablets vulnerable to socket rolls during a mid-migration native build.

@github-actions

Copy link
Copy Markdown

No blocking issues found.

Reviewed areas: EndgameCompatibility.cs (all 252 lines), Plugin.cs call sites, IsTabletName, all six transpiler/prefix/postfix patches, HasNativeSupport, ApplyCraftResultFix, manifest/thunderstore packaging, and version-bump consistency.

Transpiler stack reasoning

  • PreventRolledSockets: inserts Ldarg_1 (the ItemDrop.ItemData item parameter of the static RollMagicItem) before the Stfld socketCount, giving FilterSocketCount(int count, ItemDrop.ItemData item) the correct stack layout [magicItemRef, count, item]. Net stack effect is unchanged. ✓
  • HideSocketPreview: inserts Ldarg_0 (ItemDrop.ItemData item, first parameter of the static GetEnchantInfo) before the mutated call, producing [rarity, item] for PreviewSocketWeights(int rarity, ItemDrop.ItemData item). ✓
  • PreventTransferredSockets: replaces Ldfld socketCount with Call TransferSocketCount(object magicItem) in-place; both pop one reference and push one value, so the net stack effect is the same. The matches != 3 guard is a hard assertion against layout drift. ✓

IsTabletName range

The name.Length == prefix.Length + 2 check requires exactly two suffix characters, covering zero-padded names ve_tablet_tier01–ve_tablet_tier15. Live validation confirmed tiers 1, 5, 10, and 15 were all recognized, meaning the game uses zero-padded names. ✓

TransferSocketCount static-field assumption

craftedItem.GetValue(null) is valid only if CraftedItem is a static field on TransferMagicalEffects. This passed harness coverage; in-game crafting transfer is noted as not live-tested. If CraftedItem were an instance field, GetValue(null) would throw a TargetException at crafting time, crashing the transfer for all items and logging a BepInEx exception. Given the field name and typical crafting-state patterns this is very unlikely to be instance-scoped, and the hash verification scopes risk to the exact binary. The PR explicitly flags this coverage gap.

HasNativeSupport / double-patch safety

Checks both field presence (Enchantable, Socketable) and implementation paths in five specific methods before declaring native support. The AllowTablet/ClassifyTablet postfixes are registered manually under Owner and are excluded from the assembly-wide ApplyHarmonyPatches scan. The independent Owner string ensures Shutdown/UnpatchSelf is clean. ✓

ApplyCraftResultFix call coverage

Applied in both the native-support path and the legacy-hash path; correctly withheld for unsupported binaries (the explicit warning log communicates this). The >= 3 enabled-setter count is a correct idempotency guard. ✓

Packaging

manifest.json and thunderstore.toml both add the two Endgame runtime dependencies (StarLevelSystem, YamlDotNet). The new [[build.copy]] blocks cover the Endgame DLL, license, config patch directory, and third-party notices. Version bumps are consistent across Plugin.cs, AssemblyInfo.cs, manifest.json, and thunderstore.toml. The prior assembly version lag (0.1.100 vs mod 0.1.101) was pre-existing and is resolved here.

Nit: PreventPlacement is patched onto both CanSocket(item, item, ref string) and CanCoexist(item, item, IEnumerable<…>, ref string), checking __0 (first parameter) as the tablet in both. If CanCoexist's first parameter is the incoming shard rather than the target item, the tablet guard would silently no-op on that path. Live testing confirmed socket rejection worked end-to-end, so the parameter ordering appears correct for both methods.

@github-actions

Copy link
Copy Markdown

I have enough context now to write the review.


No blocking issues found.

Source/Endgame/EndgameBetaAccess.cs

The gating logic is structurally sound and deliberately mirrors NetworkWardAccess: CurrentConnection() at line 180 requires !_session.IsServer(), which means a listen-server host is always blocked and receives the "requires approval from a connected Steam server" message. This is the same accepted limitation in NetworkWardAccess, and the PR's validation exclusively covers dedicated-server deployments, so it is not a regression.

The sequence in IsAllowed() (lines 223–234) is slightly different from the established RefreshServerConfig() approach in NetworkWardAccess: it creates a ConfigFile snapshot, reads the value, then calls config.Reload() on the live config, and finally overwrites EndgameBetaAllowedSteamIds.Value with the snapshot value. After Reload(), the entry already holds the file's current value; the subsequent assignment is redundant but harmless since both reads come from the same file in rapid succession on the main thread. The live config does take a full Reload() on every Map Room entry attempt (vs. NetworkWardAccess's lighter pattern), but this has no incorrect behavior impact.

The RegisterAccessRpc patch at line 295 registers both RPCs on every peer regardless of role. OnRequest guards with !ZNet.instance.IsServer() and OnResponse guards with _instance != null && ReferenceEquals(rpc, access._server), so cross-role delivery is silently dropped in both directions.

Source/EpicLoot/EndgameCompatibility.cs

The hash guard at line 366–372 correctly gates all legacy IL patches to the exact known binary (802790e…), so the matches != N invariant checks in the transpilers can only fire against a layout they were designed for. The HasNativeSupport check at line 414–441 verifies both field/property presence and that each of the five enforcement paths actually calls IsSocketable in its IL, preventing premature early-exit on a partial upstream build.

The PreventRolledSockets transpiler at line 463 inserts ldarg.1 (the ItemDrop.ItemData item parameter, correct for a static RollMagicItem) before the FilterSocketCount call, keeping the stack balanced for the subsequent stfld. The HideSocketPreview transpiler at line 484 similarly inserts ldarg.0 (the item parameter in static GetEnchantInfo) before replacing the call to GetSocketCountsPerRarity with PreviewSocketWeights, which also matches the (rarity, item) stack order.

ApplyCraftResultFix's idempotency guard (line 553, >= 3 calls to Behaviour.enabled.set) correctly prevents double-patching in the native-support path and the legacy path.

The [BepInDependency(EndgameCompatibility.EndgameGuid, SoftDependency)] declaration in Plugin.cs at line 94, combined with the Chainloader.PluginInfos.ContainsKey(EndgameGuid) guard in Initialize(), ensures all Epic Loot compatibility patches are skipped cleanly when Endgame is absent.

@github-actions

Copy link
Copy Markdown

Now I have enough context. Let me write the review.


Blocking issues found.

EndgameCompatibility.cs — IsTabletName silently excludes single-digit tier tablets

Source/EpicLoot/EndgameCompatibility.cs, around line 356:

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;

prefix.Length is 14, so prefix.Length + 2 == 16. That requires exactly two characters after the prefix. NumberStyles.None does not allow a leading zero, so only unpadded two-digit numbers (10–99) can parse. Combined with tier <= 15, the only names that ever match are ve_tablet_tier10 through ve_tablet_tier15.

The sub-check tier >= 1 is dead code: a two-digit NumberStyles.None integer starts at 10, so it can never be 1–9. The intent of the range 1–15 is contradicted by the length guard.

If ValheimEndgame contains any items named ve_tablet_tier1 through ve_tablet_tier9 (15-char names), IsTablet() returns false for them and every restriction that guards on IsTablet is silently skipped:

  • AllowTablet — tier 1–9 tablets might not be enchantable (depends on EpicLoot's default behaviour)
  • PreventRolledSockets / PreventTransferredSockets — rolled and crafting-transferred sockets are not blocked
  • PreventChisel / PreventPlacement — chisel and shardstone placement are not blocked

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 ValheimEndgame.dll, which cannot be verified from the diff. The live test noted "socket/chisel rejection" as passing, but if that test was limited to ve_tablet or tiers 10–15, the single-digit gap would not have been exercised.

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

  • Map Room access control (EndgameBetaAccess.cs): the Jotunn RPC path correctly uses the routed sender peer ID (not a payload-supplied ID) to look up the authenticated Steam socket, matching the security model in NetworkWardAccess.OnRequest. The client-side approval flow (pending flag, deadline, _approvedGateway guard, re-check of IsEntrance and distance before calling Interact) is sound. Exit is unrestricted as intended.
  • IsAllowed config hot-reload: structurally identical to RefreshServerConfig — reads a fresh ConfigFile snapshot, calls Config.Reload() to keep the in-memory config consistent, then syncs the bound entry. No mtime caching, but that is a minor performance trade-off, not a correctness issue.
  • Transpilers (PreventRolledSockets, HideSocketPreview, PreventTransferredSockets): each validates an exact match count and throws on mismatch, causing Initialize to roll back all patches via Shutdown() in the catch. Safe failure mode.
  • Native support detection (HasNativeSupport): verifies each enforcement path calls IsSocketable before skipping the legacy patches. The ApplyCraftResultFix idempotency guard (counting Behaviour.enabled setter calls) is correctly applied in both the native and legacy paths.
  • NetworkWardAccess.FindSteamSocket visibility (NetworkWardAccess.cs:95): widened from private to internal to allow reuse; no behaviour change.
  • AssemblyInfo.cs version: assembly version jumps from 0.1.100 to 0.1.102, skipping 0.1.101. The plugin ModVersion string goes 0.1.101 → 0.1.102 correctly; the assembly version discrepancy is a metadata gap but not a runtime behaviour bug.

@jneb802
jneb802 merged commit 353de47 into main Oct 19, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant