diff --git a/TASKS.md b/TASKS.md index 76d9e2d..fdcab12 100644 --- a/TASKS.md +++ b/TASKS.md @@ -21,24 +21,39 @@ The initial scope is only roll definition, validation, execution, logging, and U - Recursive follow-up rolls are high-ended only. - Standard percentile rolls do not open-end and do not use a fumble range. - Initiative rolls are simple `2d10 + x` rolls with no open-ended behavior. -- The modifier `x` should be an integer and may be negative if we want Rolemaster penalties to be expressible. Current generic dice validation only allows non-negative modifiers, so this must be revisited. +- The modifier `x` should be an integer. +- Negative modifiers are supported for Rolemaster only. +- D6 and D&D 5e keep their current non-negative modifier rules unless separately re-scoped in a future change. - Existing D6 and D&D 5e behavior must remain unchanged. -## Open Questions To Resolve Before Implementation +## Clarifications -- Should the Rolemaster modifier allow negative values everywhere, or only for Rolemaster skills? -- What is the allowed fumble range for open-ended percentile skills: - - `0-5` - - `0-10` - - unrestricted `0-100` -- Should a low-ended first roll that is also `96+` ever be possible through configuration overlap: - - recommended answer: no, validation should prevent impossible overlap by constraining fumble range below `96` -- How should roll breakdown text read for low-ended subtraction: - - recommended format: `12 - (97 + 44) + 15 = -114` -- Should initiative be represented as a dedicated Rolemaster roll type, or as a generic fixed expression with Rolemaster labeling: - - recommended answer: dedicated type, because UI copy and validation differ -- Do skill groups need to support Rolemaster defaults for all of these fields: - - recommended answer: yes, to stay consistent with current prototype behavior +- The Rolemaster modifier should allow negative values only for Rolemaster. +- The standard fumble range for open-ended percentile skills is `0-5`, but can be configured individually for each skill. +- The fumble range has to be guaranteed to be less than 96. +- Roll breakdown text format for low-ended subtraction: `12 (03) -97 -44 +15 = -114` +- Initiative should be represented as a dedicated Rolemaster roll type, because UI copy and validation differ +- Skill groups need to support Rolemaster defaults for all of these fields, to stay consistent with current prototype behavior +- Rolemaster uses expression parsing with canonical syntax: + - initiative: `2d10+48` + - standard percentile: `d100+4` + - open-ended percentile: `d100!+85` + - negative modifiers are valid only for Rolemaster, for example `d100-15` + +## Architecture Guardrails + +- Keep minimal API endpoint handlers thin and continue to place gameplay validation and roll execution in services. +- Keep Blazor components focused on form state, rendering, and API orchestration; do not duplicate Rolemaster rules in component code. +- Keep request/response DTOs separate from domain/persistence models. +- Preserve the current lazy log-detail loading flow so Rolemaster detail stays out of hot list payloads. +- Prefer additive changes to contracts and storage over replacing current D6 / D&D shapes outright. + +## Backward Compatibility + +- Existing D6 and D&D 5e API request and response fields must remain valid throughout the Rolemaster rollout. +- Existing persisted skills, skill groups, and roll log rows must continue to load without migration-time data loss. +- Any richer die metadata must deserialize safely for old rows that only contain the current `RollDieResult` shape. +- Payload budgets for roster, log page, roll mutation responses, and lazy-loaded roll detail must remain explicitly guarded by tests. ## Design Direction @@ -66,7 +81,7 @@ The current model is D6-shaped: - `WildDice` - `AllowFumble` -That shape is not a clean fit for Rolemaster. The Rolemaster extension should move toward a ruleset-aware skill definition that can express roll behavior directly instead of encoding everything into a free-form expression string. +That shape is not a clean fit for Rolemaster. The Rolemaster extension should still preserve expression parsing, but it needs a ruleset-aware internal definition so the app can validate, execute, and display Rolemaster rolls safely. Recommended model additions: - `SkillRollKind` enum @@ -74,18 +89,19 @@ Recommended model additions: - `RolemasterInitiative` - `RolemasterPercentile` - `RolemasterOpenEndedPercentile` -- `RollModifier` integer - `RolemasterFumbleRange` nullable integer -- keep `DiceRollDefinition` only if needed for legacy display or migration compatibility +- keep `DiceRollDefinition` as the canonical persisted expression across rulesets +- derive `SkillRollKind` and `RollModifier` from parsed expressions unless persisting them provides a concrete implementation benefit Recommended direction: - Keep existing fields working for D6 and D&D 5e. -- Add new fields for Rolemaster rather than overloading `WildDice` / `AllowFumble`. -- Centralize skill validation into a single ruleset-aware validator that returns a canonical internal representation. +- Preserve `DiceRollDefinition` as the authoritative user-facing input for D6, D&D 5e, and Rolemaster. +- Add only the extra Rolemaster fields that cannot be encoded cleanly in the expression itself, specifically the fumble range. +- Centralize parsing and validation into a single ruleset-aware validator that returns a canonical internal representation. Tasks: - Define a ruleset-aware skill definition model in domain/contracts. -- Decide whether to preserve `DiceRollDefinition` as persisted state or derive display text from structured fields. +- Keep `DiceRollDefinition` as persisted state and define canonical formatting rules for Rolemaster expressions. - Add mapping helpers so UI and API get a stable, ruleset-aware DTO. - Ensure skill groups can store the same Rolemaster defaults as skills. @@ -115,15 +131,22 @@ Validation rules to add: - fumble range must be below `96` - reject D6-only options for Rolemaster skills - reject Rolemaster-only fields for D6 / D&D 5e skills +- reject negative modifiers for D6 / D&D 5e expressions +- accept negative modifiers for Rolemaster expressions only Tasks: - Replace or extend `ValidateSkillDefinition`. -- Add a Rolemaster-focused parser/validator path. -- Decide whether `DiceRules.ParseExpression` should remain only for free-form rulesets or become a lower-level helper. +- Extend `DiceRules.ParseExpression` with a Rolemaster-focused parser/validator path. +- Add canonical Rolemaster expression parsing rules: + - `2d10+48` and `2d10-15` for initiative + - `d100+4` and `d100-20` for standard percentile + - `d100!+85` and `d100!-15` for open-ended percentile +- Keep Rolemaster fumble range validation separate from the expression because it is configured independently. - Add canonical display formatting for Rolemaster skills, for example: - `Initiative: 2d10+15` - `Percentile: 1d100+48` - `Open-ended percentile: OE 1d100+48, fumble <= 5` +- Add explicit regression tests that D6 / D&D still reject negative modifiers after Rolemaster support lands. Affected areas: - `RpgRoller/Services/DiceRules.cs` @@ -160,13 +183,15 @@ Open-ended algorithm: Tasks: - Extend `ComputeRoll` dispatch. - Add Rolemaster-specific roll methods. +- Parse Rolemaster expressions into an internal roll definition before execution rather than branching on raw strings in multiple places. - Decide whether the current `RollDieResult` DTO is sufficient. - likely not sufficient, because it only has `Crit`, `Fumble`, `Wild`, `Removed`, `Added` - Recommended DTO extension: - `Sequence` or `Order` - `Phase` / `Kind` such as `initial`, `open-ended-high`, `open-ended-low-subtract` - `SignedContribution` or equivalent metadata for subtraction -- Update log summary generation so Rolemaster rolls remain understandable in compact log entries. +- Update log summary generation so Rolemaster rolls remain understandable in compact log entries without bloating list payloads. +- Keep lazy detail expansion as the place where the full Rolemaster breakdown and rich die metadata are loaded. Affected areas: - `RpgRoller/Services/GameService.cs` @@ -186,6 +211,7 @@ Tasks: - Decide migration defaults for legacy rows. - Update `SqliteSchemaUpgrader` coverage expectations if schema assumptions change. - Verify serialized roll log payloads remain readable for old entries after DTO extension. +- Ensure additive schema changes allow mixed old/new data during migration and startup upgrade. Data migration recommendations: - Legacy D6 / D&D 5e skills should retain current behavior with deterministic defaults. @@ -210,13 +236,16 @@ Tasks: Recommended frontend model changes: - Introduce ruleset-aware form state rather than raw D6 toggles. -- Replace "Expression" with a Rolemaster-specific editor when the selected campaign ruleset is `rolemaster`. +- Keep "Expression" as the primary input, but provide Rolemaster-specific help text, examples, and validation when the selected campaign ruleset is `rolemaster`. - Add a "Roll type" selector with options: - `Initiative` - `Percentile` - `Open-ended percentile` -- Show modifier input for all Rolemaster roll types. +- Keep the selector and expression input synchronized so users can either choose a type or see the parsed/canonical expression clearly. - Show fumble range input only for open-ended percentile. +- Use progressive disclosure so only the fields relevant to the chosen Rolemaster roll type are shown. +- Add inline validation on blur and submit, visible required indicators, and `aria-live` / alert semantics for validation errors. +- Preserve the existing app visual language instead of introducing a separate Rolemaster-specific theme. Affected areas: - `RpgRoller/Contracts/ApiContracts.cs` @@ -239,6 +268,7 @@ Tasks: - Add tooltip or title text that explains each die's role in the chain. - Update compact log summaries so they surface open-ended behavior. - Ensure private/public visibility behavior remains unchanged. +- Keep roll list rows dense and readable; reserve multi-line breakdown detail for expanded rows or detail fetches. Recommended display goals: - A normal percentile roll should read as simple and uncluttered. @@ -260,6 +290,8 @@ Service-level tests to add: - skill creation validation for each Rolemaster roll type - rejection of invalid fumble ranges - rejection of D6-only options on Rolemaster skills +- rejection of negative modifiers on D6 / D&D skills +- acceptance of negative modifiers on Rolemaster expressions - initiative roll math - standard percentile roll math - open-ended high roll with one extra roll @@ -274,11 +306,14 @@ API tests to add: - create/update Rolemaster skills - roll each Rolemaster skill type via API - verify log page and detail endpoints for Rolemaster rolls +- verify old D6 / D&D contracts still round-trip without requiring Rolemaster-only fields Frontend/regression checks: - Rolemaster ruleset appears in campaign creation UI - Rolemaster skill form shows correct conditional fields - roll results render correctly for open-ended cases +- validation errors are announced and visible for invalid Rolemaster expressions / fumble ranges +- the compact log remains readable and detail expansion shows the full Rolemaster breakdown Coverage-sensitive files likely needing tests: - `RpgRoller.Tests/Services/DiceRulesTests.cs` @@ -286,6 +321,7 @@ Coverage-sensitive files likely needing tests: - new dedicated `Rolemaster` service tests - `RpgRoller.Tests/Api/CampaignApiTests.cs` - `RpgRoller.Tests/HostingCoverageTests.cs` +- `RpgRoller.Tests/PayloadBudgetTests.cs` ### 9. Documentation updates @@ -307,11 +343,11 @@ Recommended implementation order: #### Iteration 1: ruleset plumbing - Add `Rolemaster` ruleset enum/id/display support. - Add campaign creation support. -- Add parser/validator scaffolding for Rolemaster roll types. +- Add parser/validator scaffolding for Rolemaster expressions and Rolemaster-only negative modifier support. - Add baseline tests for ruleset mapping and validation. #### Iteration 2: structured skill model -- Add persisted Rolemaster fields for skills and skill groups. +- Preserve `DiceRollDefinition` as the canonical persisted expression and add only the extra persisted Rolemaster fields the expression cannot represent. - Add migration and schema tests. - Update API contracts and service mapping. - Keep existing D6 / D&D behavior green. @@ -320,16 +356,18 @@ Recommended implementation order: - Implement initiative and standard percentile execution. - Implement open-ended percentile execution with recursive high-end chaining and low-end subtraction. - Extend die-result metadata and breakdown formatting. -- Add focused service tests for roll math. +- Add focused service tests for roll math and payload/detail serialization compatibility. #### Iteration 4: frontend editing - Update campaign ruleset selection UI. - Replace D6-only form branching with ruleset-aware skill editing. - Support Rolemaster skill groups and skills in create/edit flows. +- Add accessible inline validation and progressive disclosure to the Rolemaster editor. - Verify with ephemeral Playwright. #### Iteration 5: log/detail polish and docs - Improve roll chip rendering and compact summaries for Rolemaster. +- Re-run payload-budget checks and confirm lazy detail loading still holds. - Update README/OpenAPI/docs. - Run full local CI and coverage checks. @@ -337,9 +375,10 @@ Recommended implementation order: - The current skill contract is tightly coupled to D6-era fields, so a shallow patch will create long-term complexity. - The current `RollDieResult` type is too generic for clear Rolemaster open-ended auditing. -- Allowing negative Rolemaster modifiers may require carefully scoping validation changes so D6/D&D rules do not loosen unintentionally. +- Allowing negative Rolemaster modifiers requires carefully scoping validation changes so D6/D&D rules do not loosen unintentionally. - Migration design needs care to avoid breaking legacy SQLite databases. - UI complexity will grow if ruleset-specific fields are bolted onto the current `IsD6` logic instead of replacing it with a ruleset-aware form model. +- Richer Rolemaster die metadata could regress payload sizes or list readability if detail is not kept behind the existing lazy-load flow. ## Recommended Acceptance Criteria