# Research: Combat Conditions ## Decision 1: Condition Data Representation **Decision**: Store conditions on `Combatant` as `readonly conditions?: readonly ConditionId[]` where `ConditionId` is a string literal union type of the 15 condition identifiers. **Rationale**: A typed union ensures compile-time safety — invalid condition strings are rejected by the type checker. Using an array (not a Set) preserves JSON serialization compatibility with existing localStorage persistence. The array is kept sorted in definition order by domain operations, so display order is deterministic. **Alternatives considered**: - `Set`: Not JSON-serializable without custom logic; breaks existing transparent persistence pattern. - `Record`: More verbose, harder to iterate for display, no inherent ordering. - Branded type `ConditionId`: Possible but overkill — a string literal union provides equivalent safety with less ceremony. ## Decision 2: Domain Operation Design **Decision**: Implement `toggleCondition(encounter, combatantId, conditionId)` as the sole operation. It adds the condition if absent, removes if present, and always returns the array in fixed definition order. Both the picker and the click-to-remove tag flow use toggle. **Rationale**: The picker UI toggles conditions on/off, making a single toggle operation the natural fit. The combat row click-to-remove also uses toggle — since the condition is guaranteed to be active when its tag is visible, toggle will always remove it. A separate `removeCondition` operation was considered but deemed unnecessary for a single-user local app. The toggle emits distinct events (`ConditionAdded` / `ConditionRemoved`) depending on direction. **Alternatives considered**: - Separate `addCondition` + `removeCondition` only: Would require the picker UI to check current state before deciding which to call — pushes domain logic into the adapter layer. - `toggleCondition` + separate `removeCondition`: The dedicated remove would error if condition not active, preventing accidental re-add from stale state. Rejected as overengineering for a single-user local app. ## Decision 3: Condition Registry Location **Decision**: Define the condition registry (id, display name, icon component reference, color class) in a new domain file `conditions.ts`. The registry is a static readonly array, not a lookup map. **Rationale**: The registry is pure data with no I/O — it belongs in the domain layer. An ordered array naturally encodes the fixed display order (FR-003). The web layer imports the registry to map condition IDs to icon components and colors. **Alternatives considered**: - Registry in web layer only: Would violate the principle that domain defines the canonical condition set. Ordering logic would leak into the adapter. - Registry in application layer: No benefit over domain; application should not own entity definitions. ## Decision 4: Condition Icon/Color Mapping Architecture **Decision**: The domain `conditions.ts` defines condition metadata (id, label, iconName as string, colorClass as string). The web layer maps `iconName` strings to actual Lucide React components at render time. **Rationale**: Domain must not import React components (layer boundary). Storing icon names as strings keeps domain pure. The web layer maintains a simple `Record` lookup from icon name to component. **Alternatives considered**: - Domain exports icon components directly: Violates layered architecture — domain cannot import from `lucide-react`. - Separate mapping file in web layer only: Would duplicate the condition list, creating sync risk. ## Decision 5: Persistence Compatibility **Decision**: No migration needed. The `conditions` field is optional (`conditions?: readonly ConditionId[]`). Existing persisted encounters without conditions will rehydrate with `conditions: undefined`, which the UI treats as "no conditions." Rehydration validates that stored condition values are valid `ConditionId` strings, filtering out any unknown values. **Rationale**: Follows the same pattern as `ac?` field added in feature 016. Optional fields with undefined defaults require zero migration. **Alternatives considered**: - Default to empty array `[]`: Would work but deviates from the established optional-field pattern (ac, maxHp, currentHp all use `undefined`).