fix(2053): don't reload relationships for last character (AscensionGameDev#2057)

* fix(2053): don't reload relationships for last character

* fix: change fix to a lock model to prevent a multi-character variant of the bug when the server is under extreme load (which I can't easily reproduce locally but can theorize)

Author: pandinocoder

Intersect (Core)/GameObjects/ItemBase.cs

@@ -381,6 +381,13 @@ public static ItemBase[] GetCooldownGroup(string cooldownGroup)
                 .ToArray();
         }
 
+        public string GetPaperdollForGender(Gender gender) =>
+            gender switch
+            {
+                Gender.Male => MalePaperdoll,
+                _ => FemalePaperdoll,
+            };
+
         private void Initialize()
         {
             Name = "New Item";

Intersect.Server.Core/Database/DbInterface.cs

@@ -524,14 +524,7 @@ public static Player GetUserCharacter(User user, Guid playerId, bool explicitLoa
                 try
                 {
                     using var playerContext = CreatePlayerContext(readOnly: true, explicitLoad: false);
-                    var playerEntry = playerContext.Players.Attach(player);
-                    playerEntry.Collection(p => p.Items).Query().Load();
-                    playerEntry.Collection(player => player.Bank).Load();
-                    playerEntry.Collection(player => player.Hotbar).Load();
-                    playerEntry.Collection(player => player.Items).Load();
-                    playerEntry.Collection(player => player.Quests).Load();
-                    playerEntry.Collection(player => player.Spells).Load();
-                    playerEntry.Collection(player => player.Variables).Load();
+                    player.LoadRelationships(playerContext);
                     _ = Player.Validate(player);
                 }
                 catch (Exception exception)

Intersect.Server.Core/Database/IntersectDbContext.cs

@@ -199,7 +199,7 @@ public override int SaveChanges(bool acceptAllChangesOnSuccess)
             var rowsChanged = base.SaveChanges(acceptAllChangesOnSuccess);
 
 #if DEBUG
-            Log.Debug($"DBOP-B SaveChanges({{acceptAllChangesOnSuccess}}) #{{currentExecutionId}}");
+            Log.Debug($"DBOP-B SaveChanges({acceptAllChangesOnSuccess}) #{currentExecutionId}");
 #endif
 
             return rowsChanged;
@@ -240,7 +240,7 @@ public override int SaveChanges(bool acceptAllChangesOnSuccess)
             Log.Error(concurrencyErrors.ToString());
 
 #if DEBUG
-            Log.Debug($"DBOP-C SaveChanges({{acceptAllChangesOnSuccess}}) #{{currentExecutionId}}");
+            Log.Debug($"DBOP-C SaveChanges({acceptAllChangesOnSuccess}) #{currentExecutionId}");
 #endif
 
             ServerContext.DispatchUnhandledException(

Intersect.Server.Core/Entities/Player.Database.cs

@@ -1,7 +1,4 @@
-using System;
-using System.Collections.Generic;
-using System.ComponentModel.DataAnnotations.Schema;
-using System.Linq;
+using System.ComponentModel.DataAnnotations.Schema;
 using Intersect.Logging;
 using Intersect.Server.Database;
 using Intersect.Server.Database.PlayerData;
@@ -20,6 +17,8 @@ namespace Intersect.Server.Entities
 
     public partial class Player
     {
+        private volatile bool _saving;
+        private readonly object _savingLock = new();
 
         #region Account
 
@@ -151,6 +150,15 @@ public static bool PlayerExists(string name)
 
         public void LoadRelationships(PlayerContext playerContext)
         {
+            lock (_savingLock)
+            {
+                if (_saving)
+                {
+                    Log.Warn($"Skipping loading relationships for player {Id} because it is being saved.");
+                    return;
+                }
+            }
+
             var entityEntry = playerContext.Players.Attach(this);
             entityEntry.Collection(p => p.Bank).Load();
             entityEntry.Collection(p => p.Hotbar).Load();

Intersect.Server.Core/Entities/Player.cs

@@ -1,11 +1,6 @@
-using System;
 using System.Collections.Concurrent;
-using System.Collections.Generic;
 using System.ComponentModel.DataAnnotations.Schema;
 using System.Diagnostics;
-using System.Linq;
-using System.Threading;
-
 using Intersect.Enums;
 using Intersect.GameObjects;
 using Intersect.GameObjects.Crafting;
@@ -407,6 +402,11 @@ public void TryLogout(bool force = false, bool softLogout = false)
 
         private void Logout(bool softLogout = false)
         {
+            lock (_savingLock)
+            {
+                _saving = true;
+            }
+
             if (MapController.TryGetInstanceFromMap(MapId, MapInstanceId, out var instance))
             {
                 instance.RemoveEntity(this);
@@ -511,6 +511,11 @@ public void CompleteLogout()
 
             User?.Save();
 
+            lock (_savingLock)
+            {
+                _saving = false;
+            }
+
             Dispose();
 
 #if DIAGNOSTIC

Intersect.Server.Core/Networking/PacketHandler.cs

@@ -654,7 +654,7 @@ public void HandlePacket(Client client, LogoutPacket packet)
                 (Options.MaxCharacters > 1 || !Options.Instance.PlayerOpts.SkipCharacterSelect))
             {
                 client.Entity?.TryLogout(false, true);
-                client.Entity = null;
+                client.Entity = default;
                 PacketSender.SendPlayerCharacters(client);
             }
             else

Intersect.Server.Core/Networking/PacketSender.cs

@@ -1253,9 +1253,23 @@ public static void SendCreateCharacter(Client client)
         //CharactersPacket
         public static void SendPlayerCharacters(Client client)
         {
-            var characters = new List<CharacterPacket>();
-            if (client.User == null)
+            if (client == default)
             {
+                Log.Warn($"Tried to {nameof(SendPlayerCharacters)}() to a null client?");
+                return;
+            }
+
+            var user = client.User;
+            if (user == default)
+            {
+                Log.Warn($"Tried to {nameof(SendPlayerCharacters)}() to client with no user? ({client.Id})");
+                return;
+            }
+
+            var clientCharacters = client.Characters;
+            if (clientCharacters == default)
+            {
+                Log.Warn($"Tried to {nameof(SendPlayerCharacters)}() to client with no characters? (client {client.Id}/user {user.Id})");
                 return;
             }
 
@@ -1267,73 +1281,92 @@ public static void SendPlayerCharacters(Client client)
                 }
             }
 
-            var clientCharacters = client?.Characters;
-            if (clientCharacters == default)
+            var characterPackets = new List<CharacterPacket>();
+
+            var equipmentSlotsOptions = Options.EquipmentSlots;
+            var paperdollOrderOptions = Options.PaperdollOrder;
+
+            if (clientCharacters.Count < 1)
             {
-                Log.Error($"PLEASE REPORT THIS WITH LOGS: About to crash because {(client == default ? nameof(client) : nameof(client.Characters))} is null.");
+                CharactersPacket emptyBulkCharactersPacket = new(
+                    Array.Empty<CharacterPacket>(),
+                    client.Characters.Count < Options.MaxCharacters
+                );
+
+                if (!client.Send(emptyBulkCharactersPacket))
+                {
+                    Log.Error($"Failed to send empty bulk characters packet to {client.Id}");
+                }
+
+                return;
             }
 
-            if (client.Characters.Count > 0) /* TODO: Fix NRE when logging out and back in */
+            foreach (var character in clientCharacters.OrderByDescending(p => p.LastOnline))
             {
-                foreach (var character in client.Characters.OrderByDescending(p => p.LastOnline))
-                {
-                    var equipmentArray = character.Equipment;
-                    var equipment = new EquipmentFragment[Options.EquipmentSlots.Count + 1];
+                var equipmentArray = character.Equipment;
+                var equipment = new EquipmentFragment[equipmentSlotsOptions.Count + 1];
 
-                    //Draw the equipment/paperdolls
-                    for (var z = 0; z < Options.PaperdollOrder[1].Count; z++)
+                // Draw the equipment/paperdolls
+                var paperdollOrderOptionLayer1 = paperdollOrderOptions[1];
+                for (var z = 0; z < paperdollOrderOptionLayer1.Count; z++)
+                {
+                    var indexOfPaperdoll = equipmentSlotsOptions.IndexOf(Options.PaperdollOrder[1][z]);
+                    if (indexOfPaperdoll < 0)
                     {
-                        if (Options.EquipmentSlots.IndexOf(Options.PaperdollOrder[1][z]) > -1)
-                        {
-                            if (equipmentArray[Options.EquipmentSlots.IndexOf(Options.PaperdollOrder[1][z])] > -1 &&
-                                equipmentArray[Options.EquipmentSlots.IndexOf(Options.PaperdollOrder[1][z])] <
-                                Options.MaxInvItems)
-                            {
-                                var paperdollOrder = Options.PaperdollOrder[1][z];
-                                var equipmentSlot = Options.EquipmentSlots.IndexOf(paperdollOrder);
-                                var itemIndex = equipmentArray[equipmentSlot];
-
-                                var itemId = character
-                                    .Items[itemIndex]
-                                    .ItemId;
-
-                                if (ItemBase.Get(itemId) != null)
-                                {
-                                    var itemdata = ItemBase.Get(itemId);
-                                    equipment[z] = new EquipmentFragment
-                                    {
-                                        Name = character.Gender == 0 ? itemdata.MalePaperdoll : itemdata.FemalePaperdoll,
-                                        RenderColor = itemdata.Color,
-                                    };
-                                }
-                            }
-                        }
-                        else
+                        const string equipmentFragmentNamePlayer = "Player";
+                        if (Options.PaperdollOrder[1][z] == equipmentFragmentNamePlayer)
                         {
-                            if (Options.PaperdollOrder[1][z] == "Player")
-                            {
-                                equipment[z] = new EquipmentFragment { Name = "Player" };
-                            }
+                            equipment[z] = new EquipmentFragment { Name = equipmentFragmentNamePlayer };
                         }
+
+                        continue;
                     }
 
-                    characters.Add(
-                        new CharacterPacket(
-                            character.Id, character.Name, character.Sprite, character.Face, character.Level,
-                            ClassBase.GetName(character.ClassId), equipment
-                        )
-                    );
+                    var inventoryIndexOfEquip = equipmentArray[indexOfPaperdoll];
+                    if (inventoryIndexOfEquip <= -1 || inventoryIndexOfEquip >= Options.MaxInvItems)
+                    {
+                        continue;
+                    }
+
+                    var paperdollOrder = paperdollOrderOptionLayer1[z];
+                    var equipmentSlot = equipmentSlotsOptions.IndexOf(paperdollOrder);
+                    var itemIndex = equipmentArray[equipmentSlot];
+
+                    var itemId = character.Items[itemIndex].ItemId;
+
+                    if (!ItemBase.TryGet(itemId, out var itemDescriptor))
+                    {
+                        continue;
+                    }
+
+                    equipment[z] = new EquipmentFragment
+                    {
+                        Name = itemDescriptor.GetPaperdollForGender(character.Gender),
+                        RenderColor = itemDescriptor.Color,
+                    };
                 }
+
+                characterPackets.Add(
+                    new CharacterPacket(
+                        character.Id,
+                        character.Name,
+                        character.Sprite,
+                        character.Face,
+                        character.Level,
+                        ClassBase.GetName(character.ClassId),
+                        equipment
+                    )
+                );
             }
 
-            CharactersPacket packet = new(
-                characters.ToArray(),
+            CharactersPacket bulkCharactersPacket = new(
+                characterPackets.ToArray(),
                 client.Characters.Count < Options.MaxCharacters
             );
 
-            if (!client.Send(packet))
+            if (!client.Send(bulkCharactersPacket))
             {
-                Log.Error($"Failed to send {packet.Characters.Length} characters to {client.Id}");
+                Log.Error($"Failed to send {bulkCharactersPacket.Characters.Length} characters to {client.Id}");
             }
         }
 

Intersect.sln.DotSettings

@@ -3,6 +3,7 @@
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=cipherdata/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Expiries/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Migratable/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=Paperdoll/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=plaindata/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=randomblob/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
-	<s:Boolean x:Key="/Default/UserDictionary/Words/=singleplayer/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
+