Fix creating and deleting character slots crashing the client and server (#2172)

* Fix creating a character slot crashing the client

* a

* Fix deleting character profiles

Content.Client/ClientPreferencesManager.cs

@@ -26,6 +26,7 @@ namespace Content.Client
                 HandlePreferencesAndSettings);
             _netManager.RegisterNetMessage<MsgUpdateCharacter>(nameof(MsgUpdateCharacter));
             _netManager.RegisterNetMessage<MsgSelectCharacter>(nameof(MsgSelectCharacter));
+            _netManager.RegisterNetMessage<MsgDeleteCharacter>(nameof(MsgDeleteCharacter));
         }
 
         public void SelectCharacter(ICharacterProfile profile)
@@ -54,7 +55,12 @@ namespace Content.Client
 
         public void CreateCharacter(ICharacterProfile profile)
         {
-            UpdateCharacter(profile, Preferences.FirstEmptySlot);
+            var characters = Preferences.Characters.ToList();
+
+            characters.Add(profile);
+            Preferences = new PlayerPreferences(characters, Preferences.SelectedCharacterIndex);
+
+            UpdateCharacter(profile, characters.Count - 1);
         }
 
         public void DeleteCharacter(ICharacterProfile profile)
@@ -64,7 +70,11 @@ namespace Content.Client
 
         public void DeleteCharacter(int slot)
         {
-            UpdateCharacter(null, slot);
+            var characters = Preferences.Characters.Where((profile, index) => index != slot).ToArray();
+            Preferences = new PlayerPreferences(characters, Preferences.SelectedCharacterIndex);
+            var msg = _netManager.CreateNetMessage<MsgDeleteCharacter>();
+            msg.Slot = slot;
+            _netManager.ClientSendMessage(msg);
         }
 
         private void HandlePreferencesAndSettings(MsgPreferencesAndSettings message)

Content.Server/Database/ServerDbManager.cs

@@ -27,7 +27,7 @@ namespace Content.Server.Database
         // Preferences
         Task<PlayerPreferences> InitPrefsAsync(NetUserId userId, ICharacterProfile defaultProfile);
         Task SaveSelectedCharacterIndexAsync(NetUserId userId, int index);
-        Task SaveCharacterSlotAsync(NetUserId userId, ICharacterProfile profile, int slot);
+        Task SaveCharacterSlotAsync(NetUserId userId, ICharacterProfile? profile, int slot);
         Task<PlayerPreferences?> GetPlayerPreferencesAsync(NetUserId userId);
 
         // Username assignment (for guest accounts, so they persist GUID)
@@ -90,7 +90,7 @@ namespace Content.Server.Database
             return _db.SaveSelectedCharacterIndexAsync(userId, index);
         }
 
-        public Task SaveCharacterSlotAsync(NetUserId userId, ICharacterProfile profile, int slot)
+        public Task SaveCharacterSlotAsync(NetUserId userId, ICharacterProfile? profile, int slot)
         {
             return _db.SaveCharacterSlotAsync(userId, profile, slot);
         }

Content.Server/Preferences/ServerPreferencesManager.cs

@@ -42,6 +42,8 @@ namespace Content.Server.Preferences
                 HandleSelectCharacterMessage);
             _netManager.RegisterNetMessage<MsgUpdateCharacter>(nameof(MsgUpdateCharacter),
                 HandleUpdateCharacterMessage);
+            _netManager.RegisterNetMessage<MsgDeleteCharacter>(nameof(MsgDeleteCharacter),
+                HandleDeleteCharacterMessage);
         }
 
 
@@ -77,6 +79,13 @@ namespace Content.Server.Preferences
             var profile = message.Profile;
             var userId = message.MsgChannel.UserId;
 
+            if (profile == null)
+            {
+                Logger.WarningS("prefs",
+                    $"User {userId} sent a {nameof(MsgUpdateCharacter)} with a null profile in slot {slot}.");
+                return;
+            }
+
             if (!_cachedPlayerPrefs.TryGetValue(userId, out var prefsData) || !prefsData.PrefsLoaded.IsCompleted)
             {
                 Logger.WarningS("prefs", $"User {userId} tried to modify preferences before they loaded.");
@@ -103,6 +112,35 @@ namespace Content.Server.Preferences
             }
         }
 
+        private async void HandleDeleteCharacterMessage(MsgDeleteCharacter message)
+        {
+            var slot = message.Slot;
+            var userId = message.MsgChannel.UserId;
+
+            if (!_cachedPlayerPrefs.TryGetValue(userId, out var prefsData) || !prefsData.PrefsLoaded.IsCompleted)
+            {
+                Logger.WarningS("prefs", $"User {userId} tried to modify preferences before they loaded.");
+                return;
+            }
+
+            if (slot < 0 || slot >= MaxCharacterSlots)
+            {
+                return;
+            }
+
+            var curPrefs = prefsData.Prefs!;
+
+            var arr = new ICharacterProfile[MaxCharacterSlots];
+            curPrefs.Characters.Where((profile, index) => index != slot).ToArray().CopyTo(arr, 0);
+
+            prefsData.Prefs = new PlayerPreferences(arr, slot);
+
+            if (ShouldStorePrefs(message.MsgChannel.AuthType))
+            {
+                await _db.SaveCharacterSlotAsync(message.MsgChannel.UserId, null, message.Slot);
+            }
+        }
+
         public async void OnClientConnected(IPlayerSession session)
         {
             if (!ShouldStorePrefs(session.ConnectedClient.AuthType))

Content.Shared/Preferences/PlayerPreferences.cs

@@ -41,11 +41,20 @@ namespace Content.Shared.Preferences
         /// </summary>
         public ICharacterProfile SelectedCharacter => Characters.ElementAtOrDefault(SelectedCharacterIndex);
 
-        public int FirstEmptySlot => IndexOfCharacter(null);
+        public int FirstEmptySlot()
+        {
+            var firstEmpty = IndexOfCharacter(null);
+            return firstEmpty == -1 ? _characters.Count : firstEmpty;
+        }
 
         public int IndexOfCharacter(ICharacterProfile profile)
         {
             return _characters.FindIndex(x => x == profile);
         }
+
+        public bool TryIndexOfCharacter(ICharacterProfile profile, out int index)
+        {
+            return (index = IndexOfCharacter(profile)) != -1;
+        }
     }
 }

Content.Shared/Preferences/SharedPreferencesManager.cs

@@ -128,5 +128,32 @@ namespace Content.Shared.Preferences
                 }
             }
         }
+
+        /// <summary>
+        /// The client sends this to delete a character profile.
+        /// </summary>
+        protected class MsgDeleteCharacter : NetMessage
+        {
+            #region REQUIRED
+
+            public const MsgGroups GROUP = MsgGroups.Command;
+            public const string NAME = nameof(MsgDeleteCharacter);
+
+            public MsgDeleteCharacter(INetChannel channel) : base(NAME, GROUP) { }
+
+            #endregion
+
+            public int Slot;
+
+            public override void ReadFromBuffer(NetIncomingMessage buffer)
+            {
+                Slot = buffer.ReadInt32();
+            }
+
+            public override void WriteToBuffer(NetOutgoingMessage buffer)
+            {
+                buffer.Write(Slot);
+            }
+        }
     }
 }