Skip to content

Enforce StatusEffect.stacking (refresh replaces, resets expiry) when an effect is applied #195

Description

@The-Running-Dev

10-simulation-kind.md §6.1 (design/20-contract.md:2852-2854):

Stacking is governed by StatusEffect.stacking (§2.3): a second effect from the same sourceId with "refresh" replaces the first and resets its expiry; "stack" adds a second, independent layer. Two different sources always stack.

Flagged by qodo-code-review on PR #194 against src/engine/src/kinds/simulation/modifiers.ts:26 (collectModifiers): the collector applies every matching modifier without regard to StatusEffect.stacking, so two same-sourceId "refresh" effects in activeEffects would both contribute instead of the newer one replacing the older.

Classified out of scope for W51/PR #194 rather than a defect there, because the fix cannot live in collectModifiers. That function is a pure, read-time collector over an already-given activeEffects list — it has no way to "reset expiry" (a mutation of a stored StatusEffect). The stacking rule is inherently an insertion-time invariant: whatever adds a new StatusEffect to activeEffects (an item purchase, job/course effect, event, etc.) must, when the new effect's sourceId matches an existing one with stacking: "refresh", remove/replace the older entry rather than leave both in the list.

No such "apply effect" code exists yet anywhere in src/engine/src/kinds/simulation/ — W51's own commit message says as much ("No job/course content is wired... out of scope"). initial.ts sets activeEffects: [] and nothing else currently writes to it. This is a gap for whichever future slice adds the mechanism that inserts a StatusEffect into activeEffects (job/course/item/event application).

Suggested fix, when that slice is cut: at the insertion site, before pushing the new StatusEffect, drop any existing entry in activeEffects whose sourceId matches and whose stacking is "refresh". Add tests covering same-source refresh (replaces), same-source stack (both persist), and different-source (always stack) per the contract text above.

Done when

  • The (future) effect-application code path enforces refresh replacement / stack independence per §6.1
  • Tests cover same-source refresh, same-source stack, and different-source effects

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions