From 97f6097dad149d819bc4db918759d71cb6de9b04 Mon Sep 17 00:00:00 2001 From: Leon Friedrich <60421075+ElectroJr@users.noreply.github.com> Date: Mon, 14 Oct 2024 17:13:35 +1300 Subject: [PATCH] Add `IsQueuedForDeletion` checks to interaction system (#32526) --- .../Events/ContactInteractionEvent.cs | 2 +- .../Events/InteractionFailureEvent.cs | 2 + .../Events/InteractionSuccessEvent.cs | 2 + .../Interaction/SharedInteractionSystem.cs | 115 ++++++++++++++---- 4 files changed, 95 insertions(+), 26 deletions(-) diff --git a/Content.Shared/Interaction/Events/ContactInteractionEvent.cs b/Content.Shared/Interaction/Events/ContactInteractionEvent.cs index c9d5fba2ed..7be1c01c4a 100644 --- a/Content.Shared/Interaction/Events/ContactInteractionEvent.cs +++ b/Content.Shared/Interaction/Events/ContactInteractionEvent.cs @@ -8,7 +8,7 @@ namespace Content.Shared.Interaction.Events; /// public sealed class ContactInteractionEvent : HandledEntityEventArgs { - public readonly EntityUid Other; + public EntityUid Other; public ContactInteractionEvent(EntityUid other) { diff --git a/Content.Shared/Interaction/Events/InteractionFailureEvent.cs b/Content.Shared/Interaction/Events/InteractionFailureEvent.cs index a820048104..8670164293 100644 --- a/Content.Shared/Interaction/Events/InteractionFailureEvent.cs +++ b/Content.Shared/Interaction/Events/InteractionFailureEvent.cs @@ -3,5 +3,7 @@ namespace Content.Shared.Interaction.Events; /// /// Raised on the target when failing to pet/hug something. /// +// TODO INTERACTION +// Rename this, or move it to another namespace to make it clearer that this is specific to "petting/hugging" (InteractionPopupSystem) [ByRefEvent] public readonly record struct InteractionFailureEvent(EntityUid User); diff --git a/Content.Shared/Interaction/Events/InteractionSuccessEvent.cs b/Content.Shared/Interaction/Events/InteractionSuccessEvent.cs index da4f9e43d7..9395ddc910 100644 --- a/Content.Shared/Interaction/Events/InteractionSuccessEvent.cs +++ b/Content.Shared/Interaction/Events/InteractionSuccessEvent.cs @@ -3,5 +3,7 @@ namespace Content.Shared.Interaction.Events; /// /// Raised on the target when successfully petting/hugging something. /// +// TODO INTERACTION +// Rename this, or move it to another namespace to make it clearer that this is specific to "petting/hugging" (InteractionPopupSystem) [ByRefEvent] public readonly record struct InteractionSuccessEvent(EntityUid User); diff --git a/Content.Shared/Interaction/SharedInteractionSystem.cs b/Content.Shared/Interaction/SharedInteractionSystem.cs index ddbf3ee572..6f44d3089d 100644 --- a/Content.Shared/Interaction/SharedInteractionSystem.cs +++ b/Content.Shared/Interaction/SharedInteractionSystem.cs @@ -456,8 +456,22 @@ namespace Content.Shared.Interaction inRangeUnobstructed); } + private bool IsDeleted(EntityUid uid) + { + return TerminatingOrDeleted(uid) || EntityManager.IsQueuedForDeletion(uid); + } + + private bool IsDeleted(EntityUid? uid) + { + //optional / null entities can pass this validation check. I.e., is-deleted returns false for null uids + return uid != null && IsDeleted(uid.Value); + } + public void InteractHand(EntityUid user, EntityUid target) { + if (IsDeleted(user) || IsDeleted(target)) + return; + var complexInteractions = _actionBlockerSystem.CanComplexInteract(user); if (!complexInteractions) { @@ -466,7 +480,8 @@ namespace Content.Shared.Interaction checkCanInteract: false, checkUseDelay: true, checkAccess: false, - complexInteractions: complexInteractions); + complexInteractions: complexInteractions, + checkDeletion: false); return; } @@ -479,6 +494,7 @@ namespace Content.Shared.Interaction return; } + DebugTools.Assert(!IsDeleted(user) && !IsDeleted(target)); // all interactions should only happen when in range / unobstructed, so no range check is needed var message = new InteractHandEvent(user, target); RaiseLocalEvent(target, message, true); @@ -487,18 +503,23 @@ namespace Content.Shared.Interaction if (message.Handled) return; + DebugTools.Assert(!IsDeleted(user) && !IsDeleted(target)); // Else we run Activate. InteractionActivate(user, target, checkCanInteract: false, checkUseDelay: true, checkAccess: false, - complexInteractions: complexInteractions); + complexInteractions: complexInteractions, + checkDeletion: false); } public void InteractUsingRanged(EntityUid user, EntityUid used, EntityUid? target, EntityCoordinates clickLocation, bool inRangeUnobstructed) { + if (IsDeleted(user) || IsDeleted(used) || IsDeleted(target)) + return; + if (target != null) { _adminLogger.Add( @@ -514,9 +535,10 @@ namespace Content.Shared.Interaction $"{ToPrettyString(user):user} interacted with *nothing* using {ToPrettyString(used):used}"); } - if (RangedInteractDoBefore(user, used, target, clickLocation, inRangeUnobstructed)) + if (RangedInteractDoBefore(user, used, target, clickLocation, inRangeUnobstructed, checkDeletion: false)) return; + DebugTools.Assert(!IsDeleted(user) && !IsDeleted(used) && !IsDeleted(target)); if (target != null) { var rangedMsg = new RangedInteractEvent(user, used, target.Value, clickLocation); @@ -524,12 +546,12 @@ namespace Content.Shared.Interaction // We contact the USED entity, but not the target. DoContactInteraction(user, used, rangedMsg); - if (rangedMsg.Handled) return; } - InteractDoAfter(user, used, target, clickLocation, inRangeUnobstructed); + DebugTools.Assert(!IsDeleted(user) && !IsDeleted(used) && !IsDeleted(target)); + InteractDoAfter(user, used, target, clickLocation, inRangeUnobstructed, checkDeletion: false); } protected bool ValidateInteractAndFace(EntityUid user, EntityCoordinates coordinates) @@ -933,11 +955,18 @@ namespace Content.Shared.Interaction EntityUid used, EntityUid? target, EntityCoordinates clickLocation, - bool canReach) + bool canReach, + bool checkDeletion = true) { + if (checkDeletion && (IsDeleted(user) || IsDeleted(used) || IsDeleted(target))) + return false; + var ev = new BeforeRangedInteractEvent(user, used, target, clickLocation, canReach); RaiseLocalEvent(used, ev); + if (!ev.Handled) + return false; + // We contact the USED entity, but not the target. DoContactInteraction(user, used, ev); return ev.Handled; @@ -966,6 +995,9 @@ namespace Content.Shared.Interaction bool checkCanInteract = true, bool checkCanUse = true) { + if (IsDeleted(user) || IsDeleted(used) || IsDeleted(target)) + return false; + if (checkCanInteract && !_actionBlockerSystem.CanInteract(user, target)) return false; @@ -977,9 +1009,10 @@ namespace Content.Shared.Interaction LogImpact.Low, $"{ToPrettyString(user):user} interacted with {ToPrettyString(target):target} using {ToPrettyString(used):used}"); - if (RangedInteractDoBefore(user, used, target, clickLocation, true)) + if (RangedInteractDoBefore(user, used, target, clickLocation, canReach: true, checkDeletion: false)) return true; + DebugTools.Assert(!IsDeleted(user) && !IsDeleted(used) && !IsDeleted(target)); // all interactions should only happen when in range / unobstructed, so no range check is needed var interactUsingEvent = new InteractUsingEvent(user, used, target, clickLocation); RaiseLocalEvent(target, interactUsingEvent, true); @@ -989,8 +1022,10 @@ namespace Content.Shared.Interaction if (interactUsingEvent.Handled) return true; - if (InteractDoAfter(user, used, target, clickLocation, canReach: true)) + if (InteractDoAfter(user, used, target, clickLocation, canReach: true, checkDeletion: false)) return true; + + DebugTools.Assert(!IsDeleted(user) && !IsDeleted(used) && !IsDeleted(target)); return false; } @@ -1004,11 +1039,14 @@ namespace Content.Shared.Interaction /// Whether the is in range of the . /// /// True if the interaction was handled. Otherwise, false. - public bool InteractDoAfter(EntityUid user, EntityUid used, EntityUid? target, EntityCoordinates clickLocation, bool canReach) + public bool InteractDoAfter(EntityUid user, EntityUid used, EntityUid? target, EntityCoordinates clickLocation, bool canReach, bool checkDeletion = true) { if (target is { Valid: false }) target = null; + if (checkDeletion && (IsDeleted(user) || IsDeleted(used) || IsDeleted(target))) + return false; + var afterInteractEvent = new AfterInteractEvent(user, used, target, clickLocation, canReach); RaiseLocalEvent(used, afterInteractEvent); DoContactInteraction(user, used, afterInteractEvent); @@ -1024,6 +1062,7 @@ namespace Content.Shared.Interaction if (target == null) return false; + DebugTools.Assert(!IsDeleted(user) && !IsDeleted(used) && !IsDeleted(target)); var afterInteractUsingEvent = new AfterInteractUsingEvent(user, used, target, clickLocation, canReach); RaiseLocalEvent(target.Value, afterInteractUsingEvent); @@ -1034,9 +1073,7 @@ namespace Content.Shared.Interaction // Contact interactions are currently only used for forensics, so we don't raise used -> target } - if (afterInteractUsingEvent.Handled) - return true; - return false; + return afterInteractUsingEvent.Handled; } #region ActivateItemInWorld @@ -1068,8 +1105,13 @@ namespace Content.Shared.Interaction bool checkCanInteract = true, bool checkUseDelay = true, bool checkAccess = true, - bool? complexInteractions = null) + bool? complexInteractions = null, + bool checkDeletion = true) { + if (checkDeletion && (IsDeleted(user) || IsDeleted(used))) + return false; + + DebugTools.Assert(!IsDeleted(user) && !IsDeleted(used)); _delayQuery.TryComp(used, out var delayComponent); if (checkUseDelay && delayComponent != null && _useDelay.IsDelayed((used, delayComponent))) return false; @@ -1085,21 +1127,32 @@ namespace Content.Shared.Interaction if (checkAccess && !IsAccessible(user, used)) return false; - complexInteractions ??= SupportsComplexInteractions(user); + complexInteractions ??= _actionBlockerSystem.CanComplexInteract(user); var activateMsg = new ActivateInWorldEvent(user, used, complexInteractions.Value); RaiseLocalEvent(used, activateMsg, true); + if (activateMsg.Handled) + { + DoContactInteraction(user, used); + if (!activateMsg.WasLogged) + _adminLogger.Add(LogType.InteractActivate, LogImpact.Low, $"{ToPrettyString(user):user} activated {ToPrettyString(used):used}"); + + if (delayComponent != null) + _useDelay.TryResetDelay(used, component: delayComponent); + return true; + } + + DebugTools.Assert(!IsDeleted(user) && !IsDeleted(used)); var userEv = new UserActivateInWorldEvent(user, used, complexInteractions.Value); RaiseLocalEvent(user, userEv, true); - if (!activateMsg.Handled && !userEv.Handled) + if (!userEv.Handled) return false; - DoContactInteraction(user, used, activateMsg); + DoContactInteraction(user, used); // Still need to call this even without checkUseDelay in case this gets relayed from Activate. if (delayComponent != null) _useDelay.TryResetDelay(used, component: delayComponent); - if (!activateMsg.WasLogged) - _adminLogger.Add(LogType.InteractActivate, LogImpact.Low, $"{ToPrettyString(user):user} activated {ToPrettyString(used):used}"); + _adminLogger.Add(LogType.InteractActivate, LogImpact.Low, $"{ToPrettyString(user):user} activated {ToPrettyString(used):used}"); return true; } #endregion @@ -1118,6 +1171,9 @@ namespace Content.Shared.Interaction bool checkCanInteract = true, bool checkUseDelay = true) { + if (IsDeleted(user) || IsDeleted(used)) + return false; + _delayQuery.TryComp(used, out var delayComponent); if (checkUseDelay && delayComponent != null && _useDelay.IsDelayed((used, delayComponent))) return true; // if the item is on cooldown, we consider this handled. @@ -1138,8 +1194,9 @@ namespace Content.Shared.Interaction return true; } + DebugTools.Assert(!IsDeleted(user) && !IsDeleted(used)); // else, default to activating the item - return InteractionActivate(user, used, false, false, false); + return InteractionActivate(user, used, false, false, false, checkDeletion: false); } /// @@ -1164,6 +1221,9 @@ namespace Content.Shared.Interaction public void DroppedInteraction(EntityUid user, EntityUid item) { + if (IsDeleted(user) || IsDeleted(item)) + return; + var dropMsg = new DroppedEvent(user); RaiseLocalEvent(item, dropMsg, true); @@ -1312,15 +1372,20 @@ namespace Content.Shared.Interaction if (uidB == null || args?.Handled == false) return; - // Entities may no longer exist (banana was eaten, or human was exploded)? - if (!Exists(uidA) || !Exists(uidB)) + DebugTools.AssertNotEqual(uidA, uidB.Value); + + if (!TryComp(uidA, out MetaDataComponent? metaA) || metaA.EntityPaused) return; - if (Paused(uidA) || Paused(uidB.Value)) - return; + if (!TryComp(uidB, out MetaDataComponent? metaB) || metaB.EntityPaused) + return ; - RaiseLocalEvent(uidA, new ContactInteractionEvent(uidB.Value)); - RaiseLocalEvent(uidB.Value, new ContactInteractionEvent(uidA)); + // TODO Struct event + var ev = new ContactInteractionEvent(uidB.Value); + RaiseLocalEvent(uidA, ev); + + ev.Other = uidA; + RaiseLocalEvent(uidB.Value, ev); }