[Mathih13]

A Warlock's Soulstone does nothing today. A Character that dies with the Soulstone aura sees only Release Spirit. The 1.12.1 client shows a second button in the death dialog when the server writes a spell id into the player field PLAYER_SELF_RES_SPELL. The button sends CMSG_SELF_RES. This server wrote no such field and dropped the opcode. The importer also turned the Soulstone aura and its spell effect into inert no-ops.

This PR adds the Self-Resurrection Option, from death to revive.

• At death, kill_player reads the Soulstone aura before the death aura shed removes it. It writes one row to the new table game_self_resurrect_option. A death without a Soulstone deletes any old row.
• gw_self_resurrect (and debug_self_resurrect) uses the option. The Character revives in place at the option spell's values: Minor Soulstone gives 400 health and 700 mana. Rage comes back at 0 and energy full. There is no Resurrection Sickness, and the corpse goes. A Refusal changes nothing.
• Corpse reclaim, Spirit Healer, an accepted ally resurrect and leaving the world delete the option. Two helpers now own these deletes. clear_resurrect_request removes only the ally request, and clear_resurrect_request_and_option removes both.
• The Gateway writes the option to the owner's PLAYER_SELF_RES_SPELL through the partial-VALUES path, with no OBJECT_FIELD_TYPE, and writes 0 when the row goes. Nobody else gets the packet. CMSG_SELF_RES calls the reducer for the session's own Character.
• The importer maps raw effect 94 to the new E_SELF_RESURRECT (0x26) kind and the five Soulstone Resurrection aura ranks to A_SELF_RESURRECT (0xB5). A table keyed by aura spell id gives each rank its option spell (20707 to 3026 up to 20765 to 20761). I skipped kind 0x25 because feat(spells): Distract turns idle creatures toward the point #614 (Distract) uses it.
• CONTEXT.md gets a "Death and resurrection" section with Self-Resurrection Option and Soulstone.
• docs/self-resurrection-client-check.md is the checklist a human runs with a real client.

Decisions a maintainer may want to flip:

• Self only. Item use casts at the user, so a Warlock can only soulstone itself. Soulstone on a party member needs CMSG_USE_ITEM targets in the Gateway. That is a separate change.
• Release Spirit keeps the option, a Shard hop does not. cmangos does the same. The row stays on the old Shard's database, so a ghost released from a dungeon into another Shard loses it. The client check records whether the client offers it to a ghost at all.
• No cast path. The option runs as a direct revive, not through the cast pipeline, which refuses dead casters. So the client sees no cast visual.
• Known loss, not fixed. A Soulstone item is consumed when its 3 second cast starts, the same as every timed consumable. Interrupt the cast and the item is gone. The 30 minute item cooldown is also not enforced, because items have no category cooldowns here.

Where the code differed from the plan: the Dummy aura imports as A_FLAG, not E_SCRIPTED, so the name rescue catches the A_FLAG residue. The table holds character_guid and spell_id only. Nothing reads an identity or timestamp column, and the Gateway addresses the owner by guid. The Gateway wires insert, update and delete, because a replaced row reaches the SDK as an update.

Verification on this Mac:

• cargo test -p lyracore-module --lib: 1747 passed, including the pure self_resurrect_vitals cases and the updated taxonomy pins.
• cargo test -p lyracore-module --test self_resurrection -- --ignored: 5 durable tests passed against a published standalone. They cover the option at death, the revive values, the Refusals, Release Spirit, Spirit Healer, logout, and rage and energy.
• cargo test -p lyracore-importer: 399 passed, including all five Soulstone ranks and a Reincarnation-shaped percent row.
• cargo test -p lyracore-gateway --bin lyracore-gateway: 1969 passed, including a byte pin for field 1224, the owner-only relay, and the CMSG_SELF_RES dispatch and Refusal.
• cargo clippy on the three crates with --all-targets --all-features -D warnings: clean.
• scripts/check-gateway-bindings.py: all binding files current. schema_parity only runs on Linux, so CI checks it.

Not verified: no real client has seen the field, the button, the 30 minute duration or the 3 second cast. The corpse reclaim and the ally resurrect deletes have no durable test, because reclaim needs a 30 second delay and an ally resurrect needs a second caster.

Refs #11. Reincarnation and the real-client check remain.

Model and harness: Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 10 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: c4e281bb-fdc0-4919-8426-f26e239f486e

📥 Commits

Reviewing files that changed from the base of the PR and between 1a47b8b and ad6b5a8.

📒 Files selected for processing (35)

• CONTEXT.md
• docs/architecture.md
• docs/self-resurrection-client-check.md
• gateway/src/codec/tests.rs
• gateway/src/codec/values.rs
• gateway/src/stdb/bindings/debug_self_resurrect_reducer.rs
• gateway/src/stdb/bindings/game_self_resurrect_option_table.rs
• gateway/src/stdb/bindings/gw_self_resurrect_reducer.rs
• gateway/src/stdb/bindings/mod.rs
• gateway/src/stdb/bindings/self_resurrect_option_type.rs
• gateway/src/stdb/connection.rs
• gateway/src/stdb/reducers.rs
• gateway/src/stdb/subscriptions.rs
• gateway/src/stdb/world_store.rs
• gateway/src/stdb/world_view.rs
• gateway/src/world/handlers/loot.rs
• gateway/src/world/store.rs
• gateway/src/world/tests.rs
• gateway/tests/schema_parity.rs
• importer/src/spell.rs
• module/src/actor.rs
• module/src/combat/death.rs
• module/src/corpse.rs
• module/src/debug/mod.rs
• module/src/gw.rs
• module/src/spell/cast/targeting.rs
• module/src/spell/mod.rs
• module/src/spell/scheduler.rs
• module/src/spell/self_resurrect.rs
• module/src/spell/tables.rs
• module/src/spell/taxonomy.rs
• module/src/spell/tests.rs
• module/src/tripwires.rs
• module/src/world.rs
• module/tests/self_resurrection.rs

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Mathih13]

