Skip to content

Add Adrenaline Echo unique trinket area attack - #112

Merged
jneb802 merged 6 commits into
mainfrom
feature/adrenaline-projectile-shardstone
Sep 29, 2026
Merged

jneb802 merged 6 commits into
mainfrom
feature/adrenaline-projectile-shardstone

Conversation

@jneb802

@jneb802 jneb802 commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Prepares PraetorisClient 0.1.84 with Adrenaline Echo, a Unique trinket shardstone. When the trinket activates its full-adrenaline effect, it creates the equipped weapon's projectile at the player's position and immediately triggers its impact, with no travel. Staff of Embers creates an enchanted explosion around the player, using its normal 3-metre radius.

Plugin, assembly, manifest, and Thunderstore configuration versions are 0.1.84. The changelog describes the feature. The Release build and tcli build passed. The six-file package, icon dimensions, version, and DLL hash were checked. Prepared package: /Users/benjmarston/Develop/valheimModding/modRelease/PraetorisClient/PraetorisClient-0.1.84.zip. Publishing and the scheduled release update remain separate steps; the current staged Season 8 release is 8.0.30 with PraetorisClient 0.1.83.

The effect uses the real weapon item and normal patched projectile methods. All Epic Loot setup and burst hooks finish before impact. It costs no durability, stamina, health, or eitr. Projectile speed has no effect because there is no travel. Only weapons whose own projectile has a positive native area-damage radius qualify. Bows, crossbows, ammunition weapons, melee weapons, and empty hands produce no effect. The shard uses existing Stormcaller visuals and drop distribution, with a trinket-only effect and permanent ID 0x50520006.

Validation:

  • Release build passed. Existing project warnings remain.
  • Before the final weapon restriction, all 12 activation cases and the area test passed on Valnet client 02 in a local proof world with Valheim 1.0.16 and deployed Season 8 release 8.0.29. Thirty shared DLLs matched production hashes. The final restriction builds successfully but has not been rerun live. Earlier measurements confirmed Staff of Embers has radius 3 and ordinary arrows have radius 0; bows are now explicitly rejected.
  • Every bonus projectile had zero speed and zero distance from the player. Every impact was centered on the player. Verified damage/frost enchantments, real weapon attribution, Double Magic Shot, refresh, stagger, negative cases, and resource costs.
  • With god mode disabled, the area test damaged both nearby Trolls, ignored the distant Troll, and caused no player health loss. Both hits retained fire and frost damage of 90.55515 each.
  • Inspected new 1080p/60 fps video and equipment screenshots. The explosion occurs near 3.2 seconds in the 6.883-second video. God/ghost modes were enabled only for visual staging.
  • Restored all 483 original profile files and metadata exactly. The restored older profile still has the Epic Loot startup failures recorded before the test. Validation used a temporary production-matched repair. Production was not changed.

Evidence folder on the development Mac: /Users/benjmarston/Develop/valheim-validation-evidence/adrenaline-echo-aoe-20260928. Main artifacts: adrenaline-echo-aoe-proof.mp4, player-centered-explosion.png, adrenaline-aoe-trinket.png, adrenaline-aoe-staff.png, proof-summary.log, and baseline.json. The old folder contains the superseded travelling-projectile proof.

Limits: this proves the local client projectile and damage paths. Multiplayer target ownership, every combat adrenaline source, and every Epic Loot enchantment were not tested. Validation used a temporary probe that activated the real Player.AddAdrenaline method.

@github-actions

Copy link
Copy Markdown

Now I have enough context for a complete review.


No blocking issues found.

PR #112 — Add Adrenaline Echo unique trinket shardstone

The implementation is careful and well-constructed. Notes below, from most to least significant.


Stagger transpiler — stack layout is correct, worth documenting

EchoStaggerPatch.Transpiler (AdrenalineEchoRuntime.cs:166–177) inserts Ldarg_0; Call BlocksAttack immediately after IsStaggering(). The resulting stack at the branch instruction is bool(BlocksAttack) — replacing the raw stagger result with staggering && attack != activeEcho. That is exactly right: the echo proceeds through the stagger guard; other attacks are unchanged. The stack math is sound.


