Make the client validate character information (#3563)

Co-authored-by: Metal Gear Sloth <metalgearsloth@gmail.com>

Content.Client/ClientPreferencesManager.cs

@@ -47,6 +47,7 @@ namespace Content.Client
 
         public void UpdateCharacter(ICharacterProfile profile, int slot)
         {
+            profile.EnsureValid();
             var characters = new Dictionary<int, ICharacterProfile>(Preferences.Characters) {[slot] = profile};
             Preferences = new PlayerPreferences(characters, Preferences.SelectedCharacterIndex, Preferences.AdminOOCColor);
             var msg = _netManager.CreateNetMessage<MsgUpdateCharacter>();

Content.Client/UserInterface/CharacterSetupGui.cs

@@ -136,7 +136,7 @@ namespace Content.Client.UserInterface
                 MinSize = (2, 0)
             });
             _humanoidProfileEditor = new HumanoidProfileEditor(preferencesManager, prototypeManager, entityManager);
-            _humanoidProfileEditor.OnProfileChanged += newProfile => { UpdateUI(); };
+            _humanoidProfileEditor.OnProfileChanged += ProfileChanged;
             hBox.AddChild(_humanoidProfileEditor);
 
             UpdateUI();
@@ -155,6 +155,12 @@ namespace Content.Client.UserInterface
 
         public void Save() => _humanoidProfileEditor.Save();
 
+        private void ProfileChanged(ICharacterProfile profile, int profileSlot)
+        {
+            _humanoidProfileEditor.UpdateControls();
+            UpdateUI();
+        }
+
         private void UpdateUI()
         {
             var numberOfFullSlots = 0;

Content.Client/UserInterface/HumanoidProfileEditor.cs

@@ -62,7 +62,7 @@ namespace Content.Client.UserInterface
         private bool _isDirty;
         public int CharacterSlot;
         public HumanoidCharacterProfile Profile;
-        public event Action<HumanoidCharacterProfile> OnProfileChanged;
+        public event Action<HumanoidCharacterProfile, int> OnProfileChanged;
 
         public HumanoidProfileEditor(IClientPreferencesManager preferencesManager, IPrototypeManager prototypeManager,
             IEntityManager entityManager)
@@ -709,7 +709,7 @@ namespace Content.Client.UserInterface
         {
             IsDirty = false;
             _preferencesManager.UpdateCharacter(Profile, CharacterSlot);
-            OnProfileChanged?.Invoke(Profile);
+            OnProfileChanged?.Invoke(Profile, CharacterSlot);
         }
 
         private bool IsDirty

Content.Server/Preferences/ServerPreferencesManager.cs

@@ -48,7 +48,6 @@ namespace Content.Server.Preferences
                 HandleDeleteCharacterMessage);
         }
 
-
         private async void HandleSelectCharacterMessage(MsgSelectCharacter message)
         {
             var index = message.SelectedCharacterIndex;
@@ -107,9 +106,11 @@ namespace Content.Server.Preferences
 
             var curPrefs = prefsData.Prefs!;
 
+            profile.EnsureValid();
+
             var profiles = new Dictionary<int, ICharacterProfile>(curPrefs.Characters)
             {
-                [slot] = HumanoidCharacterProfile.EnsureValid((HumanoidCharacterProfile) profile, _protos)
+                [slot] = profile
             };
 
             prefsData.Prefs = new PlayerPreferences(profiles, slot, curPrefs.AdminOOCColor);

Content.Shared/Preferences/HumanoidCharacterProfile.cs

@@ -201,90 +201,6 @@ namespace Content.Shared.Preferences
             return new(this, _jobPriorities, list);
         }
 
