Repository navigation
fix(status): stop generic decay from racing the web struggle branch - #166
Conversation
…161) Two systems decremented :webbed each turn: the movement struggle branches (world/player-move, ai/try-move), which trample the web on the last struggle, and the generic status tick, which removes an expired status silently. The ttl drained twice per turn and always expired inside the tick, so the trample branch never ran and every web survived its captive breaking free. PR #71 exposed this on the player path by softening the web ttl 3 -> 2 (at 3 the struggle branch happened to win the race); the monster path (tick before action in creature-turn) has never trampled since webs landed in 8feaa1c. Statuses can now declare :no-decay — the generic tick leaves their ttl to the owning system. :webbed's ttl becomes what the balance knob always looked like: the number of struggles to break free. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
FYI — initia self-review notes. Root cause confirmed. Validated. Full suite green (299 tests / 2777 assertions), including One behavior change to decide on: breaking free now requires a move attempt. Resting/waiting no longer decays the web, and a webbed monster that only sleeps or attacks in place stays webbed indefinitely. This matches the documented intent ( Minor: Not a recent regression: the double-decrement dates to the original web commit ( |
|
Review context for this and the other five ready PRs, with suggested order and merge-order notes: #182 |
nnunley
left a comment
There was a problem hiding this comment.
Sorry for the delay. I think this is probably the wrong solution for this problem. I think that there should be a consolidation pass that's idempotent for the effects of actions within the turn (order of application should preferably not matter within the tick). The web decay's tick should be calculated as a forward projected time in rounds/ticks
Note that a player turn should take some multiple > 1 worth of ticks (like maybe 1000) to allow spells like haste to actually do something interesting. Trampling should have at most one entity affecting the object, and should have a handler for not just webs, but grasses, water, etc.
I'm willing to let this pass, but there feels like some hackery going on to get things working.
Thanks for the great feedback. Let me take a fresh look and see if it makes sense to move this forward or revise. |
|
@nnunley Thanks again for the review. Here's the longer response after looking through the code with Claude. I tested the "mutating state multiple times" half of your point and it doesn't hold for the per-turn counters: just two places outside
Every other status goes through I didn't test the order-dependence half — that's a different property, and single-application doesn't imply order-independence. On the tick model I think you're right: It's also what stands between us and your 1000-ticks-per-turn idea: haste can't mean much while decay counts actions rather than ticks. And we have this asymmetry: player ticks statuses inside Projected time would be the wrong fit for So unless the above changes your mind, I'll merge this as the narrow fix, since #161 is a live regression and the new assertions fail on That said, if you prefer |
|
One amendment to my own survey. The order-dependence half is already tracked: #187 is that case. I filed that on 2026-08-20, before your review, so your instinct had more behind it than my "I didn't test that half" line suggested. It goes into the consolidation-pass issue as the order half. |
nooga
left a comment
There was a problem hiding this comment.
Reviewed via automated multi-angle analysis (line-by-line, removed-behavior, cross-file trace, cleanup, efficiency, altitude, CLAUDE.md conventions) plus a full test-suite run.
Verdict: approve. The fix is correct — :no-decay cleanly exempts :webbed from the generic per-turn tick so it's only cleared by the struggle-resolution branches, closing the double-decrement race. Full suite passes (318 tests / 2825 assertions), including the two new regression tests.
Two non-blocking notes for a follow-up, not this PR:
xsofy/status.lg:63—tick-entity-statuseslooks upstatus-defstwice per status per tick (once for the:no-decaycheck, again a couple lines later asdef_). Minor hot-path inefficiency; hoisting the lookup above theifwould remove the redundant map lookup.xsofy/status.lg:24—:webbednow has no time-based expiry at all; it only clears via the struggle branches, which only fire on a move attempt. An entity that never moves again (docile AI, or a player who only attacks/waits) stays webbed indefinitely. This is likely intended ("only struggling frees you"), but it's a real behavior change from before and isn't called out in the docstring/comment — worth a one-line note.
🤖 Generated with Claude Code
Dismissing per maintainer decision: this fixes a live, single-cause regression (#161) with a narrow, well-tested mechanism. mparrett's investigation confirmed :webbed is the only status with two ttl owners, so this isn't the systemic pattern the requested consolidation pass would need to address. The legitimate parts of the feedback (tick-scaled decay for haste, generalized trampling beyond webs, order-dependence) are being tracked in a follow-up issue instead of blocking this fix. You said you were willing to let this pass — merging on that basis.


Closes #161. Full walkthrough of the mechanism is in the issue comment; short version:
:webbedttl was decremented twice per turn — once by the movement struggle branches (the only place that tramples, at ttl ≤ 1) and once by the generic status tick (which removes an expired status silently). The status always expired inside the tick, so the trample branch was unreachable. #71's ttl reduction (3 to 2) exposed it on the player path; the monster path (tick before action increature-turn) had never trampled since webs landed.Note: Temporarily changed ash from . to , in order to disambiguate and verify the changes. Not included in this PR, but might be worth discussing when we get back to gameplay aesethetics.
Approach
Statuses can declare
:no-decayinstatus-defs, and:webbeddoes: the generic tick skips it, and the struggle branches become the sole owner of the ttl. One mechanism fixes both paths, and the ttl now means what the #71 balance knob assumed: the number of struggles to break free. The alternative — trampling on expiry inside the tick — would have coupledstatus.lgtofire.lgand kept two owners of the same counter.Behavior note (balance-visible): a webbed creature that never attempts a move — say, attacking an adjacent target — now stays webbed until it does, instead of timing out of the web by standing still. That reads as correct to me; easy to revisit if it plays badly.
Tests
New
xsofy/test/status_test.lg: the generic tick leaves:webbeduntouched while:confusedstill decays, and a player struggling free ends on ash, not an intact web. Suite is green (299 tests, 2777 assertions); the new assertions fail on main.🤖 Generated with Claude Code