activeEcho guard has a one-frame blind spot (non-blocking edge case)

OnTrinketActivated checks activeEcho != null before calling StartCoroutine. activeEcho is not set to the echo instance until inside FireAfterAttack, after yield return null. If AddAdrenaline triggers the full-adrenaline branch twice within a single frame — e.g., from simultaneous multi-hit projectiles — two coroutines can be enqueued before either sets activeEcho. Both would pass the alive/weapon checks on the next frame and fire independently.

In practice, this requires two concurrent full-adrenaline fills within one physics step, which is unlikely for the trinket activation pattern. It is not a blocker, but if it surfaces it could double-fire without an exception.


definition.TypeEffects.Add assumes non-null initialization

PraetorisShardstones.cs:133 calls definition.TypeEffects.Add(...) on a freshly constructed ShardDefinition without guarding for null. The conflict-detection path at line 111 explicitly checks existing?.TypeEffects != null, which confirms TypeEffects can be null for entries deserialized from older configs. For brand-new entries created via new ShardDefinition { ... } this only works if the EpicLoot library auto-initializes TypeEffects in the constructor or property getter. The probe confirmed registration succeeds, so the library does initialize it — but this assumption is implicit and would surface as a NullReferenceException on startup if a future EpicLoot version changes that behaviour.


Probe exclusion and isolation — correct

<Compile Remove="tests/**/*.cs" /> (csproj line 52) correctly excludes the probe source from the SDK-style implicit glob. The probe's own .csproj references only EpicLoot-publicized.dll and does not reference the product assembly, so there is no accidental coupling. The ZNet.IsServer() guard prevents the probe from running outside a local world.


Resource-cost patches — all correct

  • EchoAmmoPatch returns ammoItem = AttackAmmo(echo) and __result = true while skipping UseAmmo; no ammo is consumed, and the caller sees the correct ammo reference for projectile setup.
  • EchoEitrPatch short-circuits GetAttackEitr to 0 for the echo only.
  • EchoProjectilePatch skips ProjectileAttackTriggered and calls FireProjectileBurst directly, bypassing durability deduction and reload state.
  • echo.m_consumeItem = false and echo.m_perBurstResourceUsage = false are defensive and consistent with the intent.
  • m_lastProjectile and m_lastAttackTime are restored in the finally block; PendingShot, IsTripleShotActive, and ShotProjectiles are likewise backed up and restored. The try/finally covers the exception path.

Ammo-type validation — correct

The early-return in FireAfterAttack (lines 72–77) correctly distinguishes weapons that need ammo (non-empty m_ammoType) from those that do not. For the staff case, m_ammoType is empty, so the ammo check is skipped. For bows/crossbows, the check enforces the presence of a compatible ammo item.


Conflict detection for TypeEffects-based shards — correct

The re-registration guard at PraetorisShardstones.cs:110–119 correctly handles the new slot-scoped path: it falls back to UniformEffect when TypeEffects is null (deserialized legacy entries), looks up the slot-specific effect when present, and throws on any mismatch. Idempotent re-registration from both Initialize() and the PraetorisShardConfigPatch prefix is covered.

@jneb802 jneb802 changed the title Add Adrenaline Echo unique trinket shardstone Add Adrenaline Echo unique trinket area attack Sep 29, 2026
@github-actions

Copy link
Copy Markdown

No blocking issues found.

The implementation is carefully scoped. Here is what I checked:

