# Auto-order Stonehoof per-attacker triggers with 10+ attackers (#5335)#5399
Conversation
…ckers (phase-rs#5335) Per-attacker AttackersDeclared fan-out is certified order-independent so Stonehoof grants resolve without a spurious OrderTriggers prompt, and ability_rw profiles TriggeringSource static grants against the attacking creature rather than the source. Co-authored-by: Cursor <cursoragent@cursor.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses issue #5335 by certifying per-attacker attack triggers as order-independent and updating ability_rw to resolve TriggeringSource from nested transient statics when the outer target is None. It also adds comprehensive integration tests and updates the combat harness to drain attack triggers before blockers are declared. Feedback on the changes highlights a critical issue in ability_rw.rs where only the first static ability's target is profiled when the outer target is None, which could lead to under-profiling and false auto-ordering if multiple static abilities exist; a fix is suggested to collect and profile all unique affected targets.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
[HIGH] The disjoint attack fan-out gate only checks each event object against its own trigger source, but then suppresses the legacy event-context prompt for the whole group. Evidence: crates/engine/src/game/triggers.rs:3576 says each event object must be disjoint from every member source, but the implementation only rejects ctx.pending.source_id == attacker at crates/engine/src/game/triggers.rs:3593, while the group-level event_object_excludes_sources used at crates/engine/src/game/triggers.rs:3793 is computed from only the first event object at crates/engine/src/game/triggers.rs:3738. Why it matters: a mixed group of same-controller attack triggers can contain a later single-attacker event whose attacker is another member source, and this path can still drop legacy_batch_prompt, so profiles_conflict treats event-object writes as source-disjoint and may auto-order an order-observable trigger group. Suggested fix: build the set of all group source ids and require every per-trigger attacker/event object to be absent from that set before setting disjoint_per_attacker_fanout, or keep legacy_batch_prompt unless that all-events disjointness proof is available.
Parse changes introduced by this PR · 23 card(s), 32 signature(s) (baseline: main
|
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Maintainer review: current head resolves the prior fan-out blocker by checking each event object against the whole trigger-source set, adds a negative mixed-source regression plus Stonehoof runtime coverage, and strips the root PR note artifact. Auto-merge can wait for the restarted checks.
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Maintainer fixup: current head narrows the per-attacker auto-order shortcut to the proven Stonehoof fanout case, leaving the broader distinct-event C2 guard intact; auto-merge can wait for restarted checks.
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Maintainer handler re-approval for current head: security scan clean, right-seam trigger/ability_rw review clean, discriminating Stonehoof runtime coverage present, CI green.
2 participants
Auto-order Stonehoof per-attacker triggers with 10+ attackers (#5335)
Summary
Declaring combat with Stonehoof Chieftain and 10+ other attackers surfaced a spurious
OrderTriggersprompt and left attackers without trample/indestructible before blockers — trample damage never reached the defending player. This fix certifies per-attacker attack fan-out as order-independent (CR 603.3b / CR 508.3) and teachesability_rwto readTriggeringSourcefrom nested transient statics.Fixes #5335.
Root cause
Stonehoof's attack trigger fires once per attacking creature you control. With 10+ raiders:
Trigger ordering — each sibling carries a distinct single-attacker
AttackersDeclaredevent, but the AST usesTriggeringSourceevent context. The C2 fail-closed walker andlegacy_batch_promptparity gate treated the batch as conflict-prone, forcing anOrderTriggersprompt even though each trigger only grants keywords to its own event object (commutative under CR 603.3b).Conflict analysis —
GenericEffectstatic grants carryaffected: TriggeringSourceon the nested static, not on the outertargetslot.ability_rwdefaulted toSelfRef(Stonehoof), so the batch profiler incorrectly flagged write conflicts between siblings.Combat harness —
run_combatdid not drain attack triggers before declare blockers, so integration tests (and gameplay) could reach blocking with an unresolved Stonehoof stack.Changes
Engine — trigger ordering (
triggers.rs)attack_triggers_have_disjoint_event_objectsto detect per-attacker fan-out where each event object is a distinct attacker disjoint from all trigger sources.legacy_batch_promptand auto-order without player input.attack_triggers_have_disjoint_event_objectsbuilds the full group source-id set and requires every per-trigger event object to be absent from that set (not only absent from its own trigger source). The first-event-onlyevent_object_excludes_sourcesconjunct is no longer reused for this fast path.trigger_events_match_for_orderingwhen batch profiler certifies conflict-clean commutation.Engine — ability read/write profiling (
ability_rw.rs)GenericEffectprofiling now falls back to nestedstatic_abilities[].affectedwhen the outertargetisNone, soTriggeringSourcegrants profile as writes to the triggering attacker, not Stonehoof.Tests
issue_5335_stonehoof_mass_attack.rs: three regressions — noOrderTriggersprompt with 10 attackers, trample+indestructible on each attacker after triggers resolve, 18 trample damage to player when nine attackers go unblocked.triggers.rsunit tests: mixed fan-out where an event object is another member's source must prompt; ten-raider homogeneous fan-out passes the disjoint gate.rules.rs:run_combathelper drainsOrderTriggersif present and empties the attack-trigger stack before declare blockers.Out of scope
AttackersDeclaredfan-out with disjoint event objects — still fail-closed through C2 /legacy_batch_prompt.Test plan
cargo fmt --allcargo test -p engine attack_triggers_disjointcargo test -p engine --test integration issue_5335(3/3)test-enginegreen after mergeRisk notes
ability_rwfallback only applies whenGenericEffect.targetisNoneand a nested static carriesaffected— player-chosen targets on the outer slot are unchanged.