diff --git a/TASKS.md b/TASKS.md new file mode 100644 index 0000000..6b2a855 --- /dev/null +++ b/TASKS.md @@ -0,0 +1,907 @@ +# Refactor Blueprint: GameService and Workspace + +## Purpose + +This document is the implementation blueprint for splitting two oversized classes without changing product behavior: + +- `RpgRoller/Services/GameService.cs` +- `RpgRoller/Components/Pages/Workspace.razor.cs` + +The goal is to reduce future churn, make responsibilities explicit, improve testability, and create stable seams for future features. + +This is a planning artifact only. It does not authorize behavior changes. During implementation, preserve the existing public API contracts and current user-visible behavior unless a separate task explicitly requests functional changes. + +## Primary Constraints + +- Do not use partial classes as the main decomposition strategy. +- Prefer composition of small sealed classes with clear ownership. +- Keep `IGameService` stable during the refactor unless a separate change requests an API redesign. +- Preserve current endpoint contracts and `WorkspaceQueryService` behavior. +- Preserve current workspace UX and screen flow. +- Do not revert unrelated local changes already present in the repo. +- Keep changes incremental and validation-heavy. + +## Desired End State + +### Backend + +- `GameService` becomes a thin façade/coordinator. +- Stateful concerns are centralized in a shared state owner instead of spread across one giant class. +- Domain workflows are split into small sealed collaborators. +- Pure logic helpers move into small focused files. +- File boundaries reflect change frequency and business ownership. + +### Frontend + +- `Workspace` becomes a thin Blazor component coordinator. +- State, orchestration, admin actions, play/log actions, and live event synchronization live in separate sealed collaborators. +- Razor bindings reference composed state/actions objects instead of one monolithic code-behind surface. +- Existing child component boundaries remain intact unless a very small binding adjustment is needed. + +## Current Problems To Solve + +### GameService + +Observed concerns mixed together today: + +- authentication and session lifecycle +- admin user management +- campaign lifecycle +- character lifecycle and ownership transfer +- skill and skill-group lifecycle +- dice validation and roll execution +- campaign log shaping and roll-detail formatting +- DTO mapping +- in-memory state tracking +- persistence load/save/cloning + +This creates the wrong coupling pattern: + +- changing one workflow risks merge conflicts with unrelated work +- high-level service methods depend on low-level helper implementation details +- lock/state/persistence concerns are interleaved with domain logic +- roll engine logic is too close to CRUD flows +- pure helpers are harder to discover and test in isolation + +### Workspace + +Observed concerns mixed together today: + +- initial bootstrap +- session reload/logout +- screen selection and persistence +- campaign selection and scope refresh +- management actions +- play-screen actions +- campaign log detail caching +- admin actions +- live state event synchronization +- toast/status handling +- UI state storage and computed properties + +This creates the wrong coupling pattern: + +- every change lands in one file +- state mutation rules are difficult to audit +- JS interop ownership is spread across unrelated methods +- event-driven refresh logic is coupled to screen and toast logic +- the Razor file binds against too many members on one type + +## Architecture Direction + +## 1. Backend Composition Strategy + +### 1.1 Keep a thin GameService façade + +`GameService` should stay as the application-facing implementation of `IGameService`, but it should stop owning every workflow directly. + +Target shape: + +- constructor wires shared collaborators +- each public `IGameService` method delegates to one domain-oriented sealed service +- the façade owns no domain-heavy private logic +- only lightweight delegation and composition remain in `GameService` + +This preserves integration points while still removing the monolith. + +### 1.2 Introduce a shared state owner + +Create a single shared state owner to encapsulate the in-memory model, synchronization gate, and persistence coordination used by the composed backend services. + +Candidate responsibility set: + +- lock object +- dictionaries/lists currently stored in `GameService` +- lookup helpers for common state access +- campaign-state tracker storage +- database load/save orchestration hooks + +Recommended candidate names: + +- `GameStateStore` +- `GameRuntimeState` +- `GameStateCoordinator` + +Preferred choice: `GameStateStore` + +Why: + +- "Store" matches the fact that it owns mutable in-memory state +- it avoids implying EF persistence semantics +- it gives the rest of the services a stable dependency name + +This shared state owner must be the only place that exposes the mutable collections and the synchronization gate needed by the domain services. + +### 1.3 Split backend workflows into domain services + +Create sealed services grouped by real business seams, not arbitrary line-count slices. + +Recommended services: + +1. `GameAuthService` +2. `GameUserAdministrationService` +3. `GameCampaignService` +4. `GameCharacterService` +5. `GameSkillService` +6. `GameRollService` +7. `GamePersistenceService` + +Each service should be small enough to own one family of change, but large enough that one feature does not have to hop across many tiny orchestration layers. + +### 1.4 Extract pure helper classes into separate files + +Pure or near-pure helpers should not stay hidden inside the coordinator or domain services. + +Recommended helper extraction: + +- `SkillDefinitionValidator` +- `RollVisibilityParser` +- `CustomRollOptionsResolver` +- `RollBreakdownFormatter` +- `CampaignLogSummaryBuilder` +- `GameDtoMapper` +- `GameStateCloneFactory` +- `RoleSerializer` + +If a helper remains too broad after extraction, split again. Small helper files are desirable here because they are stable, discoverable, and easy to test. + +## 2. Backend Responsibility Map + +### 2.1 GameService + +`GameService` should delegate the following methods: + +- auth/session + - `Register` + - `Login` + - `Logout` + - `GetUserBySession` + - `GetMe` +- campaigns + - `CreateCampaign` + - `GetCampaigns` + - `GetCharacterCampaignOptions` + - `GetCampaign` + - `DeleteCampaign` +- users/admin + - `GetUsernames` + - `GetUsers` + - `UpdateUserRoles` + - `DeleteUser` +- characters + - `CreateCharacter` + - `UpdateCharacter` + - `DeleteCharacter` + - `ActivateCharacter` + - `GetOwnCharacters` +- skills + - `CreateSkillGroup` + - `UpdateSkillGroup` + - `DeleteSkillGroup` + - `CreateSkill` + - `UpdateSkill` + - `DeleteSkill` + - `GetCharacterSheet` +- rolls/log + - `RollSkill` + - `RollCustom` + - `GetCampaignLog` + - `GetCampaignLogPage` + - `GetRollDetail` + - `GetCampaignStateSnapshot` +- rulesets + - `GetRulesets` + +### 2.2 GameAuthService + +Own: + +- register/login/logout/session lookup +- me-response shaping tied to session and active-character resolution +- username normalization usage in auth flows +- session creation + +Dependencies: + +- `GameStateStore` +- `IPasswordHasher` +- `GamePersistenceService` +- `GameDtoMapper` + +### 2.3 GameUserAdministrationService + +Own: + +- `GetUsernames` +- `GetUsers` +- `UpdateUserRoles` +- `DeleteUser` +- role checks used by admin flows + +Dependencies: + +- `GameStateStore` +- `GamePersistenceService` +- `GameDtoMapper` +- `RoleSerializer` +- common authorization helpers + +### 2.4 GameCampaignService + +Own: + +- create/list/get/delete campaign flows +- campaign visibility rules +- campaign context resolution for viewable campaigns +- campaign roster shaping + +Dependencies: + +- `GameStateStore` +- `GamePersistenceService` +- `GameDtoMapper` +- shared authorization/context helpers + +### 2.5 GameCharacterService + +Own: + +- create/update/delete/activate character flows +- owner transfer rules +- active-character consistency rules +- own-character listing +- character-to-campaign state tracker updates + +Dependencies: + +- `GameStateStore` +- `GamePersistenceService` +- `GameDtoMapper` +- shared authorization/context helpers + +### 2.6 GameSkillService + +Own: + +- skill-group CRUD +- skill CRUD +- character sheet read model +- skill-group assignment rules +- skill definition validation orchestration + +Dependencies: + +- `GameStateStore` +- `GamePersistenceService` +- `GameDtoMapper` +- `SkillDefinitionValidator` +- shared authorization/context helpers + +### 2.7 GameRollService + +Own: + +- `RollSkill` +- `RollCustom` +- campaign log reads +- log page reads +- roll detail reads +- campaign state snapshot reads +- roll recording +- log visibility checks +- compact log summary generation + +Dependencies: + +- `GameStateStore` +- `GamePersistenceService` +- `GameDtoMapper` +- `SkillDefinitionValidator` only if needed for custom roll flows +- `RollVisibilityParser` +- `RollEngine` +- `RollBreakdownFormatter` +- `CampaignLogSummaryBuilder` + +### 2.8 GamePersistenceService + +Own: + +- load state from database at startup +- persist current runtime state +- clone/snapshot helpers used for persistence boundaries + +Dependencies: + +- `IDbContextFactory` +- `GameStateStore` +- `GameStateCloneFactory` + +Implementation note: + +- keep EF persistence concerns here +- do not let other domain services talk directly to EF unless there is a deliberate redesign later + +## 3. Backend Cross-Cutting Helpers + +Some logic is shared across multiple domain services but should still remain explicit instead of hidden in the state store. + +Recommended small helper files: + +### Authorization and context + +- `GameAuthorization` + - user role check helpers + - can-view campaign + - can-edit character + - can-view roll + +- `GameContextResolver` + - resolve user from session + - resolve campaign context + - resolve character campaign + - resolve current campaign id + +These can be static helper classes if they remain pure over store inputs, or sealed services if constructor-injected collaborators make the code cleaner. Prefer the simpler shape once implementation starts. + +### Mapping + +- `GameDtoMapper` + - `ToUserSummary` + - `ToAdminUserSummary` + - `ToCampaignOption` + - `ToCampaignSummary` + - `ToCampaignRoster` + - `ToCharacterSheet` + - `ToCharacterSummary` + - `ToCampaignStateSnapshot` + - `ToSkillGroupSummary` + - `ToSkillSummary` + - `ToRollResult` + - `ToLogEntry` + - `ToLogListEntry` + +### Persistence and clone helpers + +- `GameStateCloneFactory` + - clone user/session/campaign/character/skill/skill-group/roll-log entry + +- `RoleSerializer` + - parse/serialize/normalize roles + +### Roll engine helpers + +- `SkillDefinitionValidator` + - expression parse/option validation + +- `RollEngine` + - top-level roll dispatch by ruleset + +- `StandardRollEngine` +- `D6RollEngine` +- `RolemasterRollEngine` + +- `RollBreakdownFormatter` + - shared textual breakdown formatting + +- `CampaignLogSummaryBuilder` + - compact summary + badges + expression extraction helpers + +The roll-engine area is a strong candidate for several very small files because the behavior is algorithmic and changes independently from CRUD flows. + +## 4. Recommended Backend File Layout + +One reasonable target layout: + +```text +RpgRoller/Services/ + GameService.cs + GameStateStore.cs + GamePersistenceService.cs + GameAuthService.cs + GameUserAdministrationService.cs + GameCampaignService.cs + GameCharacterService.cs + GameSkillService.cs + GameRollService.cs + GameAuthorization.cs + GameContextResolver.cs + GameDtoMapper.cs + GameStateCloneFactory.cs + RoleSerializer.cs + SkillDefinitionValidator.cs + RollVisibilityParser.cs + CustomRollOptionsResolver.cs + RollEngine.cs + StandardRollEngine.cs + D6RollEngine.cs + RolemasterRollEngine.cs + RollBreakdownFormatter.cs + CampaignLogSummaryBuilder.cs +``` + +Exact filenames can be adjusted during implementation if a nearby convention in the repo suggests better naming. The important part is the separation of responsibilities, not the precise suffix. + +## 5. Frontend Composition Strategy + +### 5.1 Keep Workspace as the component boundary + +`Workspace` should remain the Blazor component type used by `Workspace.razor`, but it should stop directly owning all workflow code. + +Target shape: + +- `Workspace` keeps injected dependencies and lifecycle entry points +- composed sealed collaborators handle most behavior +- `Workspace` exposes only the state/actions needed by the Razor file + +This avoids breaking the component boundary while still removing the monolith. + +### 5.2 Introduce a dedicated state holder + +Create a single state holder for workspace UI state and view-model data. + +Recommended candidate name: + +- `WorkspaceState` + +Own: + +- authenticated user/session-derived state +- selected campaign and character state +- campaign collections +- rule set collections +- admin user collection +- log detail cache +- toast state +- screen/mobile-panel state +- live connection state +- modal state +- computed helpers that are pure projections over stored state + +This state holder should be easy to inspect and should reduce the risk of hidden cross-method mutations. + +### 5.3 Split workspace orchestration into sealed collaborators + +Recommended services: + +1. `WorkspaceSessionCoordinator` +2. `WorkspaceCampaignCoordinator` +3. `WorkspacePlayCoordinator` +4. `WorkspaceAdminCoordinator` +5. `WorkspaceLiveStateController` +6. `WorkspaceFeedbackService` + +These names can be adjusted, but the responsibilities should stay distinct. + +## 6. Frontend Responsibility Map + +### 6.1 Workspace component + +Own: + +- injected dependencies +- collaborator construction/wiring +- lifecycle delegation +- JS-invokable entry points delegating to collaborators +- final binding surface consumed by Razor + +Keep thin. Do not let it become a second coordinator monolith with only renamed methods. + +### 6.2 WorkspaceState + +Own: + +- scalar UI state +- collections +- selected entities +- modal visibility and form bootstrap models +- log detail cache and loading/error maps +- toast list +- computed view properties + +Recommended computed properties to live here if kept pure: + +- selected campaign name +- selected character +- play-selected campaign +- play-selected character +- play-selected character id +- filtered play skill and skill-group views if still direct projections +- current-user role flags +- campaign delete permission +- screen flags +- connection-state label/css +- app CSS class + +### 6.3 WorkspaceSessionCoordinator + +Own: + +- initial bootstrap sequence +- session reload +- logout flow +- health retry flow +- persisted session-storage reads/writes for screen, panel, campaign, roll visibility +- clearing authenticated state + +Dependencies: + +- `WorkspaceState` +- `WorkspaceQueryService` +- `RpgRollerApiClient` +- `IJSRuntime` +- `WorkspaceLiveStateController` +- `WorkspaceFeedbackService` + +### 6.4 WorkspaceCampaignCoordinator + +Own: + +- campaign reload and selection +- campaign scope refresh +- campaign roster refresh +- character campaign options reload +- management-screen campaign and character mutations +- character modal open/close and bootstrap +- selected-character synchronization + +Dependencies: + +- `WorkspaceState` +- `WorkspaceQueryService` +- `RpgRollerApiClient` +- `IJSRuntime` +- `WorkspaceLiveStateController` +- `WorkspaceFeedbackService` + +### 6.5 WorkspacePlayCoordinator + +Own: + +- selected character activation +- selected character sheet refresh +- skill roll submission +- custom roll handling +- campaign log page refresh +- roll detail expansion/loading/cache trimming +- roll visibility changes +- play-panel errors + +Dependencies: + +- `WorkspaceState` +- `WorkspaceQueryService` +- `RpgRollerApiClient` +- `IJSRuntime` +- `WorkspaceFeedbackService` + +### 6.6 WorkspaceAdminCoordinator + +Own: + +- admin screen access enforcement +- admin user load +- toggle admin role +- delete user + +Dependencies: + +- `WorkspaceState` +- `WorkspaceQueryService` +- `RpgRollerApiClient` +- `IJSRuntime` +- `WorkspaceLiveStateController` +- `WorkspaceFeedbackService` + +### 6.7 WorkspaceLiveStateController + +Own: + +- start/stop SSE state events +- JS invokable state snapshot handling +- JS invokable connection-state updates +- current campaign-state refresh logic +- campaign-state tracking reset logic + +Dependencies: + +- `WorkspaceState` +- `WorkspaceCampaignCoordinator` +- `WorkspacePlayCoordinator` +- `IJSRuntime` + +Critical rule: + +- this class should own the live-update reconciliation logic +- do not let campaign reload logic and live event logic drift into different locations again + +### 6.8 WorkspaceFeedbackService + +Own: + +- status-to-toast behavior +- live announcement updates +- delayed toast dismissal + +Dependencies: + +- `WorkspaceState` +- component refresh callback or dispatcher mechanism + +Implementation note: + +- if callback wiring becomes awkward, this service can remain a helper owned by `Workspace` rather than a DI service +- the important part is isolating feedback behavior from campaign/admin/play workflows + +## 7. Frontend Binding Strategy + +If composition is used, `Workspace.razor` bindings will need to reference state and actions through the new composed surface. + +Example direction: + +- state data from `State` +- management actions from `Campaigns` +- play actions from `Play` +- admin actions from `Admin` +- session/bootstrap actions from `Session` + +Illustrative pattern only: + +```razor + +``` + +Another example: + +```razor + +``` + +Important guardrail: + +- avoid replacing one huge component surface with many deeply chained bindings that are hard to read +- prefer a small number of clearly named composed properties on `Workspace` + +For example: + +- `State` +- `Session` +- `Campaigns` +- `Play` +- `Admin` +- `Live` + +## 8. Recommended Frontend File Layout + +One reasonable target layout: + +```text +RpgRoller/Components/Pages/ + Workspace.razor + Workspace.razor.cs + WorkspaceState.cs + WorkspaceSessionCoordinator.cs + WorkspaceCampaignCoordinator.cs + WorkspacePlayCoordinator.cs + WorkspaceAdminCoordinator.cs + WorkspaceLiveStateController.cs + WorkspaceFeedbackService.cs + WorkspaceToast.cs +``` + +If some files become too small, merge only where the change patterns are clearly the same. Do not merge simply to reduce file count. + +## 9. Implementation Order + +The implementation phase should proceed in small safe steps. + +### Phase 1: Backend enabling extractions + +1. Extract pure helper classes from `GameService` first. +2. Keep `GameService` behavior identical while moving algorithmic helpers out. +3. Add or update tests around the extracted helpers if coverage or confidence drops. + +Why first: + +- lowest integration risk +- creates stable dependencies for later service extraction + +### Phase 2: Introduce GameStateStore and GamePersistenceService + +1. Move runtime collections, gate, and persistence primitives behind the shared state owner. +2. Move database load/save logic into persistence service. +3. Keep `GameService` still acting as the main orchestrator while the shared dependencies settle. + +Why second: + +- makes later service extraction mechanical instead of risky + +### Phase 3: Extract backend domain services + +Suggested order: + +1. auth +2. campaigns +3. characters +4. skills +5. rolls/log +6. admin/users + +This order keeps the highest-risk workflow area, rolls/logs, until the shared helpers and state seams are already stable. + +### Phase 4: Thin GameService façade + +1. Replace remaining logic in `GameService` with delegation only. +2. Confirm constructor wiring stays readable. +3. Re-run tests and local CI. + +### Phase 5: Frontend state extraction + +1. Introduce `WorkspaceState`. +2. Move raw state and pure computed projections first. +3. Keep method behavior temporarily in `Workspace` until state references are stabilized. + +Why first: + +- it shrinks the cognitive load before moving orchestration + +### Phase 6: Frontend coordinator extraction + +Suggested order: + +1. feedback/toasts +2. session/bootstrap +3. campaign management +4. play/log +5. admin +6. live-state controller + +This order reduces the chance that live event logic is extracted before its dependent refresh flows are already stable. + +### Phase 7: Razor binding cleanup + +1. Update bindings to the new composed surface. +2. Keep markup structure stable. +3. Avoid stylistic churn unrelated to composition. + +### Phase 8: Documentation and verification + +1. Update `README.md` code-organization notes if file layout changed materially. +2. Run local CI. +3. Run Playwright smoke flow. +4. Re-check coverage expectations. + +## 10. Testing and Validation Expectations + +During implementation, after each meaningful iteration: + +- run `pwsh ./scripts/ci-local.ps1` +- verify test coverage remains acceptable relative to the repo rules +- add tests where extraction creates untested seams +- after frontend-affecting iterations, run the Playwright smoke flow + +Specific test focus areas: + +### Backend + +- auth/session regression +- campaign visibility rules +- character ownership transfer +- skill-group assignment and validation +- D6 roll behavior +- Rolemaster roll behavior +- custom roll behavior +- log paging and detail visibility +- persistence reload behavior +- admin role mutation and user deletion side effects + +### Frontend + +- workspace bootstrap still restores screen/campaign/panel/visibility preferences +- admin screen still enforces role access +- play screen still refreshes log/roster/sheet correctly +- newly recorded roll still appears and auto-expands as before +- live connection state still updates announcements and fallback state + +## 11. Risks and Guardrails + +### Risk: Hidden behavior drift during backend extraction + +Guardrail: + +- keep `IGameService` contract fixed +- extract pure helpers before orchestration changes +- preserve method-level tests and add new coverage around moved logic + +### Risk: Locking and persistence bugs after splitting GameService + +Guardrail: + +- centralize mutable runtime state in one store +- centralize persistence writes in one service +- do not let each domain service invent its own lock pattern + +### Risk: Workspace composition creates binding sprawl + +Guardrail: + +- expose a small composed surface from `Workspace` +- keep naming direct and role-based +- avoid deep object chains in Razor + +### Risk: Live-state refresh logic breaks after movement + +Guardrail: + +- move refresh logic together with live event handling ownership +- keep campaign roster, selected sheet, and log refresh paths explicit +- verify with Playwright after frontend extraction + +### Risk: Over-fragmentation + +Guardrail: + +- many small helper files are good for pure logic +- domain orchestration services should remain coarse enough to own meaningful workflows +- do not create tiny services that only forward one call with no real ownership + +## 12. Non-Goals + +The implementation phase described by this blueprint should not, by default: + +- redesign API contracts +- redesign the workspace UX +- replace Blazor patterns with a different UI architecture +- change persistence strategy away from the current runtime-state-plus-SQLite model +- introduce unrelated feature work + +## 13. Definition of Done For The Refactor + +The refactor is complete when all of the following are true: + +- `GameService` is a thin coordinator, not a monolith +- backend workflows are owned by composed sealed classes with clear responsibility boundaries +- algorithmic and mapping helpers live in small focused files +- `Workspace` is a thin component coordinator, not a monolithic code-behind +- workspace state and workflow orchestration are separated cleanly +- `Workspace.razor` binds against a readable composed surface +- tests still pass +- local CI still passes +- frontend smoke verification still passes +- documentation matches the resulting structure + +## 14. Implementation Reminder + +During the actual edit phase: + +- keep changes small +- validate often +- do not revert unrelated user changes +- preserve behavior first +- optimize for reducing future churn, not merely reducing line count