[Mathih13]

What and why

Benilla exposed missing request handlers and 1.12.1 packet layouts that LyraCore's protocol dependency encodes differently. Some requests were silently ignored, and bag operations, channel cancellation and confirmation dialogs did not reach the existing gameplay correctly.

This change fixes those paths and adds Benilla as a pinned, independent Headless Client. All 277 requests in that revision now have a dispatch pattern or a reviewed raw route. Valid requests for missing gameplay return an explicit Refusal or System Message while in world. Automatic stand-state, action-bar and tutorial requests announce each missing feature once per World Session. The compatibility notes list the unavailable systems and the limits of this inventory.

• Correct ping, Realm Clock, movement, transport attachment, XP and reputation packets.
• Connect split, destroy and bag operations to Module Gates. Check both sides of swaps, protect offered trade items, refuse conflicting two-hand/offhand equipment and refresh the Character sheet after equipped gear is destroyed.
• Send bind and talent-reset confirmations, check the selected innkeeper in the Module, relay changed bind points and cancel channels whose auras sit on another unit. Channel detection includes all effects of a spell marked CHANNELED.
• Complete quest reward requests, logout and mover receipts, plus item and movement projection at world entry.
• Cover real dispatch with Benilla and run durable scenarios against private SpacetimeDB instances in CI.

What you verified

• cargo test --locked --workspace --exclude lyracore-package-delta --features lyracore-module/debug_reducers: 4,376 passed.
• The durable ownership regression passed at its new test path after the Store refactor.
• Eight durable Benilla scenarios passed on private SpacetimeDB 2.7.1 instances, including inventory persistence, channel cancellation and immediate bind-point updates.
• Fifteen durable inventory and GUID regressions passed. They cover accepted trade terms, scheduled reagent refusal, changed and restored offer quantities, and conflicting hand equipment.
• All four durable random-property tests passed after isolating the healing test from scheduled regeneration. The regeneration test retains its tick.
• All three durable mount tests passed after isolating the refusal comparison from scheduled rage decay.
• Independent adversarial Standards and Spec reviews passed after the fixes and the rebase onto a6121982.
• Formatting, workspace Clippy with all targets and features, and the generated-binding check passed.
• Plain publish from main at f09dafbd migrated a private instance without clearing it. Character rows survived, and the appended XP field was readable.

The unchanged package-delta quest tests still have two fixture failures because WHOLE_QUEST_ROW omits quest_type. Those failures are documented in the compatibility notes and are outside this change.

What you did NOT verify

No graphical build-5875 client run. The request inventory does not certify every existing handler or all 346 outgoing opcode rows. Battlegrounds, pet stables, GM tickets and the other systems listed in the compatibility notes remain unavailable.

Risk

The change reaches Gateway packet handling and Module inventory and channel rules. XpEvent.rested_bonus is appended with a zero default, and Aura gains a caster index. Gateway bindings are regenerated. The binder Gateway Verb now carries the innkeeper guid, so the Gateway and Module must update together. A later deployment must republish every Shard for the schema change.

Built with GPT-6 Astra in the Codex harness through T3 Code.

[Mathih13]

Code review: Standards and Spec

Reviewed at d90495a, diffed against the merge-base with main (f09dafbd). No issue is linked, so the Spec axis uses the PR description and docs/benilla-compatibility.md as the spec. Priorities: P1 blocks merge, P2 should fix, P3 nit or judgement call. Line numbers refer to the PR head.

Standards

Hard violations

• P1: one change per PR (CONTRIBUTING.md, "Big or mixed changes. Send one change per pull request"). One commit of about 4.7k lines carries:

  • inventory destroy and split
  • the transport movement codec
  • channel cancellation
  • rested XP plus a schema column
  • bind and respec dialogs
  • bind-point relay
  • GM ticket and battleground stubs
  • the Benilla harness and a new CI job

  Splitting it into reviewable PRs, at least protocol fixes / inventory Gates / channel + schema / Benilla harness, would make each part reviewable and revertible on its own.

