Implement the 003-remove-combatant feature that adds the possibility to remove a combatant from an encounter
This commit is contained in:
48
specs/003-remove-combatant/research.md
Normal file
48
specs/003-remove-combatant/research.md
Normal file
@@ -0,0 +1,48 @@
|
||||
# Research: Remove Combatant
|
||||
|
||||
**Feature**: 003-remove-combatant
|
||||
**Date**: 2026-03-03
|
||||
|
||||
## R1: activeIndex Adjustment Strategy on Removal
|
||||
|
||||
**Decision**: Use positional comparison between removed index and activeIndex to determine adjustment.
|
||||
|
||||
**Rationale**: The spec defines five distinct cases based on the relationship between the removed combatant's index and the current activeIndex. These map cleanly to a single conditional:
|
||||
|
||||
1. **Removed index > activeIndex** → no change (combatant was after active)
|
||||
2. **Removed index < activeIndex** → decrement activeIndex by 1 (shift left)
|
||||
3. **Removed index === activeIndex and not last** → keep same index (next combatant slides into position)
|
||||
4. **Removed index === activeIndex and last** → wrap to 0
|
||||
5. **Last remaining combatant removed** → activeIndex = 0
|
||||
|
||||
This mirrors the inverse of addCombatant's "always append, never adjust" approach — removal requires adjustment because positions shift.
|
||||
|
||||
**Alternatives considered**:
|
||||
- Storing active combatant by ID instead of index: Would simplify removal but requires changing the Encounter type (out of scope, breaks existing advanceTurn).
|
||||
- Emitting a TurnAdvanced event on active removal: Rejected — spec explicitly says roundNumber is unchanged, and the next-in-line simply inherits.
|
||||
|
||||
## R2: CombatantRemoved Event Shape
|
||||
|
||||
**Decision**: Follow the existing event pattern with `type` discriminant. Include `combatantId` and `name` fields.
|
||||
|
||||
**Rationale**: Consistent with `CombatantAdded` which carries `combatantId`, `name`, and `position`. For removal, `position` is less meaningful (the combatant is gone), so we include only ID and name.
|
||||
|
||||
**Alternatives considered**:
|
||||
- Including the removed index: Rejected — the index is ephemeral and not useful after the fact.
|
||||
- Including the full Combatant object: Over-engineered for current needs; ID + name suffices.
|
||||
|
||||
## R3: Use Case Pattern
|
||||
|
||||
**Decision**: Mirror `addCombatantUseCase` exactly — `store.get()` → domain function → `store.save()` → return events.
|
||||
|
||||
**Rationale**: No new patterns needed. The existing use case pattern handles the get-transform-save cycle cleanly.
|
||||
|
||||
## R4: UI Pattern for Remove Action
|
||||
|
||||
**Decision**: Add a remove button next to each combatant in the list. The button calls `removeCombatant(id)` from the hook.
|
||||
|
||||
**Rationale**: Minimal UI per spec. No confirmation dialog needed for MVP (spec doesn't require it). Mirrors the simplicity of the existing add form.
|
||||
|
||||
**Alternatives considered**:
|
||||
- Confirmation modal before removal: MVP baseline does not include this; can be added later.
|
||||
- Swipe-to-remove gesture: Not applicable for web MVP.
|
||||
Reference in New Issue
Block a user