-        /// <summary>
-        ///     Makes this profile valid so there's no bad data like negative ages.
-        /// </summary>
-        public static HumanoidCharacterProfile EnsureValid(
-            HumanoidCharacterProfile profile,
-            IPrototypeManager prototypeManager)
-        {
-            var age = Math.Clamp(profile.Age, MinimumAge, MaximumAge);
-            var sex = profile.Sex switch
-            {
-                Sex.Male => Sex.Male,
-                Sex.Female => Sex.Female,
-                _ => Sex.Male // Invalid enum values.
-            };
-            var gender = profile.Gender switch
-            {
-                Gender.Epicene => Gender.Epicene,
-                Gender.Female => Gender.Female,
-                Gender.Male => Gender.Male,
-                Gender.Neuter => Gender.Neuter,
-                _ => Gender.Epicene // Invalid enum values.
-            };
-
-            string name;
-            if (string.IsNullOrEmpty(profile.Name))
-            {
-                name = "Urist McHands";
-            }
-            else if (profile.Name.Length > MaxNameLength)
-            {
-                name = profile.Name[..MaxNameLength];
-            }
-            else
-            {
-                name = profile.Name;
-            }
-
-            // TODO: Avoid Z̨͇̙͉͎̭͔̼̿͋A͚̖̞̗̞͈̓̾̀ͩͩ̔L̟ͮ̈͝G̙O͍͎̗̺̺ͫ̀̽͊̓͝ͅ tier shenanigans.
-            // And other stuff like RTL overrides and such.
-            // Probably also emojis...
-
-            name = name.Trim();
-
-            var appearance = HumanoidCharacterAppearance.EnsureValid(profile.Appearance);
-
-            var prefsUnavailableMode = profile.PreferenceUnavailable switch
-            {
-                PreferenceUnavailableMode.StayInLobby => PreferenceUnavailableMode.StayInLobby,
-                PreferenceUnavailableMode.SpawnAsOverflow => PreferenceUnavailableMode.SpawnAsOverflow,
-                _ => PreferenceUnavailableMode.StayInLobby // Invalid enum values.
-            };
-
-            var clothing = profile.Clothing switch
-            {
-                ClothingPreference.Jumpsuit => ClothingPreference.Jumpsuit,
-                ClothingPreference.Jumpskirt => ClothingPreference.Jumpskirt,
-                _ => ClothingPreference.Jumpsuit // Invalid enum values.
-            };
-
-            var backpack = profile.Backpack switch
-            {
-                BackpackPreference.Backpack => BackpackPreference.Backpack,
-                BackpackPreference.Satchel => BackpackPreference.Satchel,
-                BackpackPreference.Duffelbag => BackpackPreference.Duffelbag,
-                _ => BackpackPreference.Backpack // Invalid enum values.
-            };
-
-            var priorities = new Dictionary<string, JobPriority>(profile.JobPriorities
-                .Where(p => prototypeManager.HasIndex<JobPrototype>(p.Key) && p.Value switch
-                {
-                    JobPriority.Never => false, // Drop never since that's assumed default.
-                    JobPriority.Low => true,
-                    JobPriority.Medium => true,
-                    JobPriority.High => true,
-                    _ => false
-                }));
-
-            var antags = profile.AntagPreferences
-                .Where(prototypeManager.HasIndex<AntagPrototype>)
-                .ToList();
-
-            return new HumanoidCharacterProfile(name, age, sex, gender, appearance, clothing, backpack, priorities, prefsUnavailableMode, antags);
-        }
-
         public string Summary =>
             Loc.GetString(
                 "humanoid-character-profile-summary",
@@ -308,6 +224,107 @@ namespace Content.Shared.Preferences
             return Appearance.MemberwiseEquals(other.Appearance);
         }
 