• P2: Refusal mixed with infrastructure failure (CODING_STANDARDS.md, "Refusal stays distinct from transport/infrastructure failure"; CONTEXT.md lists error under Avoid for gameplay refusals). handlers/query.rs (CMSG_BINDER_ACTIVATE, MSG_TALENT_WIPE_CONFIRM, around L103-118) and handlers/trainer.rs (handle_at_war, around L296-302) send error.to_string() to the player as chat. A SpacetimeDB timeout reaches the player as a raw anyhow string, and Err(anyhow!("That creature is not an innkeeper.")) puts a Refusal into an infrastructure error. The block is also copied three times.

• P2: glossary. "Benilla" is a new term used in the docs, CI, test names and the Fake, but CONTEXT.md does not define it (AGENTS.md: "When you add or change a term, update CONTEXT.md in the same change").

Smells (judgement calls, P3)

• Primitive Obsession / Mysterious Name. Raw opcode literals where the repo uses *_OPCODE consts:

  • 0x02eb and 0x02aa in query.rs
  • 0x0155 in world_view.rs
  • 0x00a9 in subscriptions.rs
  • hdr.opcode == 0x0125 in world/mod.rs
  • about 15 in unavailable.rs, plus bare status codes (word(0x0212, 10))

  The Module has the same habit: item_flags & 0x20 and matches!(tmpl.class, 1 | 11) in inventory.rs, and effect_kind == 0x93 in spell.rs, although A_PERIODIC_TRIGGER exists. fn word(..) does not say what it builds.

• Duplicated Code.

  • In handlers/item.rs, five arms repeat if let Some(slot) = codec::inventory_slot(..) { .. } else { ItemRefusal::WrongSlot.into() }.
  • The bag-content loop in char.rs duplicates append_final_item_slots.

• Mysterious Name. In apply_item_move, the loop shadows src and to_slot with the destination item and from_slot. The roughly 90 lines of inline equip Gates in the same loop would read better as an equip_gates(..) helper.

• Divergent Change. handle_at_war lives in trainer.rs, and is_control_receipt lives in unavailable.rs.

• Repeated Switches. world/mod.rs has three raw pre-decoders: the at-war opcode check, raw_unavailable_outbound and read_movement_client.

• Message Chain. build_peer_create uses coord.0.coord().conn.db.game_entity_motion().guid().find(..) and compares floats with exact equality to decide whether the stored movement is still current.

• Family of booleans (CODING_STANDARDS.md, exhaustive state transitions). The shared Fake in world/tests.rs branches on self.benilla_gameplay.is_some() in several Store methods.

Spec

Checked and sound

• Swaps run every Gate in both directions (inventory.rs:150-260), so the old swap bypass is closed.
• Destroying equipped gear recomputes vitals and the Character sheet (inventory.rs:72-75).
• Channel cancel uses the new by_caster index (spell/control.rs:169-176).
• rested_bonus is appended last with a zero default, and Aura only gains an index, so both migrate safely.
• The durable CI filter matches the test names.
• The packet layouts look right for build 5875. This was checked against knowledge of the protocol, not a reference capture.

(c) Implemented but looks wrong

• P2: "Missing gameplay returns an explicit Refusal or System Message." The client sends some of these requests on its own, and each now posts "… are not available on this realm." to chat (handlers/unavailable.rs:68,74,77):

  • CMSG_STANDSTATECHANGE on every sit
  • CMSG_SET_ACTIONBAR_TOGGLES at UI load
  • CMSG_TUTORIAL_FLAG on tutorial triggers

  Before this change the Gateway ignored them silently. Handle them as silent receipts instead.

• P3: "cancel channels whose auras sit on another unit." spell_channel_outbound (subscriptions.rs:~1735) scans the whole game_aura table on each channel aura change, although the new caster index exists.

• P3. is_channel_aura (crates/lyracore-shared/src/spell.rs:6) now matches any aura of a CHANNELED spell, not only periodic energize. That fits the intent, but it changes behaviour and the description does not mention it.

(a) Missing or partial