AdrenalineEchoRuntime.cs — core logic

  • Re-entrancy guard: activeEcho != null in OnTrinketActivated (line 46) prevents overlap while an echo is already executing. activeEcho is set inside the coroutine body after the one-frame delay, not at scheduling time, so two adrenaline-fill events in the same frame could schedule two coroutines. Both would fire sequentially on the next frame, each passing the alive/weapon guards independently. This matches the stated intent ("a fill from zero and a refresh of an existing status effect" both activate), so this is expected behavior, not a bug.

  • State restoration: The finally block correctly restores _pendingShot, MultiShot.IsTripleShotActive, MultiShot.ShotProjectiles, weapon.m_lastProjectile, and weapon.m_lastAttackTime regardless of exceptions, so the ongoing attack's timing and multishot state are unaffected.

  • Resource zeroing: Stamina, health, eitr, self-damage, recoil, consumeItem, requiresReload, and durability are all zeroed or suppressed on the clone. EchoAmmoPatch returns success without consuming ammo. Probe cases confirmed no resource cost.

  • EchoSetupPatch Priority.Last: Ensures velocity is zeroed after all other plugins (including Epic Loot) run their Prefix patches. Combined with EchoPositionPatch positioning the spawn at echoPosition, the projectile cannot travel.

  • EchoStaggerPatch transpiler (lines 207–219): Inserts Ldarg_0 + call to BlocksAttack after the IsStaggering call, correctly replacing the staggering bool so the echo bypasses the stagger early-return without affecting non-echo attacks.

  • Orphaned projectiles: If OnHit throws mid-loop, remaining collected projectiles stay in the scene with zero velocity. They will not travel or cause damage but won't be explicitly destroyed. The finally block clears EchoProjectiles and resets flags, so subsequent echoes are unaffected. This is a minor edge case, not a blocking behavior bug.

PraetorisShardstones.cs — slot-specific shard definition

  • The new spec.Slot.HasValue branch calls definition.TypeEffects.Add(...) on a freshly constructed ShardDefinition. If TypeEffects were null by default, this would throw on every startup for 0x50520006. Since all 12 activation cases and shard registration passed in the recorded validation, TypeEffects is initialized by the class.

  • The collision-detection path (lines 297–306) correctly falls back to UniformEffect when TypeEffects is null on an existing entry, and checks the slot-keyed effect when present. This handles backward-compatibility with config entries that predate the slot feature.

PraetorisClient.csproj

  • <Compile Remove="tests/**/*.cs" /> correctly excludes the probe from the product build. The probe's Probe.csproj is a separate project with its own references and is not shipped.

PraetorisMagicEffects.cs

  • AdrenalineEcho is defined with "NoRoll": true and "AllowedItemTypes": ["Trinket"], consistent with the trinket-only restriction enforced at runtime.

@github-actions

Copy link
Copy Markdown

No blocking issues found.

Reviewed: AdrenalineEchoRuntime.cs (new), PraetorisMagicEffects.cs (effect registration), PraetorisShardstones.cs (shard definition and slot-scoped effect support).


AdrenalineEchoRuntime.cs

The overall design is sound. Key paths checked:

  • Transpiler insertion point (ActivationPatch): The Ldfld m_adrenalinePopEffects anchor is stable and specific. Inserting ldarg_0 / call OnTrinketActivated just before it fires before the pop-effects loop and before any equipment state is mutated. Correct placement.

  • One-frame coroutine guard: The yield return null defers the echo past any in-flight Epic Loot burst hooks on the triggering attack. The re-validation on resume (player alive, same weapon, weapon still equipped) is sufficient because there are no further yield points; once execution resumes it runs to completion atomically from Unity's coroutine scheduler's perspective, so no mid-echo interruption is possible.

  • EchoSetupPatch (Priority.Last prefix on Projectile.Setup): Running last among prefixes lets this patch zero the velocity and reposition the projectile after Epic Loot's own Projectile.Setup prefixes have already run. Projectiles from other sources are excluded by the !collectingProjectiles || owner != Player.m_localPlayer guard. The flag is synchronously bounded by the non-yielding StartWithoutAnimation call, so no foreign projectile can slip in.

  • EchoStaggerPatch transpiler: After call IsStaggering, the stack is [bool]. ldarg_0 pushes the Attack instance, yielding [bool, Attack]. call BlocksAttack(bool, Attack) consumes both and leaves a replacement bool. BlocksAttack returns staggering && attack != activeEcho, so only non-echo attacks are blocked by stagger. Stack discipline is correct.

  • EchoAmmoPatch: Intercepts Attack.UseAmmo for the echo instance, supplies the pre-validated m_ammoItem, and short-circuits the original with __result = true. Prevents actual inventory deduction while satisfying the caller. Correct.

  • EchoEitrPatch: The comment correctly notes that vanilla GetAttackEitr reads from the shared weapon template rather than the cloned Attack's fields, so zeroing it here is necessary even with the cloned attack's cost fields already at 0.

  • EchoProjectilePatch: Replacing ProjectileAttackTriggered with a direct call to FireProjectileBurst skips the durability and reload code paths in the original method while still running through Epic Loot's patches on FireProjectileBurst itself. The EchoAmmoPatch covers any UseAmmo call within that burst.

  • MultiShot and weapon state save/restore: _pendingShot, IsTripleShotActive, ShotProjectiles, m_lastProjectile, and m_lastAttackTime are all captured before and restored in finally. This prevents the echo from corrupting an in-progress staff burst's roll state or the weapon's attack-tracking fields.

  • OnHit(null, echoPosition, false, Vector3.up): Passing null as the collider triggers the area-damage path without requiring a specific hit target. Centered on the player's position at coroutine start. m_hitOwner = false prevents self-damage.

