Fix admin verb PVS issue (space-wizards#21406)

Author: ElectroJr

Content.Client/Administration/UI/CustomControls/PlayerListControl.xaml.cs

@@ -60,7 +60,7 @@ private void PlayerListItemPressed(BaseButton.ButtonEventArgs? args, ListData? d
             }
             else if (args.Event.Function == EngineKeyFunctions.UseSecondary && selectedPlayer.NetEntity != null)
             {
-                _uiManager.GetUIController<VerbMenuUIController>().OpenVerbMenu(_entManager.GetEntity(selectedPlayer.NetEntity.Value));
+                _uiManager.GetUIController<VerbMenuUIController>().OpenVerbMenu(selectedPlayer.NetEntity.Value, true);
             }
         }
 

Content.Client/Administration/UI/Tabs/PlayerTab/PlayerTab.xaml.cs

@@ -14,11 +14,13 @@ namespace Content.Client.Administration.UI.Tabs.PlayerTab
     [GenerateTypedNameReferences]
     public sealed partial class PlayerTab : Control
     {
+        [Dependency] private readonly IEntityManager _entManager = default!;
+        [Dependency] private readonly IPlayerManager _playerMan = default!;
+
         private const string ArrowUp = "↑";
         private const string ArrowDown = "↓";
         private readonly Color _altColor = Color.FromHex("#292B38");
         private readonly Color _defaultColor = Color.FromHex("#2F2F3B");
-        private IEntityManager _entManager;
         private readonly AdminSystem _adminSystem;
         private IReadOnlyList<PlayerInfo> _players = new List<PlayerInfo>();
 
@@ -30,7 +32,7 @@ public sealed partial class PlayerTab : Control
 
         public PlayerTab()
         {
-            _entManager = IoCManager.Resolve<IEntityManager>();
+            IoCManager.InjectDependencies(this);
             _adminSystem = _entManager.System<AdminSystem>();
             RobustXamlLoader.Load(this);
             RefreshPlayerList(_adminSystem.PlayerList);
@@ -95,13 +97,11 @@ private void RefreshPlayerList(IReadOnlyList<PlayerInfo> players)
             foreach (var child in PlayerList.Children.ToArray())
             {
                 if (child is PlayerTabEntry)
-                    child.Orphan();
+                    child.Dispose();
             }
 
             _players = players;
-
-            var playerManager = IoCManager.Resolve<IPlayerManager>();
-            PlayerCount.Text = $"Players: {playerManager.PlayerCount}";
+            PlayerCount.Text = $"Players: {_playerMan.PlayerCount}";
 
             var sortedPlayers = new List<PlayerInfo>(players);
             sortedPlayers.Sort(Compare);

Content.Client/UserInterface/Systems/Admin/AdminUIController.cs

@@ -187,7 +187,7 @@ private void PlayerTabEntryPressed(ButtonEventArgs args)
         if (function == EngineKeyFunctions.UIClick)
             _conHost.ExecuteCommand($"vv {entity}");
         else if (function == EngineKeyFunctions.UseSecondary)
-            _verb.OpenVerbMenu(EntityManager.GetEntity(entity), true);
+            _verb.OpenVerbMenu(entity, true);
         else
             return;
 

Content.Client/Verbs/UI/VerbMenuUIController.cs

@@ -9,6 +9,7 @@
 using Robust.Client.UserInterface;
 using Robust.Client.UserInterface.Controllers;
 using Robust.Shared.Input;
+using Robust.Shared.Utility;
 
 namespace Content.Client.Verbs.UI
 {
@@ -29,7 +30,7 @@ public sealed class VerbMenuUIController : UIController, IOnStateEntered<Gamepla
         [UISystemDependency] private readonly CombatModeSystem _combatMode = default!;
         [UISystemDependency] private readonly VerbSystem _verbSystem = default!;
 
-        public EntityUid CurrentTarget;
+        public NetEntity CurrentTarget;
         public SortedSet<Verb> CurrentVerbs = new();
 
         /// <summary>
@@ -64,8 +65,25 @@ public void OnStateExited(GameplayState state)
         /// </param>
         public void OpenVerbMenu(EntityUid target, bool force = false, ContextMenuPopup? popup=null)
         {
-            if (_playerManager.LocalPlayer?.ControlledEntity is not {Valid: true} user ||
-                _combatMode.IsInCombatMode(user))
+            DebugTools.Assert(target.IsValid());
+            OpenVerbMenu(EntityManager.GetNetEntity(target), force, popup);
+        }
+
+        /// <summary>
+        ///     Open a verb menu and fill it with verbs applicable to the given target entity.
+        /// </summary>
+        /// <param name="target">Entity to get verbs on.</param>
+        /// <param name="force">Used to force showing all verbs. Only works on networked entities if the user is an admin.</param>
+        /// <param name="popup">
+        ///     If this is not null, verbs will be placed into the given popup instead.
+        /// </param>
+        public void OpenVerbMenu(NetEntity target, bool force = false, ContextMenuPopup? popup=null)
+        {
+            DebugTools.Assert(target.IsValid());
+            if (_playerManager.LocalEntity is not {Valid: true} user)
+                return;
+
+            if (!force && _combatMode.IsInCombatMode(user))
                 return;
 
             Close();
@@ -82,7 +100,7 @@ public void OpenVerbMenu(EntityUid target, bool force = false, ContextMenuPopup?
 
             // Add indicator that some verbs may be missing.
             // I long for the day when verbs will all be predicted and this becomes unnecessary.
-            if (!EntityManager.IsClientSide(target))
+            if (!target.IsClientSide())
             {
                 _context.AddElement(menu, new ContextMenuElement(Loc.GetString("verb-system-waiting-on-server-text")));
             }
@@ -248,7 +266,7 @@ private void Close()
 
         private void HandleVerbsResponse(VerbsResponseEvent msg)
         {
-            if (OpenMenu == null || !OpenMenu.Visible || CurrentTarget != EntityManager.GetEntity(msg.Entity))
+            if (OpenMenu == null || !OpenMenu.Visible || CurrentTarget != msg.Entity)
                 return;
 
             AddServerVerbs(msg.Verbs, OpenMenu);

Content.Client/Verbs/VerbSystem.cs

@@ -170,28 +170,27 @@ public bool TryGetEntityMenuEntities(MapCoordinates targetPos, [NotNullWhen(true
         /// </summary>
         public SortedSet<Verb> GetVerbs(EntityUid target, EntityUid user, Type type, bool force = false)
         {
-            return GetVerbs(target, user, new List<Type>() { type }, force);
+            return GetVerbs(GetNetEntity(target), user, new List<Type>() { type }, force);
         }
 
         /// <summary>
         ///     Ask the server to send back a list of server-side verbs, and for now return an incomplete list of verbs
         ///     (only those defined locally).
         /// </summary>
-        public SortedSet<Verb> GetVerbs(EntityUid target, EntityUid user, List<Type> verbTypes,
+        public SortedSet<Verb> GetVerbs(NetEntity target, EntityUid user, List<Type> verbTypes,
             bool force = false)
         {
-            if (!IsClientSide(target))
-            {
-                RaiseNetworkEvent(new RequestServerVerbsEvent(GetNetEntity(target), verbTypes, adminRequest: force));
-            }
+            if (!target.IsClientSide())
+                RaiseNetworkEvent(new RequestServerVerbsEvent(target, verbTypes, adminRequest: force));
 
             // Some admin menu interactions will try get verbs for entities that have not yet been sent to the player.
-            if (!Exists(target))
+            if (!TryGetEntity(target, out var local))
                 return new();
 
-            return GetLocalVerbs(target, user, verbTypes, force);
+            return GetLocalVerbs(local.Value, user, verbTypes, force);
         }
 
+
         /// <summary>
         ///     Execute actions associated with the given verb.
         /// </summary>
@@ -200,25 +199,35 @@ public SortedSet<Verb> GetVerbs(EntityUid target, EntityUid user, List<Type> ver
         /// </remarks>
         public void ExecuteVerb(EntityUid target, Verb verb)
         {
-            var user = _playerManager.LocalPlayer?.ControlledEntity;
-            if (user == null)
+            ExecuteVerb(GetNetEntity(target), verb);
+        }
+
+        /// <summary>
+        ///     Execute actions associated with the given verb.
+        /// </summary>
+        /// <remarks>
+        ///     Unless this is a client-exclusive verb, this will also tell the server to run the same verb.
+        /// </remarks>
+        public void ExecuteVerb(NetEntity target, Verb verb)
+        {
+            if ( _playerManager.LocalEntity is not {} user)
                 return;
 
             // is this verb actually valid?
             if (verb.Disabled)
             {
                 // maybe send an informative pop-up message.
                 if (!string.IsNullOrWhiteSpace(verb.Message))
-                    _popupSystem.PopupEntity(verb.Message, user.Value);
+                    _popupSystem.PopupEntity(verb.Message, user);
 
                 return;
             }
 
-            if (verb.ClientExclusive || IsClientSide(target))
+            if (verb.ClientExclusive || target.IsClientSide())
                 // is this a client exclusive (gui) verb?
-                ExecuteVerb(verb, user.Value, target);
+                ExecuteVerb(verb, user, GetEntity(target));
             else
-                EntityManager.RaisePredictiveEvent(new ExecuteVerbEvent(GetNetEntity(target), verb));
+                EntityManager.RaisePredictiveEvent(new ExecuteVerbEvent(target, verb));
         }
 
         private void HandleVerbResponse(VerbsResponseEvent msg)

Content.Shared/Verbs/SharedVerbSystem.cs

@@ -24,24 +24,25 @@ private void HandleExecuteVerb(ExecuteVerbEvent args, EntitySessionEventArgs eve
             if (user == null)
                 return;
 
-            var target = GetEntity(args.Target);
+            if (!TryGetEntity(args.Target, out var target))
+                return;
 
             // It is possible that client-side prediction can cause this event to be raised after the target entity has
             // been deleted. So we need to check that the entity still exists.
-            if (Deleted(target) || Deleted(user))
+            if (Deleted(user))
                 return;
 
             // Get the list of verbs. This effectively also checks that the requested verb is in fact a valid verb that
             // the user can perform.
-            var verbs = GetLocalVerbs(target, user.Value, args.RequestedVerb.GetType());
+            var verbs = GetLocalVerbs(target.Value, user.Value, args.RequestedVerb.GetType());
 
             // Note that GetLocalVerbs might waste time checking & preparing unrelated verbs even though we know
             // precisely which one we want to run. However, MOST entities will only have 1 or 2 verbs of a given type.
             // The one exception here is the "other" verb type, which has 3-4 verbs + all the debug verbs.
 
             // Find the requested verb.
             if (verbs.TryGetValue(args.RequestedVerb, out var verb))
-                ExecuteVerb(verb, user.Value, target);
+                ExecuteVerb(verb, user.Value, target.Value);
         }
 
         /// <summary>