RpgRoller/TASKS.md

# Rolemaster skill roll situational modifier modal

This ExecPlan is a living document. The sections `Progress`, `Surprises & Discoveries`, `Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work proceeds.

`PLANS.md` is checked into the repository root. This document must be maintained in accordance with `PLANS.md`.

## Purpose / Big Picture

After this change, clicking the dice button for any Rolemaster skill on the play screen will no longer roll immediately. Instead, a modal dialog will open first and ask for a one-time situational modifier for that upcoming roll. The player can leave it blank for zero, enter a positive number such as `20` for a bonus, or enter a negative number such as `-15` for a penalty. Pressing Enter will confirm the roll, pressing Escape will cancel it, and clicking outside the modal will also cancel it.

The important user-visible rule is that this temporary modifier must be applied everywhere the skill roll logic already uses the skill’s built-in modifier. For a skill stored as `d100!+50`, entering `20` means the first Rolemaster attempt is evaluated as `roll + 50 + 20`, not as a post-processing adjustment. That means an initial result of `8` becomes `8+50+20=78`, which falls into the existing automatic retry band and therefore triggers the retry flow. The retry attempt must also include the same `+20` situational modifier.

## Progress

- [x] (2026-04-14 21:27:50Z) Created the initial ExecPlan in `TASKS.md`, grounded in the current workspace play flow, API contract, and Rolemaster retry implementation.
- [x] (2026-04-14 21:39:42Z) Added transient `SituationalModifier` support to the skill-roll request, API endpoint, service facade, and roll pipeline without adding persistence or schema changes.
- [x] (2026-04-14 21:51:33Z) Added a Rolemaster-only pre-roll modal on the play screen with autofocus, Escape dismissal, Enter submit, outside-click dismissal, and inline validation for signed integer input.
- [x] (2026-04-14 21:39:42Z) Updated Rolemaster roll execution and breakdown formatting so temporary modifiers are shown explicitly and feed retry-band evaluation plus retry attempts.
- [x] (2026-04-14 21:51:33Z) Added service, API, and Playwright coverage for the new behavior, updated `README.md`, and prepared the touched files for cleanup, full CI, and commit.

## Surprises & Discoveries

- Observation: `TASKS.md` was empty before this plan was written, so this ExecPlan now defines the full intended work from scratch.
  Evidence: `Get-Item D:\Code\RpgRoller\TASKS.md | Format-List Length` reported `Length : 0`.

- Observation: the situational modifier fit cleanly as transient request data. No `Skill`, `RollLogEntry`, migration, or EF model change was needed for the first implementation slice.
  Evidence: the change only touched `RollSkillRequest`, the roll endpoint/service path, and Rolemaster roll formatting/execution files.

- Observation: the current Rolemaster retry rule is already based on the fully computed first attempt total, not just the raw die result, which matches the new requirement once the temporary modifier is included in that total.
  Evidence: `RpgRoller/Services/RolemasterRollEngine.cs` resolves retry bands from `firstAttempt.Total`.

- Observation: the repository already uses Blazor modal patterns with overlays and `ElementReference.FocusAsync()` for autofocus, so the new modal can follow an existing local pattern instead of inventing a second approach.
  Evidence: `RpgRoller/Components/Pages/HomeControls/SkillFormModal.razor(.cs)` renders a modal and focuses the name input in `OnAfterRenderAsync`.

- Observation: compact Rolemaster retry summaries still preview the trigger die, not the fully modified first-attempt arithmetic. The authoritative arithmetic belongs in the breakdown string.
  Evidence: with a situational modifier, the new service test now expects `8 | open-ended | retry +5` in the compact summary while the detailed breakdown is `8+50+20=78; retry(+5): 42+50+20=112; final=117`.

- Observation: Razor string parameters in the new modal call site need explicit `@` binding or the UI renders the property name as literal text.
  Evidence: the first Playwright failure snapshot showed the dialog rendering `State.PendingRolemasterSkillRollError` instead of the actual inline validation message until the binding was corrected in `Workspace.razor`.

## Decision Log

- Decision: the situational modifier will be transient request data only and will not be stored on `Skill`, `RollLogEntry`, or in a migration.
  Rationale: the feature is explicitly “once for the upcoming roll.” Persisting it would create stale state, require schema work, and misrepresent the feature.
  Date/Author: 2026-04-14 / Codex

- Decision: `RollSkillRequest` will gain an integer `SituationalModifier` field with a default of `0`, and server-side skill-roll methods will accept the same value.
  Rationale: zero is the normal case, avoids null semantics through the stack, and keeps the request payload simple for both tests and UI code.
  Date/Author: 2026-04-14 / Codex

- Decision: non-zero situational modifiers will be accepted only for Rolemaster skill rolls. Non-Rolemaster skill rolls will continue to execute immediately without showing the modal, and the server will reject any accidental non-zero modifier sent for another ruleset.
  Rationale: the user asked for this behavior only for the Rolemaster system. The server-side guard prevents future UI regressions from silently broadening the feature.
  Date/Author: 2026-04-14 / Codex

- Decision: the modal input will be stored as raw text in UI state and parsed on confirm rather than bound directly to an `int`.
  Rationale: blank must mean zero, signed values must be easy to type, and raw text avoids awkward intermediate states such as a lone `-` while the user is editing.
  Date/Author: 2026-04-14 / Codex

- Decision: Rolemaster breakdown strings will show the base skill modifier and the one-shot situational modifier as separate visible terms instead of folding them into one combined number.
  Rationale: the user needs to audit why a retry happened. `8+50+20=78` is clearer than `8+70=78`, especially when comparing the stored skill expression with the one-time adjustment.
  Date/Author: 2026-04-14 / Codex

- Decision: compact log badges do not need a new “situational modifier” badge.
  Rationale: the result number already changes, the detailed breakdown will show the exact temporary modifier, and adding a new badge for a one-shot value would create clutter with little value.
  Date/Author: 2026-04-14 / Codex

## Outcomes & Retrospective

The feature is now complete end to end. Rolemaster skill rolls no longer execute immediately from the play screen; they first open a modal that accepts an optional one-shot situational modifier, focuses the input automatically, closes on Escape or backdrop click, and validates whole-number input inline. Confirmed rolls send the temporary modifier through the existing skill-roll API, Rolemaster breakdowns show the base and situational modifiers as separate terms, and automatic retry math reuses the same situational modifier on both attempts. Service, API, and Playwright coverage now prove the backend math, the Rolemaster-only guard, and the browser interaction flow.

## Context and Orientation

This repository is an ASP.NET Core and Blazor application rooted at `D:\Code\RpgRoller`. The user-facing play screen is assembled in `RpgRoller/Components/Pages/Workspace.razor`. That page wires `CharacterPanel` to the coordinator class `RpgRoller/Components/Pages/WorkspacePlayCoordinator.cs`, which currently handles skill rolls by calling `Play.RollSkillAsync`.

The skill list and its dice buttons live in `RpgRoller/Components/Pages/HomeControls/SkillGroupBlock.razor`. Each button currently emits only the `Guid` skill identifier. `RpgRoller/Components/Pages/HomeControls/CharacterPanel.razor` and `CharacterPanel.razor.cs` forward that identifier upward through the `RollRequested` callback without opening any pre-roll UI.

The request contract for a skill roll lives in `RpgRoller/Contracts/ApiContracts.cs` as `RollSkillRequest`. The HTTP endpoint is `POST /api/skills/{skillId}/roll` in `RpgRoller/Api/SkillEndpoints.cs`. The service contract is `IGameService.RollSkill(...)` in `RpgRoller/Services/IGameService.cs`, implemented by `RpgRoller/Services/GameService.cs`, and executed by `RpgRoller/Services/GameRollService.cs`.

Rolemaster rolling behavior is implemented in `RpgRoller/Services/RollEngine.cs`, `RolemasterRollEngine.cs`, `RolemasterRetryPolicy.cs`, and `RollBreakdownFormatter.cs`. A “situational modifier” in this plan means a temporary integer that is added to or subtracted from the stored skill expression for one roll only. The stored skill expression is still the canonical thing saved on the skill, such as `d100!+50`. The situational modifier exists only in the request that triggers one roll and in the recorded breakdown text that explains that roll afterward.

UI and regression coverage already exist in the repository areas that matter here. `RpgRoller.Tests/Services/ServiceRolemasterRollTests.cs` covers Rolemaster engine behavior. `RpgRoller.Tests/Api/RolemasterApiTests.cs` covers Rolemaster HTTP behavior. `tests/e2e/smoke.spec.js` covers the browser play flow and already contains Rolemaster smoke tests, including the automatic retry badge path. These are the places to extend rather than creating disconnected new test files.

## Plan of Work

Start by widening the skill-roll request path, but keep the feature transient. In `RpgRoller/Contracts/ApiContracts.cs`, change `RollSkillRequest` so it carries both `Visibility` and `SituationalModifier`, defaulting the latter to `0`. Update `RpgRoller/Contracts/RpgRollerJsonSerializerContext.cs` only if the source generator needs to reflect the changed shape. Then thread the new integer through `RpgRoller/Api/SkillEndpoints.cs`, `RpgRoller/Services/IGameService.cs`, `RpgRoller/Services/GameService.cs`, and `RpgRoller/Services/GameRollService.cs`. In `GameRollService.RollSkill`, resolve the campaign and parsed expression exactly as today, then reject a non-zero situational modifier unless the campaign ruleset is Rolemaster. Reuse the existing authorization and visibility checks unchanged. No database or domain model changes are needed for this part.

After the request pipeline can accept the modifier, update the Rolemaster execution path so the temporary value participates in the actual roll math rather than being bolted on afterward. The cleanest repository-local shape is to extend `RollEngine.Roll(...)` and `RolemasterRollEngine.Roll(...)` with an optional `situationalModifier = 0` argument. Only the Rolemaster branch should consume it. In `RolemasterRollEngine`, add the situational modifier to the expression modifier for both standard Rolemaster rolls and open-ended percentile attempts. The first attempt total that feeds `RolemasterRetryPolicy.ResolveAutoRetryBonus(...)` must already include both the stored skill modifier and the situational modifier. The retry attempt must use the same situational modifier again. Update `RollBreakdownFormatter` so Rolemaster text remains explicit, for example `08+50+20=78` for a normal positive path and `(05) -97 +50 +20 = -22` for a low-end path. The retry breakdown must also preserve this explicit style, for example `08+50+20=78; retry(+5): 42+50+20=112; final=117`.

Then add the pre-roll modal to the workspace play flow. Keep the state and orchestration in the existing play coordinator rather than letting `CharacterPanel` call the API itself. Change the roll callback path from `Guid` to `CharacterSheetSkill` so the coordinator has the skill name and expression immediately. Update `RpgRoller/Components/Pages/HomeControls/SkillGroupBlock.razor(.cs)` and `CharacterPanel.razor(.cs)` to pass the full skill object upward. In `RpgRoller/Components/Pages/WorkspaceState.cs`, add modal state for whether the prompt is open, which skill is pending, the pending raw modifier text, whether the modal is submitting, and the current validation message. In `RpgRoller/Components/Pages/WorkspacePlayCoordinator.cs`, replace the direct-roll entry point with a Rolemaster-aware method that either opens the modal or immediately rolls with modifier `0` for other rulesets. Add companion methods to confirm and cancel the pending Rolemaster roll.

Render the modal near the bottom of `RpgRoller/Components/Pages/Workspace.razor`, alongside the existing character modals, so it shares the same page-level ownership as other overlays. Create a dedicated component pair at `RpgRoller/Components/Pages/HomeControls/RolemasterSkillRollModal.razor` and `RolemasterSkillRollModal.razor.cs` instead of expanding `CharacterPanel` further. The modal should show the skill name, the stored expression, a short help line explaining that blank means zero and negative numbers are allowed, and a single signed-number input. The input should autofocus via `ElementReference.FocusAsync()`. Escape should cancel. Enter should submit through the form. Clicking the overlay outside the dialog card should cancel, while clicking inside the card must not bubble out. Use the existing `.modal-overlay` and `.modal-card` styling patterns first; only add CSS in `RpgRoller/wwwroot/styles.css` if the modal needs a small amount of spacing or width tuning.

Validation belongs in both UI and server code. The modal should trim whitespace and treat an empty field as `0`. If parsing fails, keep the modal open and show an inline error such as “Enter a whole number like 20, -15, or leave blank for 0.” On the server, reject values outside the same Rolemaster modifier limits already enforced by `DiceRules`, namely `-1000` through `1000`. Reuse the existing API error path so invalid requests still surface as user-facing errors without breaking the page.

Finally, update documentation and tests. `README.md` must describe the new Rolemaster roll flow in current-state language, not as a changelog note. Extend service tests to prove that a situational modifier changes Rolemaster totals, triggers retry evaluation from the combined total, and applies again on the retry attempt. Extend API tests to prove the new request payload, server-side Rolemaster-only validation, and breakdown text. Extend Playwright smoke coverage so the browser proves the modal opens only on Rolemaster skill rolls, autofocus works, Enter submits, Escape and outside click dismiss, invalid text stays inline, and a positive situational bonus can be used to cause a retry-enabled result.

## Concrete Steps

All commands below run from `D:\Code\RpgRoller`.

First, implement the request and engine changes, then run targeted tests while the work is still small.

    dotnet test RpgRoller.Tests/RpgRoller.Tests.csproj --filter "FullyQualifiedName~ServiceRolemasterRollTests|FullyQualifiedName~RolemasterApiTests"

After the UI modal is in place, run the browser smoke suite directly.

    pwsh ./scripts/run-playwright.ps1

Before closing the iteration, format every touched file using the repository rule. Replace the placeholder list with the exact touched file paths separated by semicolons.

    jb cleanupcode --build=False RpgRoller/Contracts/ApiContracts.cs;RpgRoller/Api/SkillEndpoints.cs;RpgRoller/Services/IGameService.cs;RpgRoller/Services/GameService.cs;RpgRoller/Services/GameRollService.cs;RpgRoller/Services/RollEngine.cs;RpgRoller/Services/RolemasterRollEngine.cs;RpgRoller/Services/RollBreakdownFormatter.cs;RpgRoller/Components/Pages/Workspace.razor;RpgRoller/Components/Pages/WorkspaceState.cs;RpgRoller/Components/Pages/WorkspacePlayCoordinator.cs;RpgRoller/Components/Pages/HomeControls/CharacterPanel.razor;RpgRoller/Components/Pages/HomeControls/CharacterPanel.razor.cs;RpgRoller/Components/Pages/HomeControls/SkillGroupBlock.razor;RpgRoller/Components/Pages/HomeControls/SkillGroupBlock.razor.cs;RpgRoller/Components/Pages/HomeControls/RolemasterSkillRollModal.razor;RpgRoller/Components/Pages/HomeControls/RolemasterSkillRollModal.razor.cs;RpgRoller/wwwroot/styles.css;RpgRoller.Tests/Services/ServiceRolemasterRollTests.cs;RpgRoller.Tests/Api/RolemasterApiTests.cs;tests/e2e/smoke.spec.js;README.md

Run the repository-wide local CI script as the final proof.

    pwsh ./scripts/ci-local.ps1

Then create one brief commit for the iteration.

    git add TASKS.md README.md RpgRoller/Contracts/ApiContracts.cs RpgRoller/Api/SkillEndpoints.cs RpgRoller/Services/IGameService.cs RpgRoller/Services/GameService.cs RpgRoller/Services/GameRollService.cs RpgRoller/Services/RollEngine.cs RpgRoller/Services/RolemasterRollEngine.cs RpgRoller/Services/RollBreakdownFormatter.cs RpgRoller/Components/Pages/Workspace.razor RpgRoller/Components/Pages/WorkspaceState.cs RpgRoller/Components/Pages/WorkspacePlayCoordinator.cs RpgRoller/Components/Pages/HomeControls/CharacterPanel.razor RpgRoller/Components/Pages/HomeControls/CharacterPanel.razor.cs RpgRoller/Components/Pages/HomeControls/SkillGroupBlock.razor RpgRoller/Components/Pages/HomeControls/SkillGroupBlock.razor.cs RpgRoller/Components/Pages/HomeControls/RolemasterSkillRollModal.razor RpgRoller/Components/Pages/HomeControls/RolemasterSkillRollModal.razor.cs RpgRoller/wwwroot/styles.css RpgRoller.Tests/Services/ServiceRolemasterRollTests.cs RpgRoller.Tests/Api/RolemasterApiTests.cs tests/e2e/smoke.spec.js
    git commit -m "Add Rolemaster situational roll modifier prompt"

Expected proof points during implementation are:

    A Rolemaster skill such as d100!+50 opens the modal instead of rolling immediately.
    Leaving the field blank and pressing Enter records a normal roll.
    Typing 20 and pressing Enter records a breakdown that visibly contains +20.
    If the first attempt becomes 76 through 110 after adding the situational modifier, the existing retry flow still fires.
    A D6 or D&D 5e skill still rolls immediately with no popup.

## Validation and Acceptance

Acceptance is behavioral, not just “the code compiles.”

Start the browser smoke environment with `pwsh ./scripts/run-playwright.ps1` or run the application locally and navigate to the play screen. Create or seed a Rolemaster campaign, a character, and an open-ended skill such as `Observation` with `d100!+50`, `fumbleRange: 5`, and `rolemasterAutoRetry: true`.

On the play screen, clicking `Roll Observation` must open a modal dialog. The modifier input must receive focus immediately. Pressing Escape must close the dialog with no roll recorded. Clicking the backdrop outside the dialog must also close it with no roll recorded. Reopening the dialog and pressing Enter with the field blank must submit a normal zero-modifier roll.

Reopen the dialog and enter `20`. After confirming, the recorded roll detail must show the temporary modifier as a separate term in the breakdown. If the first computed attempt lands in the retry window, the breakdown must show the same `+20` in both attempts and the final total must reflect the retry bonus on top of the retry attempt. The compact log entry should still show the existing retry badge behavior, and expanding the detail should show the existing attempt markers on the dice chips.

Run `dotnet test RpgRoller.Tests/RpgRoller.Tests.csproj` and expect the suite to pass. The new or updated tests should fail before the feature is implemented and pass after it is complete. Run `pwsh ./scripts/ci-local.ps1` and expect the build, tests, coverage gate, and Playwright smoke test to pass end-to-end.

## Idempotence and Recovery

This feature should be implemented additively and is safe to retry. Re-running the same code-edit steps should only replace the intended current-state logic. No migration is expected for this feature because the modifier is transient. If a draft implementation accidentally introduces persistence for the modifier, remove that persistence before considering the work complete.

If the UI modal gets into a bad state during development, the safe recovery path is to clear only the workspace prompt state in `WorkspaceState` and retry the interaction. If Playwright fails because the temporary application process is still running, stop the lingering `dotnet` process once, rerun the script, and confirm the health endpoint responds before rechecking the smoke suite.

## Artifacts and Notes

The most important visible transcript to preserve during implementation is the breakdown text for a retry-causing situational bonus. A representative successful result should look like this shape, with different random numbers allowed:

    08+50+20=78; retry(+5): 42+50+20=112; final=117

The corresponding browser-level proof should include a log row that shows the final result, retains the existing retry badge, and expands into detail whose dice titles still include attempt markers such as:

    Roll 8, step 1, attempt 1, Rolemaster open-ended initial
    Roll 42, step 1, retry attempt 2, Rolemaster open-ended initial

## Interfaces and Dependencies

At the end of the implementation, these repository interfaces should exist in the following shapes.

In `RpgRoller/Contracts/ApiContracts.cs`, define:

    public sealed record RollSkillRequest(string Visibility, int SituationalModifier = 0);

In `RpgRoller/Services/IGameService.cs`, define:

    ServiceResult<RollResult> RollSkill(string sessionToken, Guid skillId, string visibility, int situationalModifier = 0);

In `RpgRoller/Services/GameRollService.cs`, define a matching method:

    public ServiceResult<RollResult> RollSkill(string sessionToken, Guid skillId, string visibility, int situationalModifier)

In `RpgRoller/Services/RollEngine.cs`, extend the Rolemaster path with:

    public (int Total, string Breakdown, IReadOnlyList<RollDieResult> Dice) Roll(
        RulesetKind ruleset,
        DiceExpression expression,
        int wildDice,
        bool allowFumble,
        int? fumbleRange,
        bool rolemasterAutoRetry = false,
        int situationalModifier = 0)

In `RpgRoller/Components/Pages/HomeControls/SkillGroupBlock.razor.cs` and `CharacterPanel.razor.cs`, change the roll callback shape to carry the full skill:

    [Parameter]
    public EventCallback<CharacterSheetSkill> RollSkillRequested { get; set; }

    [Parameter]
    public EventCallback<CharacterSheetSkill> RollRequested { get; set; }

Create a new modal component at `RpgRoller/Components/Pages/HomeControls/RolemasterSkillRollModal.razor` and `.razor.cs` that accepts at least the visibility flag, skill label, expression label, raw modifier text, submit state, and confirm/cancel callbacks. The component should use only existing Blazor and repository dependencies; no third-party modal library is required or desired.

Plan revision note (2026-04-14 / Codex): created the initial ExecPlan for the new Rolemaster one-shot situational modifier modal because `TASKS.md` was empty and the feature needs a self-contained implementation guide before coding begins.
Plan revision note (2026-04-14 / Codex): updated the living plan after the first backend slice landed so progress, discoveries, and retrospective match the new transient request path, explicit breakdown formatting, and added service/API coverage.
Plan revision note (2026-04-14 / Codex): updated the living plan again after the UI slice completed so the document now reflects the shipped modal behavior, the Playwright coverage, and the final end-to-end outcome.