+        public void EnsureValid()
+        {
+            var age = Math.Clamp(Age, MinimumAge, MaximumAge);
+
+            var sex = Sex switch
+            {
+                Sex.Male => Sex.Male,
+                Sex.Female => Sex.Female,
+                _ => Sex.Male // Invalid enum values.
+            };
+
+            var gender = Gender switch
+            {
+                Gender.Epicene => Gender.Epicene,
+                Gender.Female => Gender.Female,
+                Gender.Male => Gender.Male,
+                Gender.Neuter => Gender.Neuter,
+                _ => Gender.Epicene // Invalid enum values.
+            };
+
+            string name;
+            if (string.IsNullOrEmpty(Name))
+            {
+                name = "Urist McHands";
+            }
+            else if (Name.Length > MaxNameLength)
+            {
+                name = Name[..MaxNameLength];
+            }
+            else
+            {
+                name = Name;
+            }
+
+            // TODO: Avoid Z̨͇̙͉͎̭͔̼̿͋A͚̖̞̗̞͈̓̾̀ͩͩ̔L̟ͮ̈͝G̙O͍͎̗̺̺ͫ̀̽͊̓͝ͅ tier shenanigans.
+            // And other stuff like RTL overrides and such.
+            // Probably also emojis...
+
+            name = name.Trim();
+
+            var appearance = HumanoidCharacterAppearance.EnsureValid(Appearance);
+
+            var prefsUnavailableMode = PreferenceUnavailable switch
+            {
+                PreferenceUnavailableMode.StayInLobby => PreferenceUnavailableMode.StayInLobby,
+                PreferenceUnavailableMode.SpawnAsOverflow => PreferenceUnavailableMode.SpawnAsOverflow,
+                _ => PreferenceUnavailableMode.StayInLobby // Invalid enum values.
+            };
+
+            var clothing = Clothing switch
+            {
+                ClothingPreference.Jumpsuit => ClothingPreference.Jumpsuit,
+                ClothingPreference.Jumpskirt => ClothingPreference.Jumpskirt,
+                _ => ClothingPreference.Jumpsuit // Invalid enum values.
+            };
+
+            var backpack = Backpack switch
+            {
+                BackpackPreference.Backpack => BackpackPreference.Backpack,
+                BackpackPreference.Satchel => BackpackPreference.Satchel,
+                BackpackPreference.Duffelbag => BackpackPreference.Duffelbag,
+                _ => BackpackPreference.Backpack // Invalid enum values.
+            };
+
+            var prototypeManager = IoCManager.Resolve<IPrototypeManager>();
+
+            var priorities = new Dictionary<string, JobPriority>(JobPriorities
+                .Where(p => prototypeManager.HasIndex<JobPrototype>(p.Key) && p.Value switch
+                {
+                    JobPriority.Never => false, // Drop never since that's assumed default.
+                    JobPriority.Low => true,
+                    JobPriority.Medium => true,
+                    JobPriority.High => true,
+                    _ => false
+                }));
+
+            var antags = AntagPreferences
+                .Where(prototypeManager.HasIndex<AntagPrototype>)
+                .ToList();
+
+            Name = name;
+            Age = age;
+            Sex = sex;
+            Gender = gender;
+            Appearance = appearance;
+            Clothing = clothing;
+            Backpack = backpack;
+
+            _jobPriorities.Clear();
+
+            foreach (var (job, priority) in priorities)
+            {
+                _jobPriorities.Add(job, priority);
+            }
+
+            PreferenceUnavailable = prefsUnavailableMode;
+
+            _antagPreferences.Clear();
+            _antagPreferences.AddRange(antags);
+        }
+
         public override bool Equals(object? obj)
         {
             return obj is HumanoidCharacterProfile other && MemberwiseEquals(other);

Content.Shared/Preferences/ICharacterProfile.cs

@@ -5,7 +5,14 @@ namespace Content.Shared.Preferences
     public interface ICharacterProfile
     {
         string Name { get; }
+
         ICharacterAppearance CharacterAppearance { get; }
+
         bool MemberwiseEquals(ICharacterProfile other);
+
+        /// <summary>
+        ///     Makes this profile valid so there's no bad data like negative ages.
+        /// </summary>
+        void EnsureValid();
     }
 }