xTr1m/RpgRoller
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add refactor blueprint tasks

Browse Source
This commit is contained in:
Frank Tovar
2026-04-04 20:59:20 +02:00
parent 8c5a88afc5
commit 7ec2887df2
1 changed files with 907 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

907
TASKS.md Normal file
View File

@@ -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<UserAccount>`
- `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<RpgRollerDbContext>`
- `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
<AppHeader
User="State.User"
CampaignName="@State.SelectedCampaignName"
LogoutRequested="Session.LogoutAsync" />
```
Another example:
```razor
<CampaignManagementPanel
Campaigns="State.Campaigns"
SelectedCampaignId="State.SelectedCampaignId"
CampaignSelectionChanged="Campaigns.OnCampaignSelectionChangedAsync"
DeleteCampaignRequested="Campaigns.DeleteSelectedCampaignAsync" />
```
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