Cleanup, code review, comments.

Content.Client/Administration/ClientAdminManager.cs

@@ -21,8 +21,6 @@ namespace Content.Client.Administration
 
         public event Action? AdminStatusUpdated;
 
-        public AdminFlags? Flags => _adminData?.Flags;
-
         public bool HasFlag(AdminFlags flag)
         {
             return _adminData?.HasFlag(flag) ?? false;
@@ -67,12 +65,12 @@ namespace Content.Client.Administration
             _adminData = message.Admin;
             if (_adminData != null)
             {
-                var flagsText = string.Join("|", AdminFlagsExt.FlagsToNames(_adminData.Flags));
+                var flagsText = string.Join("|", AdminFlagsHelper.FlagsToNames(_adminData.Flags));
                 Logger.InfoS("admin", $"Updated admin status: {_adminData.Active}/{_adminData.Title}/{flagsText}");
             }
             else
             {
-                Logger.InfoS("admin", $"Updated admin status: Not admin");
+                Logger.InfoS("admin", "Updated admin status: Not admin");
             }
 
             AdminStatusUpdated?.Invoke();

Content.Client/Administration/IClientAdminManager.cs

@@ -1,19 +1,50 @@
 using System;
 using Content.Shared.Administration;
 
+#nullable enable
+
 namespace Content.Client.Administration
 {
+    /// <summary>
+    ///     Manages server admin permissions for the local player.
+    /// </summary>
     public interface IClientAdminManager
     {
-        public event Action AdminStatusUpdated;
+        /// <summary>
+        ///     Fired when the admin status of the local player changes, such as losing admin privileges.
+        /// </summary>
+        event Action AdminStatusUpdated;
 
-        AdminFlags? Flags { get; }
+        /// <summary>
+        ///     Checks whether the local player has an admin flag.
+        /// </summary>
+        /// <param name="flag">The flags to check. Multiple flags can be specified, they must all be held.</param>
+        /// <returns>False if the local player is not an admin, inactive, or does not have all the flags specified.</returns>
         bool HasFlag(AdminFlags flag);
 
+        /// <summary>
+        ///     Check if a player can execute a specified console command.
+        /// </summary>
         bool CanCommand(string cmdName);
+
+        /// <summary>
+        ///     Check if the local player can open the VV menu.
+        /// </summary>
         bool CanViewVar();
+
+        /// <summary>
+        ///     Check if the local player can spawn stuff in with the entity/tile spawn panel.
+        /// </summary>
         bool CanAdminPlace();
+
+        /// <summary>
+        ///     Check if the local player can execute server-side C# scripts.
+        /// </summary>
         bool CanScript();
+
+        /// <summary>
+        ///     Check if the local player can open the admin menu.
+        /// </summary>
         bool CanAdminMenu();
 
         void Initialize();

Content.Client/Chat/ChatBox.cs

@@ -106,15 +106,12 @@ namespace Content.Client.Chat
             AllButton.OnToggled += OnFilterToggled;
             LocalButton.OnToggled += OnFilterToggled;
             OOCButton.OnToggled += OnFilterToggled;
+            AdminButton.OnToggled += OnFilterToggled;
 
             hBox.AddChild(AllButton);
             hBox.AddChild(LocalButton);
             hBox.AddChild(OOCButton);
-            if(AdminButton != null)
+            hBox.AddChild(AdminButton);
-            {
-                AdminButton.OnToggled += OnFilterToggled;
-                hBox.AddChild(AdminButton);
-            }
 
             AddChild(outerVBox);
         }

Content.Client/UserInterface/Permissions/PermissionsEui.cs

@@ -241,7 +241,7 @@ namespace Content.Client.UserInterface.Permissions
 
                 al.AddChild(rankControl);
 
-                var flagsText = AdminFlagsExt.PosNegFlagsText(admin.PosFlags, admin.NegFlags);
+                var flagsText = AdminFlagsHelper.PosNegFlagsText(admin.PosFlags, admin.NegFlags);
 
                 al.AddChild(new Label
                 {
@@ -264,7 +264,7 @@ namespace Content.Client.UserInterface.Permissions
             foreach (var kv in s.AdminRanks)
             {
                 var rank = kv.Value;
-                var flagsText = string.Join(' ', AdminFlagsExt.FlagsToNames(rank.Flags).Select(f => $"+{f}"));
+                var flagsText = string.Join(' ', AdminFlagsHelper.FlagsToNames(rank.Flags).Select(f => $"+{f}"));
                 _menu.AdminRanksList.AddChild(new Label {Text = rank.Name});
                 _menu.AdminRanksList.AddChild(new Label
                 {
@@ -390,7 +390,7 @@ namespace Content.Client.UserInterface.Permissions
                     VSeparationOverride = 0
                 };
 
-                foreach (var flag in AdminFlagsExt.AllFlags)
+                foreach (var flag in AdminFlagsHelper.AllFlags)
                 {
                     // Can only grant out perms you also have yourself.
                     // Primarily intended to prevent people giving themselves +HOST with +PERMISSIONS but generalized.
@@ -527,7 +527,7 @@ namespace Content.Client.UserInterface.Permissions
 
                 NameEdit = new LineEdit
                 {
-                    PlaceHolder = "Rank name",
+                    PlaceHolder = Loc.GetString("Rank name"),
                 };
 
                 if (data != null)
@@ -538,7 +538,7 @@ namespace Content.Client.UserInterface.Permissions
                 SaveButton = new Button {Text = Loc.GetString("Save"), SizeFlagsHorizontal = SizeFlags.ShrinkEnd | SizeFlags.Expand};
                 var flagsBox = new VBoxContainer();
 
-                foreach (var flag in AdminFlagsExt.AllFlags)
+                foreach (var flag in AdminFlagsHelper.AllFlags)
                 {
                     // Can only grant out perms you also have yourself.
                     // Primarily intended to prevent people giving themselves +HOST with +PERMISSIONS but generalized.

Content.Server/Administration/AdminManager.cs

@@ -95,7 +95,7 @@ namespace Content.Server.Administration
             _chat.DispatchServerMessage(session, Loc.GetString("You are now an admin."));
 
             var plyData = session.ContentData()!;
-            plyData.ExplicitlyDeadminned = true;
+            plyData.ExplicitlyDeadminned = false;
             reg.Data.Active = true;
 
             _chat.SendAdminAnnouncement(Loc.GetString("{0} re-adminned themselves.", session.Name));
@@ -203,7 +203,7 @@ namespace Content.Server.Administration
                     if (map.TryGetNode("Flags", out var flagsNode))
                     {
                         var flagNames = flagsNode.AsString().Split(",", StringSplitOptions.RemoveEmptyEntries);
-                        var flags = AdminFlagsExt.NamesToFlags(flagNames);
+                        var flags = AdminFlagsHelper.NamesToFlags(flagNames);
                         foreach (var cmd in commands)
                         {
                             if (!_adminCommands.TryGetValue(cmd, out var exFlags))
@@ -316,7 +316,7 @@ namespace Content.Server.Administration
                 var data = new AdminData
                 {
                     Title = Loc.GetString("Host"),
-                    Flags = AdminFlagsExt.Everything,
+                    Flags = AdminFlagsHelper.Everything,
                 };
 
                 return (data, null, true);
@@ -335,12 +335,12 @@ namespace Content.Server.Administration
 
                 if (dbData.AdminRank != null)
                 {
-                    flags = AdminFlagsExt.NamesToFlags(dbData.AdminRank.Flags.Select(p => p.Flag));
+                    flags = AdminFlagsHelper.NamesToFlags(dbData.AdminRank.Flags.Select(p => p.Flag));
                 }
 
                 foreach (var dbFlag in dbData.Flags)
                 {
-                    var flag = AdminFlagsExt.NameToFlag(dbFlag.Flag);
+                    var flag = AdminFlagsHelper.NameToFlag(dbFlag.Flag);
                     if (dbFlag.Negative)
                     {
                         flags &= ~flag;

Content.Server/Administration/Commands/DeAdminCommand.cs

@@ -6,7 +6,7 @@ using Robust.Shared.IoC;
 
 #nullable enable
 
-namespace Content.Server.Administration
+namespace Content.Server.Administration.Commands
 {
     [UsedImplicitly]
     [AdminCommand(AdminFlags.None)]

Content.Server/Administration/Commands/ReAdminCommand.cs

@@ -4,7 +4,7 @@ using Robust.Shared.IoC;
 
 #nullable enable
 
-namespace Content.Server.Administration
+namespace Content.Server.Administration.Commands
 {
     [AnyCommand]
     public class ReAdminCommand : IClientCommand

Content.Server/Administration/PermissionsEui.cs

@@ -39,17 +39,17 @@ namespace Content.Server.Administration
 
             StateDirty();
             LoadFromDb();
-            _adminManager.OnPermsChanged += AdminManagerOnOnPermsChanged;
+            _adminManager.OnPermsChanged += AdminManagerOnPermsChanged;
         }
 
         public override void Closed()
         {
             base.Closed();
 
-            _adminManager.OnPermsChanged -= AdminManagerOnOnPermsChanged;
+            _adminManager.OnPermsChanged -= AdminManagerOnPermsChanged;
         }
 
-        private void AdminManagerOnOnPermsChanged(AdminPermsChangedEventArgs obj)
+        private void AdminManagerOnPermsChanged(AdminPermsChangedEventArgs obj)
         {
             // Close UI if user loses +PERMISSIONS.
             if (obj.Player == Player && !UserAdminFlagCheck(AdminFlags.Permissions))
@@ -72,8 +72,8 @@ namespace Content.Server.Administration
             {
                 Admins = _admins.Select(p => new PermissionsEuiState.AdminData
                 {
-                    PosFlags = AdminFlagsExt.NamesToFlags(p.a.Flags.Where(f => !f.Negative).Select(f => f.Flag)),
+                    PosFlags = AdminFlagsHelper.NamesToFlags(p.a.Flags.Where(f => !f.Negative).Select(f => f.Flag)),
-                    NegFlags = AdminFlagsExt.NamesToFlags(p.a.Flags.Where(f => f.Negative).Select(f => f.Flag)),
+                    NegFlags = AdminFlagsHelper.NamesToFlags(p.a.Flags.Where(f => f.Negative).Select(f => f.Flag)),
                     Title = p.a.Title,
                     RankId = p.a.AdminRankId,
                     UserId = new NetUserId(p.a.UserId),
@@ -82,7 +82,7 @@ namespace Content.Server.Administration
 
                 AdminRanks = _adminRanks.ToDictionary(a => a.Id, a => new PermissionsEuiState.AdminRankData
                 {
-                    Flags = AdminFlagsExt.NamesToFlags(a.Flags.Select(p => p.Flag)),
+                    Flags = AdminFlagsHelper.NamesToFlags(a.Flags.Select(p => p.Flag)),
                     Name = a.Name
                 })
             };
@@ -185,7 +185,7 @@ namespace Content.Server.Administration
 
             await _db.UpdateAdminRankAsync(rank);
 
-            var flagText = string.Join(' ', AdminFlagsExt.FlagsToNames(ur.Flags).Select(f => $"+{f}"));
+            var flagText = string.Join(' ', AdminFlagsHelper.FlagsToNames(ur.Flags).Select(f => $"+{f}"));
             Logger.InfoS("admin.perms", $"{Player} updated admin rank {rank.Name}/{flagText}.");
 
             _adminManager.ReloadAdminsWithRank(ur.Id);
@@ -207,7 +207,7 @@ namespace Content.Server.Administration
 
             await _db.AddAdminRankAsync(rank);
 
-            var flagText = string.Join(' ', AdminFlagsExt.FlagsToNames(ar.Flags).Select(f => $"+{f}"));
+            var flagText = string.Join(' ', AdminFlagsHelper.FlagsToNames(ar.Flags).Select(f => $"+{f}"));
             Logger.InfoS("admin.perms", $"{Player} added admin rank {rank.Name}/{flagText}.");
         }
 
@@ -272,7 +272,7 @@ namespace Content.Server.Administration
 
             var name = playerRecord?.LastSeenUserName ?? ua.UserId.ToString();
             var title = ua.Title ?? "<no title>";
-            var flags = AdminFlagsExt.PosNegFlagsText(ua.PosFlags, ua.NegFlags);
+            var flags = AdminFlagsHelper.PosNegFlagsText(ua.PosFlags, ua.NegFlags);
 
             Logger.InfoS("admin.perms", $"{Player} updated admin {name} to {title}/{rankName}/{flags}");
 
@@ -347,7 +347,7 @@ namespace Content.Server.Administration
             await _db.AddAdminAsync(admin);
 
             var title = ca.Title ?? "<no title>";
-            var flags = AdminFlagsExt.PosNegFlagsText(ca.PosFlags, ca.NegFlags);
+            var flags = AdminFlagsHelper.PosNegFlagsText(ca.PosFlags, ca.NegFlags);
 
             Logger.InfoS("admin.perms", $"{Player} added admin {name} as {title}/{rankName}/{flags}");
 
@@ -392,7 +392,7 @@ namespace Content.Server.Administration
 
                 ret = rank.Name;
 
-                var rankFlags = AdminFlagsExt.NamesToFlags(rank.Flags.Select(p => p.Flag));
+                var rankFlags = AdminFlagsHelper.NamesToFlags(rank.Flags.Select(p => p.Flag));
                 if (!UserAdminFlagCheck(rankFlags))
                 {
                     // Can't assign a rank with flags you don't have yourself.
@@ -421,8 +421,8 @@ namespace Content.Server.Administration
 
         private static List<AdminFlag> GenAdminFlagList(AdminFlags posFlags, AdminFlags negFlags)
         {
-            var posFlagList = AdminFlagsExt.FlagsToNames(posFlags);
+            var posFlagList = AdminFlagsHelper.FlagsToNames(posFlags);
-            var negFlagList = AdminFlagsExt.FlagsToNames(negFlags);
+            var negFlagList = AdminFlagsHelper.FlagsToNames(negFlags);
 
             return posFlagList
                 .Select(f => new AdminFlag {Negative = false, Flag = f})
@@ -432,7 +432,7 @@ namespace Content.Server.Administration
 
         private static List<AdminRankFlag> GenRankFlagList(AdminFlags flags)
         {
-            return AdminFlagsExt.FlagsToNames(flags).Select(f => new AdminRankFlag {Flag = f}).ToList();
+            return AdminFlagsHelper.FlagsToNames(flags).Select(f => new AdminRankFlag {Flag = f}).ToList();
         }
 
         private bool UserAdminFlagCheck(AdminFlags flags)
@@ -442,8 +442,8 @@ namespace Content.Server.Administration
 
         private bool CanTouchAdmin(Admin admin)
         {
-            var posFlags = AdminFlagsExt.NamesToFlags(admin.Flags.Where(f => !f.Negative).Select(f => f.Flag));
+            var posFlags = AdminFlagsHelper.NamesToFlags(admin.Flags.Where(f => !f.Negative).Select(f => f.Flag));
-            var rankFlags = AdminFlagsExt.NamesToFlags(
+            var rankFlags = AdminFlagsHelper.NamesToFlags(
                 admin.AdminRank?.Flags.Select(f => f.Flag) ?? Array.Empty<string>());
 
             var totalFlags = posFlags | rankFlags;
@@ -452,7 +452,7 @@ namespace Content.Server.Administration
 
         private bool CanTouchRank(DbAdminRank rank)
         {
-            var rankFlags = AdminFlagsExt.NamesToFlags(rank.Flags.Select(f => f.Flag));
+            var rankFlags = AdminFlagsHelper.NamesToFlags(rank.Flags.Select(f => f.Flag));
 
             return UserAdminFlagCheck(rankFlags);
         }

Content.Server/Chat/ChatManager.cs

@@ -214,7 +214,6 @@ namespace Content.Server.Chat
             var clients = _playerManager
                 .GetPlayersBy(x => x.AttachedEntity != null && x.AttachedEntity.HasComponent<GhostComponent>())
                 .Select(p => p.ConnectedClient);
-            ;
 
             var msg = _netManager.CreateNetMessage<MsgChatMessage>();
             msg.Channel = ChatChannel.Dead;

Content.Shared/Administration/AdminData.cs

@@ -2,35 +2,64 @@
 
 namespace Content.Shared.Administration
 {
+    /// <summary>
+    ///     Represents data for a single server admin.
+    /// </summary>
     public sealed class AdminData
     {
-        public const string DefaultTitle = "Admin";
-
         // Can be false if they're de-adminned with the ability to re-admin.
+        /// <summary>
+        ///     Whether the admin is currently active. This can be false if they have de-adminned mid-round.
+        /// </summary>
         public bool Active;
+
+        /// <summary>
+        ///     The admin's title.
+        /// </summary>
         public string? Title;
+
+        /// <summary>
+        ///     The admin's permission flags.
+        /// </summary>
         public AdminFlags Flags;
 
+        /// <summary>
+        ///     Checks whether this admin has an admin flag.
+        /// </summary>
+        /// <param name="flag">The flags to check. Multiple flags can be specified, they must all be held.</param>
+        /// <returns>False if this admin is not <see cref="Active"/> or does not have all the flags specified.</returns>
         public bool HasFlag(AdminFlags flag)
         {
             return Active && (Flags & flag) == flag;
         }
 
+        /// <summary>
+        ///     Check if this admin can open the VV menu.
+        /// </summary>
         public bool CanViewVar()
         {
             return HasFlag(AdminFlags.VarEdit);
         }
 
+        /// <summary>
+        ///     Check if this admin can spawn stuff in with the entity/tile spawn panel.
+        /// </summary>
         public bool CanAdminPlace()
         {
             return HasFlag(AdminFlags.Spawn);
         }
 
+        /// <summary>
+        ///     Check if this admin can execute server-side C# scripts.
+        /// </summary>
         public bool CanScript()
         {
             return HasFlag(AdminFlags.Host);
         }
 
+        /// <summary>
+        ///     Check if this admin can open the admin menu.
+        /// </summary>
         public bool CanAdminMenu()
         {
             return HasFlag(AdminFlags.Admin);

Content.Shared/Administration/AdminFlagsHelper.cs

@@ -5,16 +5,28 @@ using System.Numerics;
 
 namespace Content.Shared.Administration
 {
-    public static class AdminFlagsExt
+    /// <summary>
+    ///     Contains various helper methods for working with admin flags.
+    /// </summary>
+    public static class AdminFlagsHelper
     {
+        // As you can tell from the boatload of bitwise ops,
+        // writing this class was genuinely fun.
+
         private static readonly Dictionary<string, AdminFlags> NameFlagsMap = new Dictionary<string, AdminFlags>();
         private static readonly string[] FlagsNameMap = new string[32];
 
+        /// <summary>
+        ///     Every admin flag in the game, at once!
+        /// </summary>
         public static readonly AdminFlags Everything;
 
+        /// <summary>
+        ///     A list of all individual admin flags.
+        /// </summary>
         public static readonly IReadOnlyList<AdminFlags> AllFlags;
 
-        static AdminFlagsExt()
+        static AdminFlagsHelper()
         {
             var t = typeof(AdminFlags);
             var flags = (AdminFlags[]) Enum.GetValues(t);
@@ -24,6 +36,8 @@ namespace Content.Shared.Administration
             {
                 var name = value.ToString().ToUpper();
 
+                // If, in the future, somebody adds a combined admin flag or something for convenience,
+                // ignore it.
                 if (BitOperations.PopCount((uint) value) != 1)
                 {
                     continue;
@@ -38,6 +52,15 @@ namespace Content.Shared.Administration
             AllFlags = allFlags.ToArray();
         }
 
+        /// <summary>
+        ///     Converts an enumerable of admin flag names to a bitfield.
+        /// </summary>
+        /// <remarks>
+        ///     The flags must all be uppercase.
+        /// </remarks>
+        /// <exception cref="ArgumentException">
+        ///     Thrown if a string that is not a valid admin flag is contained in <paramref name="names"/>.
+        /// </exception>
         public static AdminFlags NamesToFlags(IEnumerable<string> names)
         {
             var flags = AdminFlags.None;
@@ -54,11 +77,23 @@ namespace Content.Shared.Administration
             return flags;
         }
 
+        /// <summary>
+        ///     Gets the flag bit for an admin flag name.
+        /// </summary>
+        /// <remarks>
+        ///     The flag name must be all uppercase.
+        /// </remarks>
+        /// <exception cref="KeyNotFoundException">
+        ///     Thrown if <paramref name="name"/> is not a valid admin flag name.
+        /// </exception>
         public static AdminFlags NameToFlag(string name)
         {
             return NameFlagsMap[name];
         }
 
+        /// <summary>
+        ///     Converts a bitfield of admin flags to an array of all the flag names set.
+        /// </summary>
         public static string[] FlagsToNames(AdminFlags flags)
         {
             var array = new string[BitOperations.PopCount((uint) flags)];

Content.Tests/Shared/Administration/AdminFlagsExtTest.cs

@@ -17,7 +17,7 @@ namespace Content.Tests.Shared.Administration
         {
             var names = namesConcat.Split(",", StringSplitOptions.RemoveEmptyEntries);
 
-            Assert.That(AdminFlagsExt.NamesToFlags(names), Is.EqualTo(flags));
+            Assert.That(AdminFlagsHelper.NamesToFlags(names), Is.EqualTo(flags));
         }
 
         [Test]
@@ -29,7 +29,7 @@ namespace Content.Tests.Shared.Administration
         {
             var names = namesConcat.Split(",", StringSplitOptions.RemoveEmptyEntries);
 
-            Assert.That(AdminFlagsExt.FlagsToNames(flags), Is.EquivalentTo(names));
+            Assert.That(AdminFlagsHelper.FlagsToNames(flags), Is.EquivalentTo(names));
         }
     }
 }