From 9a58a6954b73c452c12c3601324fbd3d270c207d Mon Sep 17 00:00:00 2001 From: karashiiro <49822414+karashiiro@users.noreply.github.com> Date: Mon, 23 Jun 2025 19:33:22 -0700 Subject: [PATCH] Fix FT roles not being assigned sometimes In DefaultLogParsingRulesSelector, we go through the fight names to see which content we need to check prog for. We do this by comparing fight names to role names, since the role names all have the fight name in them. However, fight names can sometimes be empty, in which case they'll always match any string. This had the unfortunate effect of always marking affected logs as DRS fights, since that gets checked before FT. This commit fixes the issue by skipping fights with empty/whitespace names. --- .../FFXIV/DelubrumReginae/DRRunCommands.cs | 38 ++++++++++--------- src/Prima.Tests/LogParserServiceTests.cs | 37 ++++++++++++++++++ .../Game/FFXIV/FFLogs/LogParserService.cs | 24 ++++++++++-- .../Rules/DefaultLogParsingRulesSelector.cs | 5 +++ 4 files changed, 83 insertions(+), 21 deletions(-) diff --git a/src/Prima.Application/Commands/FFXIV/DelubrumReginae/DRRunCommands.cs b/src/Prima.Application/Commands/FFXIV/DelubrumReginae/DRRunCommands.cs index 3ffcf5b..3a6b88c 100644 --- a/src/Prima.Application/Commands/FFXIV/DelubrumReginae/DRRunCommands.cs +++ b/src/Prima.Application/Commands/FFXIV/DelubrumReginae/DRRunCommands.cs @@ -1,6 +1,7 @@ using Discord; using Discord.Commands; using Discord.Net; +using Microsoft.Extensions.Logging; using Prima.DiscordNet; using Prima.DiscordNet.Attributes; using Prima.Game.FFXIV.FFLogs; @@ -8,7 +9,6 @@ using Prima.Models.FFLogs; using Prima.Resources; using Prima.Services; -using Serilog; namespace Prima.Application.Commands.FFXIV.DelubrumReginae; @@ -17,11 +17,13 @@ public class DRRunCommands : ModuleBase { private readonly IDbService _db; private readonly ILogParserService _logParser; + private readonly ILogger _logger; - public DRRunCommands(IDbService db, ILogParserService logParser) + public DRRunCommands(IDbService db, ILogParserService logParser, ILogger logger) { _db = db; _logParser = logParser; + _logger = logger; } [Command("setroler", RunMode = RunMode.Async)] @@ -55,7 +57,7 @@ await member.SendMessageAsync( } catch (HttpException e) when (e.DiscordCode == DiscordErrorCode.CannotSendMessageToUser) { - Log.Warning("Can't send direct message to user {DiscordName}", member.ToString()); + _logger.LogWarning("Can't send direct message to user {DiscordName}", member.ToString()); } } @@ -85,14 +87,14 @@ public async Task RPrgDrs() [Command("addprogrole", RunMode = RunMode.Async)] [Alias("addprogroles")] [Description( - "Adds progression roles to server members from a log. Rolers can also manually add roles using this command.")] + "Adds progression roles to server members from a _logger. Rolers can also manually add roles using this command.")] [RestrictToGuilds(SpecialGuilds.CrystalExploratoryMissions)] public async Task AddDelubrumProgRoleAsync([Remainder] string args) { using var typing = Context.Channel.EnterTypingState(); var isFFLogs = FFLogsUtils.IsLogLink(args); - Log.Information("FFLogs link provided: {IsFFLogsLinkProvided}", isFFLogs); + _logger.LogInformation("FFLogs link provided: {IsFFLogsLinkProvided}", isFFLogs); if (isFFLogs) { @@ -105,7 +107,7 @@ public async Task AddDelubrumProgRoleAsync([Remainder] string args) && !executor.HasRole(579916868035411968, Context) // or Mentor && !executor.GuildPermissions.KickMembers) // or can kick users { - Log.Information("User does not have roler role"); + _logger.LogInformation("User does not have roler role"); var res = await ReplyAsync($"{Context.User.Mention}, you don't have the roler role!"); await Task.Delay(5000); await res.DeleteAsync(); @@ -131,7 +133,7 @@ public async Task AddDelubrumProgRoleAsync([Remainder] string args) StringComparison.InvariantCultureIgnoreCase)); if (role == null) { - Log.Information("Role name {RoleName} is invalid", roleName); + _logger.LogInformation("Role name {RoleName} is invalid", roleName); var res = await ReplyAsync( $"{Context.User.Mention}, no role by that name exists! Make sure you spelled it correctly."); await Task.Delay(5000); @@ -141,7 +143,7 @@ public async Task AddDelubrumProgRoleAsync([Remainder] string args) if (!DelubrumProgressionRoles.Roles.ContainsKey(role.Id)) { - Log.Information("Role key {RoleKey} is invalid", role.Id); + _logger.LogInformation("Role key {RoleKey} is invalid", role.Id); return; } @@ -158,7 +160,7 @@ await Task.WhenAll(members } catch (Exception e) { - Log.Error(e, "Failed to add roles to user {DiscordName}", m.ToString()); + _logger.LogError(e, "Failed to add roles to user {DiscordName}", m.ToString()); return Task.CompletedTask; } })); @@ -209,7 +211,7 @@ await Task.WhenAll(members } catch (Exception e) { - Log.Error(e, "Failed to remove role from user {DiscordName}", m.ToString()); + _logger.LogError(e, "Failed to remove role from user {DiscordName}", m.ToString()); return Task.CompletedTask; } })); @@ -228,7 +230,7 @@ public async Task ReadLog(string logLink) switch (logParsingResult) { case LogParsingResult.Failure failure: - Log.Error("Failed to read log: {ErrorMessage}", failure.ErrorMessage); + _logger.LogError("Failed to read log: {ErrorMessage}", failure.ErrorMessage); await ReplyAsync(failure.ErrorMessage); return; case LogParsingResult.Success success: @@ -279,20 +281,20 @@ await ReplyAsync( } } - private static async Task AssignProgressionRole(IRole role, IGuildUser user, ILogParsingRules rules) + private async Task AssignProgressionRole(IRole role, IGuildUser user, ILogParsingRules rules) { if (user.HasRole(rules.FinalClearRoleId)) { - Log.Information("User {DiscordName} already has clear role", user.ToString()); + _logger.LogInformation("User {DiscordName} already has clear role", user.ToString()); return false; } - Log.Information("Checking role {RoleName} on user {DiscordName}", role.Name, + _logger.LogInformation("Checking role {RoleName} on user {DiscordName}", role.Name, user.ToString()); if (!user.HasRole(role)) { await user.AddRoleAsync(role); - Log.Information("Added role {RoleName} to user {DiscordName}", role.Name, + _logger.LogInformation("Added role {RoleName} to user {DiscordName}", role.Name, user.ToString()); return true; } @@ -300,14 +302,14 @@ private static async Task AssignProgressionRole(IRole role, IGuildUser use return false; } - private static async Task RemoveProgressionRole(IRole role, IGuildUser user) + private async Task RemoveProgressionRole(IRole role, IGuildUser user) { - Log.Information("Checking role {RoleName} on user {DiscordName}", role.Name, + _logger.LogInformation("Checking role {RoleName} on user {DiscordName}", role.Name, user.ToString()); if (user.HasRole(role)) { await user.RemoveRoleAsync(role); - Log.Information("Removed role {RoleName} from user {DiscordName}", role.Name, + _logger.LogInformation("Removed role {RoleName} from user {DiscordName}", role.Name, user.ToString()); } } diff --git a/src/Prima.Tests/LogParserServiceTests.cs b/src/Prima.Tests/LogParserServiceTests.cs index 0d79f5f..5285eec 100644 --- a/src/Prima.Tests/LogParserServiceTests.cs +++ b/src/Prima.Tests/LogParserServiceTests.cs @@ -212,6 +212,43 @@ public async Task ReadLog_MissedUsers_ReportsInResult() Assert.That(success.MissedUsers[0].Name, Is.EqualTo("TestPlayer")); } + [Test] + public async Task ReadLog_ForkedTower_DeadStars_AssignsCorrectRoles() + { + var testLog = CreateTestLogWithEncounter("Dead Stars", true, 100); + _mockFFLogsClient.SetupLog(testLog); + _mockActorMapper.SetupUsers(new Dictionary + { + { 1, new DiscordXIVUser { Name = "TestPlayer", World = "TestWorld", DiscordId = 12345 } }, + }); + + var result = await _service.ReadLog("https://www.fflogs.com/reports/abcde12345", _mockActorMapper); + + Assert.That(result, Is.InstanceOf()); + var success = (LogParsingResult.Success)result; + Assert.That(success.RoleAssignments.Count, Is.EqualTo(1)); + Assert.That(success.Rules, Is.InstanceOf()); + + var assignment = success.RoleAssignments[0]; + var roleActions = assignment.RoleActions; + Assert.That(roleActions.Count, Is.EqualTo(3)); + + // Should add Demon Tablet Progression role + Assert.That(roleActions.Any(ra => + ra.ActionType == LogParsingResult.RoleActionType.Add && + ra.RoleId == ForkedTowerRules.DemonTabletProgression)); + + // Should add Dead Stars Progression role + Assert.That(roleActions.Any(ra => + ra.ActionType == LogParsingResult.RoleActionType.Add && + ra.RoleId == ForkedTowerRules.DeadStarsProgression)); + + // Should add Marble Dragon Progression role (kill role) + Assert.That(roleActions.Any(ra => + ra.ActionType == LogParsingResult.RoleActionType.Add && + ra.RoleId == ForkedTowerRules.MarbleDragonProgression)); + } + [Test] public async Task ReadLog_ForkedTower_MarbleDragon_AssignsCorrectRoles() { diff --git a/src/Prima/Game/FFXIV/FFLogs/LogParserService.cs b/src/Prima/Game/FFXIV/FFLogs/LogParserService.cs index d3e42ea..8ef5c53 100644 --- a/src/Prima/Game/FFXIV/FFLogs/LogParserService.cs +++ b/src/Prima/Game/FFXIV/FFLogs/LogParserService.cs @@ -52,6 +52,8 @@ public async Task ReadLog(string logLink, IBatchActorMapper ac public async Task ReadLog(string logLink, IBatchActorMapper actorMapper, ILogParsingRules rules) { + _logger.LogInformation("Parsing log with ruleset {RulesetName}", rules.GetType().Name); + // Log validation var validationResult = await ValidateLog(logLink); if (validationResult is LogValidationResult.Failure validationFailure) @@ -77,7 +79,7 @@ public async Task ReadLog(string logLink, IBatchActorMapper ac { if (!rules.ShouldProcessEncounter(encounter.Difficulty)) { - Log.Warning("Encounter {Encounter} should not be processed based on difficulty {Difficulty}", + _logger.LogWarning("Encounter {Encounter} should not be processed based on difficulty {Difficulty}", encounter.Name, encounter.Difficulty); continue; } @@ -85,14 +87,16 @@ public async Task ReadLog(string logLink, IBatchActorMapper ac var progressionRoleName = rules.GetProgressionRoleName(encounter.Name); if (progressionRoleName == null) { - Log.Warning("No progression role mapping found for encounter: {EncounterName}", encounter.Name); + _logger.LogWarning("No progression role mapping found for encounter: {EncounterName}", + encounter.Name); continue; } var killRoleId = rules.GetKillRoleId(progressionRoleName); if (killRoleId == 0) { - Log.Warning("No kill role ID found for progression role: {ProgressionRole}", progressionRoleName); + _logger.LogWarning("No kill role ID found for progression role: {ProgressionRole}", + progressionRoleName); continue; } @@ -107,6 +111,8 @@ public async Task ReadLog(string logLink, IBatchActorMapper ac var actor = actors.Find(a => a.Id == id); if (actor != null) { + _logger.LogWarning("Can't find Discord user for actor {ActorId}: ({World}) {CharacterName}", + actor.Id, actor.Server, actor.Name); missedUsers.Add(actor); } @@ -115,6 +121,7 @@ public async Task ReadLog(string logLink, IBatchActorMapper ac if (discordUser == null) { + _logger.LogWarning("Can't find Discord user {DiscordId}", id); continue; } @@ -124,6 +131,9 @@ public async Task ReadLog(string logLink, IBatchActorMapper ac if (killRoleId == rules.FinalClearRoleId && encounter.Kill == true) { + _logger.LogInformation("Adding clear role for {EncounterName} to Discord user {DiscordName}", + encounter.Name, discordUser.ToString()); + // Remove all contingent roles roleActions.AddRange(contingentRoleIds .Select(progRoleId => new LogParsingResult.RoleAction @@ -141,6 +151,9 @@ public async Task ReadLog(string logLink, IBatchActorMapper ac } else { + _logger.LogInformation("Adding prog roles for {EncounterName} to Discord user {DiscordName}", + encounter.Name, discordUser.ToString()); + // Give all contingent roles as well as the clear role for the fight if cleared roleActions.AddRange(contingentRoleIds .Select(progRoleId => new LogParsingResult.RoleAction @@ -151,6 +164,9 @@ public async Task ReadLog(string logLink, IBatchActorMapper ac if (encounter.Kill == true) { + _logger.LogInformation("Adding kill role for {EncounterName} to Discord user {DiscordName}", + encounter.Name, discordUser.ToString()); + roleActions.Add(new LogParsingResult.RoleAction { ActionType = LogParsingResult.RoleActionType.Add, @@ -169,6 +185,8 @@ public async Task ReadLog(string logLink, IBatchActorMapper ac RoleActions = kvp.Value, }) .ToList(); + + _logger.LogInformation("Successfully parsed log with ruleset {RulesetName}", rules.GetType().Name); return new LogParsingResult.Success { RoleAssignments = roleAssignments, diff --git a/src/Prima/Game/FFXIV/FFLogs/Rules/DefaultLogParsingRulesSelector.cs b/src/Prima/Game/FFXIV/FFLogs/Rules/DefaultLogParsingRulesSelector.cs index f817b3b..975c864 100644 --- a/src/Prima/Game/FFXIV/FFLogs/Rules/DefaultLogParsingRulesSelector.cs +++ b/src/Prima/Game/FFXIV/FFLogs/Rules/DefaultLogParsingRulesSelector.cs @@ -11,6 +11,11 @@ public ILogParsingRules GetParsingRules(LogInfo.ReportDataWrapper.ReportData.Rep { foreach (var encounter in report.Fights) { + if (string.IsNullOrWhiteSpace(encounter.Name)) + { + continue; + } + if (DelubrumProgressionRoles.Roles.Values.Any(roleName => roleName.Contains(encounter.Name))) { return new DelubrumReginaeSavageRules();