Code review: Standards and Spec

I reviewed main...ad6b5a8 (3 commits) on two separate axes. The generated stdb/bindings files were not reviewed. Priorities: P1 must fix, P2 should fix, P3 nit.

Standards

The diff has no hard breach of CODING_STANDARDS.md, the unslop rules or the CONTEXT.md Avoid lists.

• P2. CONTEXT.md does not match the code. The glossary says the Self-Resurrection Option is "kept through Release Spirit". But remove_live_character calls clear_resurrect_request_and_option, and a Transfer removes the live row on the source Shard. So a dungeon death released into an outdoor graveyard loses the option. The client-check doc states this, but the glossary does not. Carry the option through Transfer, or write the exception into the CONTEXT.md entry.
• P3. Mixed spelling of the term. The code uses self_resurrect_option, self_res_option_changed, self_res_spell_of and record_self_resurrect_option for one glossary term. Keep the table and protocol names, but use one spelling in new Rust identifiers. "Shard hop" in the client-check doc should be "Transfer".
• P3. Unclear prose (unslop). In self_resurrect.rs, "Delete both ways back character_guid may hold" is hard to read. Say "Delete the pending resurrect request and the Self-Resurrection Option." Replace the → in the self_res_option_changed doc comment with plain words.
• P3. The tripwire category is wrong. tripwires.rs puts game_self_resurrect_option under "transient spell state removed, consumed, or made inert by its deadline", but the option has no deadline.
• P3. Comment density (AGENTS.md "Legacy code style"). The importer arms for E_SELF_RESURRECT, A_SELF_RESURRECT and 94 => have long essay comments. Shorten them.
• P3. Smell, Speculative Generality. The E_SELF_RESURRECT arm in apply_effect only logs and returns EffectHit::none() for a path that is never cast. The taxonomy tag alone is enough.
• P3. Smell, Primitive Obsession. self_resurrect_vitals takes five loose scalars. This is acceptable for a single caller.
• Duplicated Code: the CMSG_SELF_RES arm adds one more copy of the WorldState::InWorld(iw) => iw.self_guid match. This shape is already repeated across the repo, so it does not belong in this PR.

Spec (#11)

I found no correctness bug in the main path. record_self_resurrect_option runs before the death aura shed. A second kill returns early, so it cannot erase the option. Reclaim, Spirit Healer, an accepted ally resurrect, self-res and logout all clear the option. Power matches vanilla: mana is capped, rage goes to 0 and energy fills. Only the owner gets the relay.

Missing or partial

• P1, blocks closing A Shaman who dies with Reincarnation ready gets no Use Reincarnation button #11, and the PR correctly uses Refs. The spec says: "the buff-duration/consume behavior is confirmed against the real client by a human, not guessed." docs/self-resurrection-client-check.md is still open.
• P2. The spec says: "a buff granted earlier by a Warlock's Create Soulstone". The PR is self-only, but the importer and tests already model the aura as T_TARGET_ALLY. File the CMSG_USE_ITEM targeting follow-up so that this does not get lost.
• P2. The spec says: "consuming that buff/item on use". An interrupted cast loses the item, and the 30-minute cooldown is not enforced. Both are documented, but both differ from vanilla and neither is verified.
• P3. The corpse reclaim and ally-resurrect deletes have no durable test. These are two of the five revive paths.

Implemented, but it looks wrong

• P2. The spec says: "clearing the dead/ghost state". A ghost keeps the option after Release Spirit, and do_self_resurrect then revives it in place at the graveyard and deletes the corpse. Vanilla clients hide the button for a ghost. If the client cannot send CMSG_SELF_RES in this state, the path is dead. If it can, the revive location is a guess. Decide this during the client check, or drop the option on Release Spirit.
• P3. debug_set_health(>0) (module/src/debug/mod.rs:227) revives without clearing the option, so a living Character keeps a nonzero PLAYER_SELF_RES_SPELL until its next death. This affects debug only.
• P3. An unknown Soulstone rank imports with p0 = 0 and gives no option, with no warning. Log it in the importer coverage output.

Scope creep: the Reincarnation-shaped percent branch and the 21169 test go past Soulstone. They are small, they share effect 94, and the PR body lists Reincarnation as remaining work, so this is acceptable.

---

Summary. Standards: 8 findings, all soft. The worst is the CONTEXT.md entry, which promises the option survives Release Spirit while a Transfer drops it. Spec: 9 findings. The worst is the open real-client check, which blocks #11 but not this PR. The worst code-level finding is ghost self-res at the graveyard.

---

Generated by Claude Code