From 23ed6c2bf3837d9a35f3d00454cee8f88b75a485 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=82ngelo=20Tadeucci?= Date: Mon, 9 Feb 2026 23:11:57 -0300 Subject: [PATCH 1/3] Improve player save with concurrency handling Replace Console.WriteLine with structured logging and add robust concurrency handling to SavePlayer. Introduces GetLastModifiedTimestamps to fetch last-modified timestamps, a retry loop (max 5 attempts) for DbUpdateConcurrencyException, per-entity diff logging, normalization of CreationTime, and suppression of noisy JSON property diffs via IsJsonStructurallyEqual. Adds helper methods (FormatValue, JsonNoiseProperties) and necessary using directives for EF Core change tracking and System.Text.Json. These changes improve diagnostics and reliability when saving player/account/unlock entities. --- .../Storage/Game/GameStorage.User.cs | 147 +++++++++++++++++- 1 file changed, 143 insertions(+), 4 deletions(-) diff --git a/Maple2.Database/Storage/Game/GameStorage.User.cs b/Maple2.Database/Storage/Game/GameStorage.User.cs index 800d694a..dd4124f2 100644 --- a/Maple2.Database/Storage/Game/GameStorage.User.cs +++ b/Maple2.Database/Storage/Game/GameStorage.User.cs @@ -1,10 +1,13 @@ -using Maple2.Database.Extensions; +using System.Text.Json; +using Maple2.Database.Extensions; using Maple2.Database.Model; using Maple2.Model.Enum; using Maple2.Model.Game; using Maple2.Model.Metadata; using Maple2.Server.Game.Manager.Config; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.ChangeTracking; +using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.Extensions.Logging; using Account = Maple2.Model.Game.Account; using Character = Maple2.Model.Game.Character; @@ -207,6 +210,25 @@ from outdoor in plot.DefaultIfEmpty() return home; } + public (DateTime CharacterLastModified, DateTime AccountLastModified, DateTime UnlockLastModified)? GetLastModifiedTimestamps(long characterId) { + var result = Context.Character.Where(character => character.Id == characterId) + .Join(Context.Account, character => character.AccountId, account => account.Id, (character, account) => new { + character, + account, + }) + .Join(Context.CharacterUnlock, @t => @t.character.Id, unlock => unlock.CharacterId, (@t, unlock) => new { + CharacterLastModified = @t.character.LastModified, + AccountLastModified = @t.account.LastModified, + UnlockLastModified = unlock.LastModified, + }) + .AsNoTracking() + .FirstOrDefault(); + if (result == null) { + return null; + } + return (result.CharacterLastModified, result.AccountLastModified, result.UnlockLastModified); + } + // We pass in objectId only for Player initialization. public Player? LoadPlayer(long accountId, long characterId, int objectId, short channel) { Context.ChangeTracker.QueryTrackingBehavior = QueryTrackingBehavior.TrackAll; @@ -278,7 +300,7 @@ from outdoor in plot.DefaultIfEmpty() } public bool SavePlayer(Player player) { - Console.WriteLine($"> Begin Save... {Context.ContextId}"); + Logger.LogInformation("> Begin Save... {ContextId}:{CharacterId}", Context.ContextId, player.Character.Id); Model.Account account = player.Account; account.Currency = new AccountCurrency { @@ -314,8 +336,125 @@ public bool SavePlayer(Player player) { unlock.CharacterId = character.Id; Context.Update(unlock); - Context.ChangeTracker.Entries().DisplayStates(); - return Context.TrySaveChanges(); + bool saved = false; + int attempt = 0; + const int maxAttempts = 5; + + while (!saved && attempt < maxAttempts) { + try { + attempt++; + Context.SaveChanges(); + saved = true; + } catch (DbUpdateConcurrencyException ex) { + Logger.LogWarning("> Concurrency conflict (attempt {Attempt}) for CharacterId={CharacterId}", attempt, player.Character.Id); + foreach (EntityEntry entry in ex.Entries) { + string entityName = entry.Metadata.ClrType.Name; + if (entry.Entity is not Model.Account && entry.Entity is not Model.Character && entry.Entity is not CharacterUnlock) { + Logger.LogInformation(" Unsupported concurrency entity {EntityName}, rethrowing.", entityName); + throw; + } + + PropertyValues? databaseValues = entry.GetDatabaseValues(); + if (databaseValues == null) { + Logger.LogInformation(" Entity {EntityName} appears deleted in DB. Aborting save.", entityName); + return false; + } + PropertyValues proposedValues = entry.CurrentValues; + + Logger.LogWarning(" Diff for {EntityName}:", entityName); + foreach (IProperty property in proposedValues.Properties) { + if (property.IsConcurrencyToken) { + object? originalValue = entry.OriginalValues[property]; + object? currentValue = proposedValues[property]; + object? databaseValue2 = databaseValues[property]; + Logger.LogError(" {PropertyName}: original='{S}' current='{FormatValue1}' db='{S1}' ", property.Name, FormatValue(originalValue), FormatValue(currentValue), FormatValue(databaseValue2)); + continue; + } + if (property.Name.Equals("Password", StringComparison.OrdinalIgnoreCase)) { + continue; + } + + object? proposedValue = proposedValues[property]; + object? databaseValue = databaseValues[property]; + + // Handle CreationTime as immutable: always trust database value and suppress logging + if (property.Name.Equals("CreationTime", StringComparison.OrdinalIgnoreCase)) { + if (proposedValue is DateTime propCt && databaseValue is DateTime dbCt) { + // If they differ only by fractional seconds / timezone, normalize by taking db value + if (propCt != dbCt) { + proposedValues[property] = dbCt; + } + } else if (databaseValue != null) { + proposedValues[property] = databaseValue; // non-DateTime edge case + } + continue; // don't log CreationTime differences + } + + if (property.Name.Contains("CreationTime", StringComparison.OrdinalIgnoreCase) && + proposedValue is DateTime pvDt && pvDt == default && + databaseValue is DateTime dbDt && dbDt != default) { + proposedValues[property] = databaseValue; + continue; + } + + if (IsJsonStructurallyEqual(property.Name, proposedValue, databaseValue)) { + // Logger.LogWarning($" {property.Name}: proposed and db are structurally equal JSON. proposed='{FormatValue(proposedValue)}' db='{FormatValue(databaseValue)}'"); + continue; + } + + if (!Equals(proposedValue, databaseValue)) { + Logger.LogInformation(" {PropertyName}: proposed='{S}' db='{FormatValue1}'", property.Name, FormatValue(proposedValue), FormatValue(databaseValue)); + } + } + + entry.OriginalValues.SetValues(databaseValues); + } + } catch (Exception ex) { + Logger.LogError("> Save failed (non-concurrency) CharacterId={CharacterId} attempt={Attempt}\n{Exception}", player.Character.Id, attempt, ex); + return false; + } + } + if (!saved) { + Logger.LogError("> Save failed after {MaxAttempts} attempts CharacterId={CharacterId}", maxAttempts, player.Character.Id); + return false; + } + + // get updated values after save + (DateTime CharacterLastModified, DateTime AccountLastModified, DateTime UnlockLastModified)? newPlayer = GetLastModifiedTimestamps(character.Id); + if (newPlayer == null) { + Logger.LogError("> Save succeeded but failed to fetch updated timestamps CharacterId={CharacterId}", player.Character.Id); + } + + Logger.LogInformation("> Save complete {ContextId}:{CharacterId}", Context.ContextId, player.Character.Id); + return true; + } + + // Added helper methods for JSON diff suppression & formatting + private static readonly HashSet JsonNoiseProperties = new(StringComparer.OrdinalIgnoreCase) { + "Cooldown", + "Currency", + "Experience", + "Mastery", + "Profile", + }; + + private static bool IsJsonStructurallyEqual(string propertyName, object? proposed, object? database) { + if (!JsonNoiseProperties.Contains(propertyName)) return false; + if (proposed == null && database == null) return true; + if (proposed == null || database == null) return false; + try { + string p = JsonSerializer.Serialize(proposed); + string d = JsonSerializer.Serialize(database); + return string.Equals(p, d, StringComparison.Ordinal); + } catch { return false; } + } + + private static string FormatValue(object? value) { + if (value == null) return ""; + if (value is DateTime dt) return dt.ToString("O"); + Type t = value.GetType(); + if (t.IsPrimitive || value is string) return value.ToString() ?? string.Empty; + return t.Name; } public bool SaveCharacter(Character character) { From c8e2e1ffa695563febe2ed0e385e4ac77e4f1e4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=82ngelo=20Tadeucci?= Date: Mon, 9 Feb 2026 23:37:35 -0300 Subject: [PATCH 2/3] Make LastModified properties init-only Change LastModified in Account.cs and Character.cs from mutable setters to init-only properties (get; init;) so the timestamp can only be set during object initialization. This enforces immutability after construction and helps prevent accidental updates to the LastModified field. --- Maple2.Database/Model/Account.cs | 2 +- Maple2.Database/Model/Character.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Maple2.Database/Model/Account.cs b/Maple2.Database/Model/Account.cs index c291cb5c..b685002f 100644 --- a/Maple2.Database/Model/Account.cs +++ b/Maple2.Database/Model/Account.cs @@ -33,7 +33,7 @@ internal class Account { public bool ActiveGoldPass { get; set; } public DateTime CreationTime { get; set; } - public DateTime LastModified { get; set; } + public DateTime LastModified { get; init; } public bool Online { get; set; } public string Permissions { get; set; } diff --git a/Maple2.Database/Model/Character.cs b/Maple2.Database/Model/Character.cs index 880bbe39..27ca3f3b 100644 --- a/Maple2.Database/Model/Character.cs +++ b/Maple2.Database/Model/Character.cs @@ -30,7 +30,7 @@ internal class Character { public required Mastery Mastery { get; set; } public DateTime DeleteTime { get; set; } public DateTime CreationTime { get; set; } - public DateTime LastModified { get; set; } + public DateTime LastModified { get; init; } [return: NotNullIfNotNull(nameof(other))] public static implicit operator Character?(Maple2.Model.Game.Character? other) { From 20e14955ba46dc8d66367d30b789a52d58de836e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=82ngelo=20Tadeucci?= Date: Mon, 9 Feb 2026 23:54:15 -0300 Subject: [PATCH 3/3] Make LastModified settable; update save flow Change LastModified properties from init-only to settable on Account, Character, Unlock, and CharacterUnlock so in-memory models can be updated after DB saves. In GameStorage.User.SavePlayer add a fail-fast rethrow/log for unexpected concurrency entities, return false when fetching updated timestamps fails, and assign the fetched LastModified timestamps back to the player Account/Character/Unlock. These changes ensure saved timestamps are propagated to runtime objects and surface logic errors for unsupported entities. --- .gitignore | 2 ++ Maple2.Database/Model/CharacterUnlock.cs | 2 +- Maple2.Database/Storage/Game/GameStorage.User.cs | 7 +++++++ Maple2.Model/Game/User/Account.cs | 2 +- Maple2.Model/Game/User/Character.cs | 2 +- Maple2.Model/Game/User/Player.cs | 2 +- 6 files changed, 13 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 5939969f..e8765a73 100644 --- a/.gitignore +++ b/.gitignore @@ -378,3 +378,5 @@ FodyWeavers.xsd /Maple2.Server.Game/DebugTriggers .idea/.idea.Maple2/.idea/AugmentWebviewStateStore.xml /.idea/.idea.Maple2/.idea +.claude/settings.local.json +.mcp.json diff --git a/Maple2.Database/Model/CharacterUnlock.cs b/Maple2.Database/Model/CharacterUnlock.cs index 42f3cd13..930dd432 100644 --- a/Maple2.Database/Model/CharacterUnlock.cs +++ b/Maple2.Database/Model/CharacterUnlock.cs @@ -21,7 +21,7 @@ internal class CharacterUnlock { public required IDictionary CollectedItems { get; set; } public required InventoryExpand Expand { get; set; } public short HairSlotExpand { get; set; } - public DateTime LastModified { get; init; } + public DateTime LastModified { get; set; } public static implicit operator CharacterUnlock(Maple2.Model.Game.Unlock? other) { return other == null ? new CharacterUnlock { diff --git a/Maple2.Database/Storage/Game/GameStorage.User.cs b/Maple2.Database/Storage/Game/GameStorage.User.cs index dd4124f2..35dd05f6 100644 --- a/Maple2.Database/Storage/Game/GameStorage.User.cs +++ b/Maple2.Database/Storage/Game/GameStorage.User.cs @@ -350,6 +350,9 @@ public bool SavePlayer(Player player) { foreach (EntityEntry entry in ex.Entries) { string entityName = entry.Metadata.ClrType.Name; if (entry.Entity is not Model.Account && entry.Entity is not Model.Character && entry.Entity is not CharacterUnlock) { + // Intentionally re-throw for unsupported entity types as fail-fast behavior during development. + // SavePlayer only handles concurrency conflicts for Account, Character, and CharacterUnlock. + // If other entities are unexpectedly involved, this indicates a logic error that should be caught immediately. Logger.LogInformation(" Unsupported concurrency entity {EntityName}, rethrowing.", entityName); throw; } @@ -423,7 +426,11 @@ public bool SavePlayer(Player player) { (DateTime CharacterLastModified, DateTime AccountLastModified, DateTime UnlockLastModified)? newPlayer = GetLastModifiedTimestamps(character.Id); if (newPlayer == null) { Logger.LogError("> Save succeeded but failed to fetch updated timestamps CharacterId={CharacterId}", player.Character.Id); + return false; } + player.Account.LastModified = newPlayer.Value.AccountLastModified; + player.Character.LastModified = newPlayer.Value.CharacterLastModified; + player.Unlock.LastModified = newPlayer.Value.UnlockLastModified; Logger.LogInformation("> Save complete {ContextId}:{CharacterId}", Context.ContextId, player.Character.Id); return true; diff --git a/Maple2.Model/Game/User/Account.cs b/Maple2.Model/Game/User/Account.cs index 8e67cfa6..29500f0d 100644 --- a/Maple2.Model/Game/User/Account.cs +++ b/Maple2.Model/Game/User/Account.cs @@ -4,7 +4,7 @@ namespace Maple2.Model.Game; public class Account { #region Immutable - public DateTime LastModified { get; init; } + public DateTime LastModified { get; set; } public long Id { get; init; } public required string Username { get; init; } diff --git a/Maple2.Model/Game/User/Character.cs b/Maple2.Model/Game/User/Character.cs index b626beff..2f0b3779 100644 --- a/Maple2.Model/Game/User/Character.cs +++ b/Maple2.Model/Game/User/Character.cs @@ -8,7 +8,7 @@ namespace Maple2.Model.Game; public class Character { #region Immutable public long CreationTime { get; init; } - public DateTime LastModified { get; init; } + public DateTime LastModified { get; set; } public long Id { get; init; } public long AccountId { get; init; } diff --git a/Maple2.Model/Game/User/Player.cs b/Maple2.Model/Game/User/Player.cs index d6a386d3..6675ef04 100644 --- a/Maple2.Model/Game/User/Player.cs +++ b/Maple2.Model/Game/User/Player.cs @@ -20,7 +20,7 @@ public Player(Account account, Character character, int objectId) { } public class Unlock { - public DateTime LastModified { get; init; } + public DateTime LastModified { get; set; } public IDictionary Expand { get; init; } = new Dictionary(); public short HairSlotExpand;