From 0cb41dd0044e81fb99f7ad455b5566d0187b036f Mon Sep 17 00:00:00 2001 From: Frank Tovar Date: Thu, 26 Feb 2026 14:56:56 +0100 Subject: [PATCH] Cleanup campaign management ux --- RpgRoller.Tests/Api/CampaignApiTests.cs | 12 ++--- RpgRoller.Tests/Api/RollVisibilityApiTests.cs | 2 +- RpgRoller.Tests/Api/SystemApiTests.cs | 2 +- .../Services/ServiceCampaignTests.cs | 4 +- .../Services/ServicePersistenceTests.cs | 4 +- RpgRoller/Api/CharacterEndpoints.cs | 4 +- .../CampaignManagementPanel.razor | 16 +------ .../CampaignManagementPanel.razor.cs | 4 +- .../HomeControls/CharacterFormModal.razor.cs | 2 +- .../Pages/HomeControls/CharacterPanel.razor | 8 ++-- .../HomeControls/CharacterPanel.razor.cs | 3 -- RpgRoller/Components/Pages/Workspace.razor | 1 - RpgRoller/Components/Pages/Workspace.razor.cs | 48 ++----------------- RpgRoller/Contracts/ApiContracts.cs | 2 - RpgRoller/Services/GameService.cs | 44 +++++++++-------- RpgRoller/Services/IGameService.cs | 6 +-- openapi/RpgRoller.json | 6 +-- 17 files changed, 55 insertions(+), 113 deletions(-) diff --git a/RpgRoller.Tests/Api/CampaignApiTests.cs b/RpgRoller.Tests/Api/CampaignApiTests.cs index 4b5196a..f870cdc 100644 --- a/RpgRoller.Tests/Api/CampaignApiTests.cs +++ b/RpgRoller.Tests/Api/CampaignApiTests.cs @@ -15,7 +15,7 @@ public sealed class CampaignApiTests : ApiTestBase await RegisterAsync(gmClient, "gm", "Password123", "Game Master"); await LoginAsync(gmClient, "gm", "Password123"); - var campaign = await PostAsync(gmClient, "/api/campaigns", new("Alpha Campaign", "dnd5e")); + var campaign = await PostAsync(gmClient, "/api/campaigns", new("Alpha Campaign", "dnd5e")); var gmCharacter = await PostAsync(gmClient, "/api/characters", new("Arin", campaign.Id)); @@ -38,14 +38,13 @@ public sealed class CampaignApiTests : ApiTestBase var details = await GetAsync(gmClient, $"/api/campaigns/{campaign.Id}"); Assert.Equal(campaign.Id, details.Id); - Assert.Single(details.Characters); - Assert.Single(details.Skills); + Assert.Equal(1, details.Characters); var currentCampaignCharacters = await GetAsync>(gmClient, "/api/characters/current-campaign"); Assert.Single(currentCampaignCharacters); Assert.Equal(gmCharacter.Id, currentCampaignCharacters[0].Id); - var otherCampaign = await PostAsync(gmClient, "/api/campaigns", new("Beta Campaign", "d6")); + var otherCampaign = await PostAsync(gmClient, "/api/campaigns", new("Beta Campaign", "d6")); var updatedCharacter = await PutAsync(gmClient, $"/api/characters/{gmCharacter.Id}", new("Arin Updated", otherCampaign.Id)); @@ -70,7 +69,7 @@ public sealed class CampaignApiTests : ApiTestBase await RegisterAsync(receiverClient, "receiver2", "Password123", "Receiver"); await LoginAsync(receiverClient, "receiver2", "Password123"); - var campaign = await PostAsync(gmClient, "/api/campaigns", new("Grouped Campaign", "d6")); + var campaign = await PostAsync(gmClient, "/api/campaigns", new("Grouped Campaign", "d6")); var character = await PostAsync(ownerClient, "/api/characters", new("Grouped Hero", campaign.Id)); var createdGroup = await PostAsync(ownerClient, $"/api/characters/{character.Id}/skill-groups", new("Combat", "2D+1", 1, true)); @@ -103,8 +102,5 @@ public sealed class CampaignApiTests : ApiTestBase var receiverActivate = await receiverClient.PostAsync($"/api/characters/{character.Id}/activate", null); Assert.Equal(HttpStatusCode.OK, receiverActivate.StatusCode); - - var details = await GetAsync(gmClient, $"/api/campaigns/{campaign.Id}"); - Assert.DoesNotContain(details.SkillGroups, group => group.Id == renamedGroup.Id); } } diff --git a/RpgRoller.Tests/Api/RollVisibilityApiTests.cs b/RpgRoller.Tests/Api/RollVisibilityApiTests.cs index 5e097eb..62a1c87 100644 --- a/RpgRoller.Tests/Api/RollVisibilityApiTests.cs +++ b/RpgRoller.Tests/Api/RollVisibilityApiTests.cs @@ -17,7 +17,7 @@ public sealed class RollVisibilityApiTests : ApiTestBase await RegisterAsync(gmClient, "gm", "Password123", "GM"); await LoginAsync(gmClient, "gm", "Password123"); - var campaign = await PostAsync(gmClient, "/api/campaigns", new("Main", "d6")); + var campaign = await PostAsync(gmClient, "/api/campaigns", new("Main", "d6")); await RegisterAsync(playerClient, "player", "Password123", "Player"); await LoginAsync(playerClient, "player", "Password123"); diff --git a/RpgRoller.Tests/Api/SystemApiTests.cs b/RpgRoller.Tests/Api/SystemApiTests.cs index 4751dc6..5ea25e0 100644 --- a/RpgRoller.Tests/Api/SystemApiTests.cs +++ b/RpgRoller.Tests/Api/SystemApiTests.cs @@ -18,7 +18,7 @@ public sealed class SystemApiTests : ApiTestBase await RegisterAsync(client, "sse", "Password123", "Sse User"); await LoginAsync(client, "sse", "Password123"); - var campaign = await PostAsync(client, "/api/campaigns", new("SSE", "d6")); + var campaign = await PostAsync(client, "/api/campaigns", new("SSE", "d6")); var sseResponse = await client.GetAsync($"/api/events/state?campaignId={campaign.Id}", HttpCompletionOption.ResponseHeadersRead); Assert.Equal(HttpStatusCode.OK, sseResponse.StatusCode); diff --git a/RpgRoller.Tests/Services/ServiceCampaignTests.cs b/RpgRoller.Tests/Services/ServiceCampaignTests.cs index 43dfeed..6955e31 100644 --- a/RpgRoller.Tests/Services/ServiceCampaignTests.cs +++ b/RpgRoller.Tests/Services/ServiceCampaignTests.cs @@ -29,7 +29,7 @@ public sealed class ServiceCampaignTests var activateSuccess = service.ActivateCharacter(gmSession, character.Id); Assert.True(activateSuccess.Succeeded); - var currentCharacters = service.GetCurrentCampaignCharacters(gmSession); + var currentCharacters = service.GetOwnCharacters(gmSession); Assert.True(currentCharacters.Succeeded); Assert.Single(ServiceTestSupport.GetValue(currentCharacters)); @@ -45,7 +45,7 @@ public sealed class ServiceCampaignTests service.Register("user", "Password123", "User"); var sessionToken = ServiceTestSupport.GetValue(service.Login("user", "Password123")).SessionToken; - var result = service.GetCurrentCampaignCharacters(sessionToken); + var result = service.GetOwnCharacters(sessionToken); Assert.False(result.Succeeded); } diff --git a/RpgRoller.Tests/Services/ServicePersistenceTests.cs b/RpgRoller.Tests/Services/ServicePersistenceTests.cs index 39a3815..4ed461d 100644 --- a/RpgRoller.Tests/Services/ServicePersistenceTests.cs +++ b/RpgRoller.Tests/Services/ServicePersistenceTests.cs @@ -33,7 +33,7 @@ public sealed class ServicePersistenceTests Assert.False(service.UpdateCharacter(ownerSession, Guid.NewGuid(), "Renamed", campaign.Id).Succeeded); Assert.False(service.ActivateCharacter(string.Empty, ownerCharacter.Id).Succeeded); Assert.False(service.ActivateCharacter(gmSession, ownerCharacter.Id).Succeeded); - Assert.False(service.GetCurrentCampaignCharacters(string.Empty).Succeeded); + Assert.False(service.GetOwnCharacters(string.Empty).Succeeded); Assert.False(service.CreateSkill(string.Empty, ownerCharacter.Id, "Stealth", "2D+1", 1, true).Succeeded); Assert.False(service.CreateSkill(ownerSession, Guid.NewGuid(), "Stealth", "2D+1", 1, true).Succeeded); Assert.False(service.CreateSkill(otherSession, ownerCharacter.Id, "Stealth", "2D+1", 1, true).Succeeded); @@ -67,7 +67,7 @@ public sealed class ServicePersistenceTests using var staleCurrentHarness = ServiceTestSupport.CreateHarnessFromPath(harness.DbPath, 2, 3, 4); var staleCurrentService = staleCurrentHarness.Service; - var staleCurrentCampaign = staleCurrentService.GetCurrentCampaignCharacters(ownerSession); + var staleCurrentCampaign = staleCurrentService.GetOwnCharacters(ownerSession); Assert.False(staleCurrentCampaign.Succeeded); using (var db = harness.CreateDbContext()) { diff --git a/RpgRoller/Api/CharacterEndpoints.cs b/RpgRoller/Api/CharacterEndpoints.cs index 7680f14..b91f487 100644 --- a/RpgRoller/Api/CharacterEndpoints.cs +++ b/RpgRoller/Api/CharacterEndpoints.cs @@ -31,9 +31,9 @@ internal static class CharacterEndpoints return ApiResultMapper.ToApiResult(result); }); - group.MapGet("/characters/current-campaign", (HttpContext context, IGameService game) => + group.MapGet("/characters", (HttpContext context, IGameService game) => { - var result = game.GetCurrentCampaignCharacters(context.GetRequiredSessionToken()); + var result = game.GetOwnCharacters(context.GetRequiredSessionToken()); return ApiResultMapper.ToApiResult(result); }); diff --git a/RpgRoller/Components/Pages/HomeControls/CampaignManagementPanel.razor b/RpgRoller/Components/Pages/HomeControls/CampaignManagementPanel.razor index 9e4da3d..ef2e043 100644 --- a/RpgRoller/Components/Pages/HomeControls/CampaignManagementPanel.razor +++ b/RpgRoller/Components/Pages/HomeControls/CampaignManagementPanel.razor @@ -9,27 +9,15 @@ } else { - + } - @if (SelectedCampaign is null) - { -

No campaign selected.

- } - else - { -

Name: @SelectedCampaign.Name

-

Ruleset: @SelectedCampaign.RulesetId

-

GM: @SelectedCampaign.Gm.DisplayName (@SelectedCampaign.Gm.Username)

-

Characters visible: @SelectedCampaign.Characters.Count

- } - } diff --git a/RpgRoller/Components/Pages/HomeControls/CharacterPanel.razor.cs b/RpgRoller/Components/Pages/HomeControls/CharacterPanel.razor.cs index 0907263..05e62b7 100644 --- a/RpgRoller/Components/Pages/HomeControls/CharacterPanel.razor.cs +++ b/RpgRoller/Components/Pages/HomeControls/CharacterPanel.razor.cs @@ -291,9 +291,6 @@ public partial class CharacterPanel [Parameter] public Func CanEditSkill { get; set; } = _ => false; - [Parameter] - public Func CanRollSkill { get; set; } = _ => false; - [Parameter] public EventCallback CharacterSelected { get; set; } diff --git a/RpgRoller/Components/Pages/Workspace.razor b/RpgRoller/Components/Pages/Workspace.razor index 7308ded..b74a853 100644 --- a/RpgRoller/Components/Pages/Workspace.razor +++ b/RpgRoller/Components/Pages/Workspace.razor @@ -78,7 +78,6 @@ SkillDefinitionLabel="SkillDefinitionLabel" CanEditCharacter="CanEditCharacter" CanEditSkill="CanEditSkill" - CanRollSkill="CanRollSkill" CharacterSelected="SelectCharacterAsync" EditCharacterRequested="OpenEditCharacterModal" SkillCreated="OnSkillCreatedAsync" diff --git a/RpgRoller/Components/Pages/Workspace.razor.cs b/RpgRoller/Components/Pages/Workspace.razor.cs index 0ff667c..2548585 100644 --- a/RpgRoller/Components/Pages/Workspace.razor.cs +++ b/RpgRoller/Components/Pages/Workspace.razor.cs @@ -136,7 +136,7 @@ public partial class Workspace : IAsyncDisposable private async Task ReloadCampaignsAsync(Guid? preferredCampaignId) { - var campaigns = await ApiClient.RequestAsync>("GET", "/api/campaigns"); + var campaigns = await ApiClient.RequestAsync>("GET", "/api/campaigns"); Campaigns = campaigns.OrderBy(c => c.Name, StringComparer.OrdinalIgnoreCase).ToList(); if (Campaigns.Count == 0) @@ -191,30 +191,6 @@ public partial class Workspace : IAsyncDisposable } } - private async Task ManualRefreshAsync() - { - if (IsMutating) - return; - - IsMutating = true; - try - { - await CheckHealthAsync(); - var reloaded = await ReloadAuthenticatedSessionAsync(SelectedCampaignId); - if (!reloaded) - { - await LoggedOut.InvokeAsync("Session expired. Please log in again."); - return; - } - - SetStatus("Campaign data refreshed.", false); - } - finally - { - IsMutating = false; - } - } - private async Task LogoutAsync() { if (IsMutating) @@ -435,23 +411,10 @@ public partial class Workspace : IAsyncDisposable return; } - var selectedSkill = SelectedCampaign.Skills.FirstOrDefault(skill => skill.Id == skillId); - if (selectedSkill is null) - { - SetStatus("Skill is no longer available. Refresh campaign data.", true); - return; - } - - if (!CanRollSkill(selectedSkill)) - { - SetStatus("You are not allowed to roll this skill.", true); - return; - } - IsMutating = true; try { - LastRoll = await ApiClient.RequestAsync("POST", $"/api/skills/{selectedSkill.Id}/roll", new RollSkillRequest(RollVisibility)); + LastRoll = await ApiClient.RequestAsync("POST", $"/api/skills/{skillId}/roll", new RollSkillRequest(RollVisibility)); await RefreshCampaignScopeAsync(); SetStatus("Roll recorded.", false); @@ -487,11 +450,6 @@ public partial class Workspace : IAsyncDisposable return character is not null && CanEditCharacter(character); } - private bool CanRollSkill(SkillSummary skill) - { - return CanEditSkill(skill); - } - [JSInvokable] public async Task OnStateEventReceived(long _) { @@ -736,7 +694,7 @@ public partial class Workspace : IAsyncDisposable private Guid? ActiveCharacterId { get; set; } private Guid? SelectedCampaignId { get; set; } private CampaignDetails? SelectedCampaign { get; set; } - private List Campaigns { get; set; } = []; + private List Campaigns { get; set; } = []; private List CampaignLog { get; set; } = []; private List Rulesets { get; set; } = []; private Guid? SelectedCharacterId { get; set; } diff --git a/RpgRoller/Contracts/ApiContracts.cs b/RpgRoller/Contracts/ApiContracts.cs index 28d59c1..873b5e2 100644 --- a/RpgRoller/Contracts/ApiContracts.cs +++ b/RpgRoller/Contracts/ApiContracts.cs @@ -16,8 +16,6 @@ public sealed record RulesetDefinition(string Id, string Name, string DiceSyntax public sealed record CreateCampaignRequest(string Name, string RulesetId); -public sealed record CampaignSummary(Guid Id, string Name, string RulesetId, Guid GmUserId); - public sealed record CampaignDetails(Guid Id, string Name, string RulesetId, UserSummary Gm, IReadOnlyList Characters, IReadOnlyList SkillGroups, IReadOnlyList Skills); public sealed record CreateCharacterRequest(string Name, Guid CampaignId); diff --git a/RpgRoller/Services/GameService.cs b/RpgRoller/Services/GameService.cs index ee52017..7079338 100644 --- a/RpgRoller/Services/GameService.cs +++ b/RpgRoller/Services/GameService.cs @@ -127,20 +127,20 @@ public sealed class GameService : IGameService } } - public ServiceResult CreateCampaign(string sessionToken, string name, string rulesetId) + public ServiceResult CreateCampaign(string sessionToken, string name, string rulesetId) { if (string.IsNullOrWhiteSpace(name)) - return ServiceResult.Failure("invalid_campaign_name", "Campaign name is required."); + return ServiceResult.Failure("invalid_campaign_name", "Campaign name is required."); var ruleset = DiceRules.TryParseRulesetId(rulesetId); if (ruleset is null) - return ServiceResult.Failure("invalid_ruleset", "Unknown ruleset."); + return ServiceResult.Failure("invalid_ruleset", "Unknown ruleset."); lock (m_Gate) { var user = ResolveUserLocked(sessionToken); if (user is null) - return ServiceResult.Failure("unauthorized", "You must be logged in."); + return ServiceResult.Failure("unauthorized", "You must be logged in."); var campaign = new Campaign { @@ -153,25 +153,25 @@ public sealed class GameService : IGameService m_CampaignsById[campaign.Id] = campaign; PersistStateLocked(); - return ServiceResult.Success(ToCampaignSummary(campaign)); + return ServiceResult.Success(ToCampaignDetails(campaign)); } } - public ServiceResult> GetCampaigns(string sessionToken) + public ServiceResult> GetCampaigns(string sessionToken) { lock (m_Gate) { var user = ResolveUserLocked(sessionToken); if (user is null) - return ServiceResult>.Failure("unauthorized", "You must be logged in."); + return ServiceResult>.Failure("unauthorized", "You must be logged in."); var campaignIds = new HashSet(m_CampaignsById.Values.Where(c => c.GmUserId == user.Id).Select(c => c.Id)); foreach (var character in m_CharactersById.Values.Where(c => c.OwnerUserId == user.Id)) campaignIds.Add(character.CampaignId); - var results = campaignIds.Select(id => m_CampaignsById[id]).OrderBy(c => c.Name, StringComparer.OrdinalIgnoreCase).Select(ToCampaignSummary).ToArray(); + var results = campaignIds.Select(id => m_CampaignsById[id]).OrderBy(c => c.Name, StringComparer.OrdinalIgnoreCase).Select(ToCampaignDetails).ToArray(); - return ServiceResult>.Success(results); + return ServiceResult>.Success(results); } } @@ -183,11 +183,9 @@ public sealed class GameService : IGameService if (!context.Succeeded) return ServiceResult.Failure(context.Error!.Code, context.Error.Message); - var (user, campaign) = context.Value!; + var (_, campaign) = context.Value; var gm = m_UsersById[campaign.GmUserId]; - var isGm = campaign.GmUserId == user.Id; - - var characters = m_CharactersById.Values.Where(c => c.CampaignId == campaign.Id).Where(c => isGm || c.OwnerUserId == user.Id).OrderBy(c => c.Name, StringComparer.OrdinalIgnoreCase).Select(ToCharacterSummary).ToArray(); + var characters = m_CharactersById.Values.Where(c => c.CampaignId == campaign.Id).Select(ToCharacterSummary).ToList(); var visibleCharacterIds = characters.Select(c => c.Id).ToHashSet(); @@ -318,7 +316,7 @@ public sealed class GameService : IGameService } } - public ServiceResult> GetCurrentCampaignCharacters(string sessionToken) + public ServiceResult> GetOwnCharacters(string sessionToken) { lock (m_Gate) { @@ -326,10 +324,7 @@ public sealed class GameService : IGameService if (user is null) return ServiceResult>.Failure("unauthorized", "You must be logged in."); - if (!TryGetCurrentCampaignIdLocked(user, out var campaignId)) - return ServiceResult>.Failure("no_active_character", "No active character is selected."); - - var characters = m_CharactersById.Values.Where(c => c.CampaignId == campaignId && c.OwnerUserId == user.Id).OrderBy(c => c.Name, StringComparer.OrdinalIgnoreCase).Select(ToCharacterSummary).ToArray(); + var characters = m_CharactersById.Values.Where(c => c.OwnerUserId == user.Id).OrderBy(c => c.Name, StringComparer.OrdinalIgnoreCase).Select(ToCharacterSummary).ToArray(); return ServiceResult>.Success(characters); } @@ -803,9 +798,18 @@ public sealed class GameService : IGameService return new(user.Id, user.Username, user.DisplayName); } - private static CampaignSummary ToCampaignSummary(Campaign campaign) + private CampaignDetails ToCampaignDetails(Campaign campaign) { - return new(campaign.Id, campaign.Name, DiceRules.ToRulesetId(campaign.Ruleset), campaign.GmUserId); + lock (m_Gate) + { + var gm = m_UsersById[campaign.GmUserId]; + var characters = m_CharactersById.Values.Where(c => c.CampaignId == campaign.Id).Select(ToCharacterSummary).ToList(); + var visibleCharacterIds = characters.Select(c => c.Id).ToHashSet(); + var skillGroups = m_SkillGroupsById.Values.Where(g => visibleCharacterIds.Contains(g.CharacterId)).OrderBy(g => g.Name, StringComparer.OrdinalIgnoreCase).Select(ToSkillGroupSummary).ToArray(); + var skills = m_SkillsById.Values.Where(s => visibleCharacterIds.Contains(s.CharacterId)).OrderBy(s => s.Name, StringComparer.OrdinalIgnoreCase).Select(ToSkillSummary).ToArray(); + + return new(campaign.Id, campaign.Name, DiceRules.ToRulesetId(campaign.Ruleset), ToUserSummary(gm), characters, skillGroups, skills); + } } private static CharacterSummary ToCharacterSummary(Character character) diff --git a/RpgRoller/Services/IGameService.cs b/RpgRoller/Services/IGameService.cs index 1629500..9674b53 100644 --- a/RpgRoller/Services/IGameService.cs +++ b/RpgRoller/Services/IGameService.cs @@ -12,15 +12,15 @@ public interface IGameService UserSummary? GetUserBySession(string sessionToken); ServiceResult GetMe(string sessionToken); - ServiceResult CreateCampaign(string sessionToken, string name, string rulesetId); - ServiceResult> GetCampaigns(string sessionToken); + ServiceResult CreateCampaign(string sessionToken, string name, string rulesetId); + ServiceResult> GetCampaigns(string sessionToken); ServiceResult GetCampaign(string sessionToken, Guid campaignId); ServiceResult> GetUsernames(string sessionToken); ServiceResult CreateCharacter(string sessionToken, string name, Guid campaignId); ServiceResult UpdateCharacter(string sessionToken, Guid characterId, string name, Guid campaignId, string? ownerUsername = null); ServiceResult ActivateCharacter(string sessionToken, Guid characterId); - ServiceResult> GetCurrentCampaignCharacters(string sessionToken); + ServiceResult> GetOwnCharacters(string sessionToken); ServiceResult CreateSkillGroup(string sessionToken, Guid characterId, string name, string diceRollDefinition, int wildDice, bool allowFumble); ServiceResult UpdateSkillGroup(string sessionToken, Guid skillGroupId, string name, string diceRollDefinition, int wildDice, bool allowFumble); diff --git a/openapi/RpgRoller.json b/openapi/RpgRoller.json index 7f576c4..fd35ed1 100644 --- a/openapi/RpgRoller.json +++ b/openapi/RpgRoller.json @@ -444,12 +444,12 @@ } } }, - "/api/characters/current-campaign": { + "/api/characters": { "get": { - "operationId": "getCurrentCampaignCharacters", + "operationId": "getOwnCharacters", "responses": { "200": { - "description": "Current campaign characters owned by the user.", + "description": "Characters owned by the user.", "content": { "application/json": { "schema": {