feat(engine): grid-461 PR-2 — AoE templates on the combat grid (#1251)#1257
Conversation
Adds sphere/cone/line area-of-effect templates on the (x,y) combat grid and wires them into cast_spell. When a grid cast passes origin=[x,y], the engine projects the spell's SRD shape onto the cells, returns affected_tile_coords, and auto-resolves save-for-half over the occupants caught (reusing the existing target_ids multi-target loop). - combat_grid: pure sphere_cells / cone_cells / line_cells (Chebyshev metric, edge-clipped to grid bounds); 8-facing cone/line snapping. - server: _aoe_template_cells maps SRD shape_type/shape_size -> geometry; cast_spell gains one ListArg param (origin). Additive/default-off: no origin (or off-grid, or an explicit target_ids) == byte-identical to today. - Line-of-effect (walls between origin and a cell) stubbed permissive, TODO(#1252). Schema cost: +242 B on the engine tool-schema slab (headroom 1565 -> 1323 B, under the 120 KB budget). Part of #1100.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Review rework on PR #1257: remove the caster-cell carve-out from the AoE occupant loop. Per SRD 5.2 a creature within a spell's area is affected and the caster is not exempt (Fireball centered on yourself). The prior carve-out was also internally inconsistent — a companion on the caster's cell was hit while the caster was not. The caster now rides the same save-for-half loop; the DM narrates. Cone/line geometry already excludes the emitter cell, so a cone/line still never catches its own caster. Tests: sphere centered on the caster -> caster appears in the resolved targets and takes a save; cone/line assert the caster stays excluded (no regression).
evaOS review status: completedPR: #1257 - feat(engine): grid-461 PR-2 — AoE templates on the combat grid (#1251) evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #1257 Review URL: #1257 (review) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d574333d37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for cb in c.combat.order: | ||
| if cb.character_id == character_id: | ||
| continue |
There was a problem hiding this comment.
When the caster stands inside their own sphere template, such as a Fireball centered on their cell or any burst whose affected_tile_coords includes the caster, this unconditional skip keeps them out of aoe_targets. cast_spell then spends the slot and auto-damages every other occupant while making self-centered/close bursts risk-free; the caster should only be excluded for emitter-only shapes like cones/lines, not for sphere templates that actually cover their cell.
Useful? React with 👍 / 👎.
| if shape == "cone": | ||
| return combat_grid.cone_cells(src, aim, size, w, h, cs) |
There was a problem hiding this comment.
When origin is supplied for a touch spell whose cone is granted to another creature, such as Dragon's Breath, the SRD record still has shape_type == 'cone', so this projects the cone from the caster and the existing AoE loop immediately applies the save-for-damage result. A grid cast of that spell on an ally can therefore damage creatures in front of the caster on the casting turn instead of setting up the touched creature's later breath action.
Useful? React with 👍 / 👎.
| if shape == "sphere": | ||
| # A burst centred on the aim cell — origin-independent (no facing needed). | ||
| return combat_grid.sphere_cells(aim, size, w, h, cs) |
There was a problem hiding this comment.
When origin is passed for a delayed sphere such as Delayed Blast Fireball, this branch projects it like an immediate Fireball, and the new origin handling feeds every occupant into the existing save-for-damage loop during the cast. That consumes the slot/concentration and deals the 12d6 immediately, even though this spell should create a bead that explodes only when the spell ends or is touched.
Useful? React with 👍 / 👎.
![evaos-code-review-bot[bot]](https://avatars.githubusercontent.com/in/4184532?s=60&v=4){kind=small}
There was a problem hiding this comment.
Walkthrough
PR: #1257 - feat(engine): grid-461 PR-2 — AoE templates on the combat grid (#1251)
Head: 49ebecb6dfe03a1cf2b0f688f9cac5c7cf12339f into main. Review event: COMMENT.
Estimated review effort: 1/5 (~10 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|
Review Signal
No validated inline findings.
Dropped findings before posting: 2. High-severity findings: 0.
Risk Taxonomy
No finding categories.
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: Unity editor or Play Mode smoke - WorldOS repo profile implies Unity runtime risk. Proof: Unity editor smoke; Play Mode log; scene/prefab screenshot or recording.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Prefer correctness, persistence, CI, release, and regression findings over style-only feedback.
Profile proof expectations: Look for Unity editor, play-mode, fixture, or focused smoke evidence when runtime behavior changes.
Related Context
Related issues/PRs: #1251, #1100, #1252, #1253, #1254.
Suggested labels: none.
Suggested reviewers: none from current metadata.
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
* feat(engine): grid-461 PR-3 — cover + line-of-sight (#1252) Adds terrain-aware line-of-sight and SRD 5.2 cover on the combat grid, building on the #1257 AoE templates. combat_grid.py (pure): - _supercover_cells / line_blockers: cell->cell supercover ray over the grid's blocking (impassable) cells. Corner-graze tie-break is permissive — a diagonal graze severs the ray only when BOTH shoulder cells of the step are blockers (a lone diagonal blocker is slipped past). Symmetric. - has_line_of_effect: True iff the ray crosses no blocker (endpoints excluded). - cover_between / cover_ac_bonus: 0 blockers=none, 1=half(+2), 2+=three- quarters(+5), a fully-walled ray=total (untargetable). server.py (wiring, additive/default-off): - cast_spell: cull AoE cells with no line of effect from the template origin (the deferred #1257 TODO) — a wall shields cells behind it. - attack: when grid_enabled AND both combatants placed, fold cover AC into the target AC (half/three-quarters), refuse a total-cover shot up front, and surface a result key. Off-grid / unplaced == byte-identical. No new tool params (cover derives from existing state; surfaced in the result payload) — tool-schema byte delta 0. Part of #1100. * fix(engine): trace AoE line-of-effect from the spell's point of origin (#1252) Rework per review: the LoE cull traced every shape from the aim cell, which is SRD-correct for a sphere (the burst's origin IS the aim) but wrong for a cone/line, whose point of origin is the CASTER. A Burning Hands aimed past a wall would leak onto cells beyond the wall — unreachable from the caster but passing an aim-origin ray. _aoe_template_cells now returns (cells, loe_origin): aim for sphere, the caster's cell (src) for cone/line. cast_spell culls with that origin, keeping the 'origin cell always retained' semantics relative to loe_origin. Tests: cone aimed across a wall (cells behind the wall culled even though an aim-origin ray would keep them) + line stops at a blocker; sphere behaviour and all existing AoE tests unchanged. --------- Co-authored-by: Eva <arncalso@gmail.com>
1 participant
What
grid-461 PR-2 — geometric area-of-effect templates (sphere / cone / line) on the
(x,y)combat grid, mapped to affected cells + multi-target resolution for area spells. Closes #1251; part of the combat epic #1100.Given a grid cast with an
origin=[x,y]aim cell, the engine projects the spell's SRD 5.2 shape onto the grid, returnsaffected_tile_coords, and auto-resolves save-for-half over the occupants standing in those cells.Approach — template geometry
Pure, I/O-free helpers in
combat_grid.py(mirroring the PR-1 movement-spine style), all using the module's one Chebyshev distance metric and edge-clipped to the grid extents:sphere_cells(center, radius_ft, …)— cells withinradius_ft/cell_sizeking-moves of the burst point. A Fireball bursts atorigin(origin-independent).cone_cells(origin, toward, length_ft, …)— SRD cone (width == distance from the point) snapped to one of the 8 grid facings from the caster towardorigin; 1:1 widening quadrant fan; the emitter cell is excluded.line_cells(origin, toward, length_ft, width_ft, …)— a linelength_ftlong ×width_ftwide, thickened symmetrically about the centre line, along the 8-facing towardorigin.server._aoe_template_cellsmaps the SRD record'sshape_type/shape_size(feet) onto those helpers. The occupants of the covered cells feed the existingtarget_idsshared-roll save-for-half loop — no new state writer, no new damage path.Invariants
cast_spellwrite path. No newsave_campaigncallers.origin(or off-grid, or an explicittarget_ids) ⇒ byte-identical to today. Regression tests assert no template keys leak and the explicit-list path is untouched.target_id/target_idssemantics; added exactly oneListArgparam (origin). Schema slab grew +242 B (headroom 1565 → 1323 B, under the 120 KB budget;test_tool_schema_budgetgreen).TODO(#1252).Tests
New
tests/test_grid_aoe.py(18 tests): sphere/cone/line cell-set geometry at several origins + facings (orthogonal & diagonal), edge clipping at grid bounds; end-to-end Fireball-class cast hitting exactly the template occupants; honest save-for-half accounting; and byte-identical regression proofs (no-origincast + explicit-target_idscast unchanged).(run with
-p no:xdist, single-process.)Part of #1100.