• P2: "All 277 requests … have a dispatch pattern or a reviewed raw route." gateway/examples/benilla_inventory.rs:363-377 fails only on no_dispatch_pattern_found. Arms marked explicitly_ignored, and handlers that only log on failure, still pass, so the inventory does not enforce the "explicit Refusal or System Message" claim. Raw unavailable replies are also dropped when the session is not in world (world/mod.rs:1153-1159).
• P3. If the Character row is missing, the talent-reset quote closes gossip and sends nothing, so the player gets no Refusal (handlers/query.rs:328).

(b) Not in the description (scope creep, P3)

The following changes are reasonable, but the description should list them or they should move to their own PRs (see the Standards P1):

• CMSG_QUESTGIVER_REQUEST_REWARD
• logout cancel and logout
• CMSG_SET_ACTIVE_MOVER
• peer CREATE built from stored movement
• bag pointers at world entry
• no fall damage on transports
• use, equip and swap from inside bags

Some legacy rationale comments were deleted (the split anti-dupe note, cancel.rs, movement.rs). They recorded reasons, not just commentary.

---

Summary. Standards: 3 hard findings and 7 smells. The worst is P1: the PR bundles many unrelated changes into one commit. Spec: 8 findings and no blocker. The worst is P2: routine client requests (sitting, action bar toggles, tutorials) now post "not available" System Messages to chat.

---

Generated by Claude Code

[Mathih13]

Resolved the confirmed findings and rebased onto the Gateway Store family refactor in a6121982.

• Bind, talent reset and At-War requests now return a typed InteractionOutcome. Only Module Refusals become System Messages. Transport and infrastructure failures remain errors. A missing talent quote also gets an explicit reply.
• Automatic stand-state, action-bar and tutorial requests announce each missing feature once per World Session. This retains the requested missing-feature notice without repeating it on every request.
• The PR description now names quest reward requests, logout, active-mover receipts, world-entry projection and channel detection across all effects of a CHANNELED spell. The compatibility notes distinguish dispatch inventory from behavior verification and limit world-service replies to an in-world Character.

The independent adversarial reviews also found defects outside the original comment. The fixes preserve accepted trade terms, refuse conflicting hand equipment, validate the exact innkeeper in the Module, and parse GM ticket framing without decompressing untrusted attachments. Scheduled crafts consume all reagents atomically before charging power. Automatic grants refresh changed trade offers and clear both acceptances; failed craft restoration sends the restored quantities. Regressions reproduced both partial reagent loss and a stale offer quantity before the fixes.

I retained one PR because the requested change is complete compatibility with this pinned protocol target, including its verification and missing-system inventory. Benilla is the third-party dependency named and pinned in the compatibility notes. It introduces no new gameplay concept. The remaining naming and file-organization suggestions do not identify a behavior defect; the rebase uses the new Store family structure.

The channel Relay's Gateway bindings expose only the primary-key accessor, so the Module's caster index is unavailable there. The scan runs only after the channel/caster gates. The exact movement comparison checks whether the stored projection still describes the same serialized values.

Validation and the final independent Standards and Spec review results are recorded in the PR description. No production Realm was touched.

[Mathih13]

Fixed the durable (2) failure in 991e9e50.

The unchanged random-property healing test expected 240 health but observed 242. A forced five-second pause reproduced the exact failure: scheduled regeneration raised health from 100 to 102 before the spell correctly added 140. The test now stops the creature tick in its private fixture before measuring the heal. The separate regeneration test still runs its tick, and production code is unchanged.

The forced-delay reproduction passes with the fix. All four random-property durable tests, Module test Clippy and formatting passed after removing the temporary instrumentation. An independent adversarial review found no issue with the isolation or remaining coverage.

[Mathih13]

Fixed the durable mount test failure in 87fafbca.

The untrained-mount refusal test compared power across two reads while scheduled rage decay was running. A temporary five-second delay reproduced the CI result exactly: power was already 950 instead of 1000 before gw_use_item ran. The request did not spend it.

The test now stops the creature tick in its private Module instance before taking the snapshots. The same delayed reproduction passes. All refusal, item, vitals and aura assertions remain. The temporary delay and logging are removed.

All three durable mount tests, Clippy, formatting and an independent adversarial review passed. Production behavior is unchanged.