PraetorisShardstones.cs

  • definition.TypeEffects.Add(spec.Slot.Value, effect): ShardDefinition is an EpicLoot type; its TypeEffects property is initialized by the library and not null on a freshly constructed instance. The existing?.TypeEffects != null guard in the duplicate-ID check is defensive for config-deserialized definitions that predate slot-scoped effects (where the field may not have been populated). No NullReferenceException risk on the construction path.

  • Duplicate-ID check logic: For a new trinket-scoped spec, existingEffect falls through to existing.UniformEffect when existing.TypeEffects is null (old config entry). If the existing entry has TypeEffects, the correct slot is probed. Either way the effect-type comparison fires and the collision is detected. Correct.

  • Permanent ID 0x50520006: Follows the existing pattern; no collision with the four existing IDs (0x50520001–0x50520003, 0x50520005).

Acknowledged limits (from PR description, not blocking): Multiplayer target ownership, all combat adrenaline sources, and all Epic Loot enchantment interactions were not covered. These are reasonable scope boundaries for a local proof-of-concept validation.

@github-actions

Copy link
Copy Markdown

No blocking issues found.

AdrenalineEchoRuntime.cs

The static-state model (activeEcho, echoPosition, collectingProjectiles, EchoProjectiles) is guarded by the activeEcho != null check in OnTrinketActivated, but that check runs before yield return null. There is a one-frame window during which a second rapid full-adrenaline trigger (e.g., from a multi-hit weapon that tops up adrenaline twice in one frame's damage callbacks) could pass the guard and start a second coroutine, racing to assign activeEcho and corrupt the shared list. In practice this window is very narrow, but it is a real edge case for burst-filling weapons.

EchoSetupPatch runs at Priority.Last, which correctly ensures its velocity = Vector3.zero write survives any Epic Loot prefix patches that run at normal priority. No issue there.

The EchoStaggerPatch transpiler correctly advances past the IsStaggering call before inserting, so BlocksAttack(bool, Attack) receives the stagger result on the stack and the attack instance from ldarg_0, replacing the original bool without disturbing the call site.

The EchoEitrPatch covers the parameterless GetAttackEitr() overload. If any code path calls an overloaded form or reads the eitr field directly from the weapon's shared data rather than through the method, the cost would not be zeroed. The PR description validates zero cost in testing, so this appears correct.

weapon.m_lastProjectile and weapon.m_lastAttackTime are saved before StartWithoutAnimation and restored in finally, which prevents the echo from polluting the weapon's attack-tracking state for the next real shot.

PraetorisShardstones.cs

The re-registration guard (lines 110–114) correctly handles the new slot-based path: existingEffect is initially set from UniformEffect, then overwritten by TryGetValue only when spec.Slot.HasValue and TypeEffects != null. For a slot-specific shard (Adrenaline Echo) re-encountered on config reload, TryGetValue will find the key and set existingEffect correctly so the check does not throw. For legacy uniform-effect shards, spec.Slot is null so the branch is never entered and the old path is unchanged.

Line 133 calls definition.TypeEffects.Add(...) without a null guard. This is safe only if ShardDefinition.TypeEffects is auto-initialized by the EpicLoot library's constructor or via a lazy property. Validation confirms this works, but the dependency on library internals is worth noting if EpicLoot-publicized.dll is ever updated.

PraetorisMagicEffects.cs

AdrenalineEcho is correctly declared as CanBeAugmented: false, CanBeDisenchanted: false, CanBeRunified: false, and restricted to Trinket item type, consistent with its design as a unique trinket-only shard.

Acknowledged gaps from PR description

Multiplayer target ownership, all combat adrenaline sources, and the full Epic Loot enchantment matrix were not tested live. The final weapon restriction (area-projectile-only) built successfully but was not re-run live. These are accepted scope limits for the feature, not new risks introduced by the diff.

@jneb802
jneb802 merged commit 9c114f8 into main Sep 29, 2026
1 check passed
@github-actions

Copy link
Copy Markdown

No blocking issues found.

Source/EpicLoot/AdrenalineEchoRuntime.cs

The core implementation is sound. A few observations worth recording:

OnTrinketActivated → FireAfterAttack reentrancy window (AdrenalineEchoRuntime.cs:68–91): The activeEcho != null guard in OnTrinketActivated is checked before the coroutine runs, but activeEcho is not set until the coroutine resumes after yield return null. If the trinket activation transpiler fires twice within the same frame (e.g., a multihit fills adrenaline twice in one Update), two coroutines are queued before either sets activeEcho, allowing both to proceed. In Unity's coroutine model the two would run sequentially in the next frame — first completes and clears activeEcho, second then passes the null-check and fires independently. This is an extremely narrow window in practice (requires two trinket activations in a single frame), and the subsequent weapon-identity checks (player.GetCurrentWeapon() != weapon, !weapon.m_equipped) in the coroutine limit the damage, but it is a structural gap in the guard.

EchoSetupPatch sets fields before vanilla Setup runs (AdrenalineEchoRuntime.cs:178–190): The Prefix runs with Priority.Last, so it fires after all other prefixes but before the vanilla method and any postfixes. Epic Loot postfixes on Projectile.Setup (e.g. Explosive Arrows) run after this patch and may set m_hitOwner or other flags the patch already cleared. The code compensates by calling OnHit(null, ...) immediately after the burst completes, before any physics frame advances, which short-circuits any lingering state. The comment at line 139 ("Finish all setup and burst hooks before impact") confirms this is deliberate.

definition.TypeEffects.Add() called without null guard (PraetorisShardstones.cs:133): The corresponding verification path defensively checks existing?.TypeEffects != null (line 111), while the creation path calls .Add() directly on a freshly constructed ShardDefinition. Since ShardDefinition is from the compiled EpicLoot library, the field must be auto-initialized — otherwise the effect could not have passed the 12-activation test the PR reports. The null guard in the verification path is plausibly for deserialized/older config compatibility, not a signal that new objects can have a null collection. No action needed, but worth confirming against the library source if the EpicLoot version is ever bumped.

Transpiler correctness (ActivationPatch, EchoStaggerPatch): Both transpilers are correct. ActivationPatch inserts ldarg_0 / call OnTrinketActivated before the ldfld m_adrenalinePopEffects instruction — the pushed Player reference is consumed by the call, leaving the original IL stack unaffected. EchoStaggerPatch inserts after call IsStaggering, passing the bool result and ldarg_0 (the Attack) to BlocksAttack, which replaces the stagger bool with staggering && attack != activeEcho; the downstream branch instruction sees the replacement value correctly.

MultiShot state save/restore (AdrenalineEchoRuntime.cs:127–163): The three-field snapshot (pendingShot, IsTripleShotActive, ShotProjectiles) plus m_lastProjectile / m_lastAttackTime is restored unconditionally in finally, which handles both the normal path and exceptions. Clean.

Version and metadata consistency: Assembly, plugin constant, manifest, and thunderstore versions all move to 0.1.84. Changelog entry is present. No issues.

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