diff --git a/Content.Server/GameObjects/Components/Pulling/PullableComponent.cs b/Content.Server/GameObjects/Components/Pulling/PullableComponent.cs index 9aca41096a..5d4668718f 100644 --- a/Content.Server/GameObjects/Components/Pulling/PullableComponent.cs +++ b/Content.Server/GameObjects/Components/Pulling/PullableComponent.cs @@ -41,26 +41,17 @@ namespace Content.Server.GameObjects.Components.Pulling return; } - var controller = targetPhysics.EnsureController(); - data.Visibility = VerbVisibility.Visible; - data.Text = controller.Puller == userPhysics + data.Text = component.Puller == userPhysics ? Loc.GetString("Stop pulling") : Loc.GetString("Pull"); } protected override void Activate(IEntity user, PullableComponent component) { - if (!user.TryGetComponent(out IPhysicsComponent? userCollidable) || - !component.Owner.TryGetComponent(out IPhysicsComponent? targetCollidable) || - targetCollidable.Anchored) - { - return; - } - - var controller = targetCollidable.EnsureController(); - - if (controller.Puller == userCollidable) + // There used to be sanity checks here for no reason. + // Why no reason? Because they're supposed to be performed in TryStartPull. + if (component.Puller == user) { component.TryStopPull(); } diff --git a/Content.Shared/GameObjects/Components/Pulling/SharedPullableComponent.cs b/Content.Shared/GameObjects/Components/Pulling/SharedPullableComponent.cs index 4fbf16931f..49111b3fc6 100644 --- a/Content.Shared/GameObjects/Components/Pulling/SharedPullableComponent.cs +++ b/Content.Shared/GameObjects/Components/Pulling/SharedPullableComponent.cs @@ -25,46 +25,139 @@ namespace Content.Shared.GameObjects.Components.Pulling [ComponentDependency] private readonly IPhysicsComponent? _physics = default!; + /// + /// Only set in Puller->set! Only set in unison with _pullerPhysics! + /// private IEntity? _puller; + private IPhysicsComponent? _pullerPhysics; + public IPhysicsComponent? PullerPhysics => _pullerPhysics; + /// + /// The current entity pulling this component. + /// Setting this performs the entire setup process for pulling. + /// public virtual IEntity? Puller { get => _puller; - private set + set { if (_puller == value) { return; } - _puller = value; - Dirty(); + // New value. Abandon being pulled by any existing object. + if (_puller != null) + { + var oldPuller = _puller; + var oldPullerPhysics = _pullerPhysics; + _puller = null; + _pullerPhysics = null; + + if (_physics != null) + { + var message = new PullStoppedMessage(oldPullerPhysics, _physics); + + oldPuller.SendMessage(null, message); + Owner.SendMessage(null, message); + + oldPuller.EntityManager.EventBus.RaiseEvent(EventSource.Local, message); + _physics.WakeBody(); + _physics.TryRemoveController(); + } + // else-branch warning is handled below + } + + // Now that is settled, prepare to be pulled by a new object. if (_physics == null) { + Logger.WarningS("c.go.c.pulling", "Well now you've done it, haven't you? SharedPullableComponent on {0} didn't have an IPhysicsComponent.", Owner); return; } - PullController controller; - - if (value == null) + if (value != null) { - if (_physics.TryGetController(out controller)) + // Pulling a new object : Perform sanity checks. + + if (!_canStartPull(value)) { - controller.StopPull(); + return; } - return; - } + if (!value.TryGetComponent(out var valuePhysics)) + { + return; + } - controller = _physics.EnsureController(); - controller.StartPull(value); + if (!value.TryGetComponent(out var valuePuller)) + { + return; + } + + // Ensure that the puller is not currently pulling anything. + // If this isn't done, then it happens too late, and the start/stop messages go out of order, + // and next thing you know it thinks it's not pulling anything even though it is! + + var oldPulling = valuePuller.Pulling; + if (oldPulling != null) + { + if (oldPulling.TryGetComponent(out SharedPullableComponent? pullable)) + { + pullable.Puller = null; + } + else + { + Logger.WarningS("c.go.c.pulling", "Well now you've done it, haven't you? Someone transferred pulling to this component (on {0}) while presently pulling something that has no Pullable component (on {1})!", Owner, oldPulling); + return; + } + } + + // Continue with pulling process. + + var pullAttempt = new PullAttemptMessage(valuePhysics, _physics); + + value.SendMessage(null, pullAttempt); + + if (pullAttempt.Cancelled) + { + return; + } + + Owner.SendMessage(null, pullAttempt); + + if (pullAttempt.Cancelled) + { + return; + } + + // Pull start confirm + + _puller = value; + _pullerPhysics = valuePhysics; + + _physics.EnsureController().Manager = this; + var message = new PullStartedMessage(_pullerPhysics, _physics); + + _puller.SendMessage(null, message); + Owner.SendMessage(null, message); + + _puller.EntityManager.EventBus.RaiseEvent(EventSource.Local, message); + + _physics.WakeBody(); + } + // Code here will not run if pulling a new object was attempted and failed because of the returns from the refactor. } } public bool BeingPulled => Puller != null; - public bool CanStartPull(IEntity puller) + /// + /// Sanity-check pull. This is called from Puller setter, so it will never deny a pull that's valid by setting Puller. + /// It might allow an impossible pull (i.e: puller has no PhysicsComponent somehow). + /// Ultimately this is only used separately to stop TryStartPull from cancelling a pull for no reason. + /// + private bool _canStartPull(IEntity puller) { if (!puller.HasComponent()) { @@ -96,7 +189,7 @@ namespace Content.Shared.GameObjects.Components.Pulling public bool TryStartPull(IEntity puller) { - if (!CanStartPull(puller)) + if (!_canStartPull(puller)) { return false; } @@ -201,30 +294,25 @@ namespace Content.Shared.GameObjects.Components.Pulling return; } + SharedAlertsComponent? pulledStatus = Owner.GetComponentOrNull(); + switch (message) { case PullStartedMessage msg: - AddPullingStatuses(msg.Puller.Owner); + if (pulledStatus != null) + { + pulledStatus.ShowAlert(AlertType.Pulled); + } break; case PullStoppedMessage msg: - RemovePullingStatuses(msg.Puller.Owner); + if (pulledStatus != null) + { + pulledStatus.ClearAlert(AlertType.Pulled); + } break; } } - private void AddPullingStatuses(IEntity puller) - { - if (Owner.TryGetComponent(out SharedAlertsComponent? pulledStatus)) - { - pulledStatus.ShowAlert(AlertType.Pulled); - } - - if (puller.TryGetComponent(out SharedAlertsComponent? ownerStatus)) - { - ownerStatus.ShowAlert(AlertType.Pulling, onClickAlert: OnClickAlert); - } - } - private void OnClickAlert(ClickAlertEventArgs args) { EntitySystem @@ -234,19 +322,6 @@ namespace Content.Shared.GameObjects.Components.Pulling .TryStopPull(); } - private void RemovePullingStatuses(IEntity puller) - { - if (Owner.TryGetComponent(out SharedAlertsComponent? pulledStatus)) - { - pulledStatus.ClearAlert(AlertType.Pulled); - } - - if (puller.TryGetComponent(out SharedAlertsComponent? ownerStatus)) - { - ownerStatus.ClearAlert(AlertType.Pulling); - } - } - public override void OnRemove() { TryStopPull(); diff --git a/Content.Shared/GameObjects/Components/Pulling/SharedPullerComponent.cs b/Content.Shared/GameObjects/Components/Pulling/SharedPullerComponent.cs index f6355e0a42..391eeb5ccd 100644 --- a/Content.Shared/GameObjects/Components/Pulling/SharedPullerComponent.cs +++ b/Content.Shared/GameObjects/Components/Pulling/SharedPullerComponent.cs @@ -1,9 +1,14 @@ #nullable enable +using Content.Shared.Alert; +using Content.Shared.GameObjects.Components.Mobs; using Content.Shared.GameObjects.Components.Movement; +using Content.Shared.GameObjects.EntitySystems; using Content.Shared.Physics.Pull; using Robust.Shared.GameObjects; +using Robust.Shared.GameObjects.Systems; using Robust.Shared.Interfaces.GameObjects; using Component = Robust.Shared.GameObjects.Component; +using Robust.Shared.Log; namespace Content.Shared.GameObjects.Components.Pulling { @@ -58,15 +63,34 @@ namespace Content.Shared.GameObjects.Components.Pulling return; } + SharedAlertsComponent? ownerStatus = Owner.GetComponentOrNull(); + switch (message) { case PullStartedMessage msg: Pulling = msg.Pulled.Owner; + if (ownerStatus != null) + { + ownerStatus.ShowAlert(AlertType.Pulling, onClickAlert: OnClickAlert); + } break; case PullStoppedMessage _: Pulling = null; + if (ownerStatus != null) + { + ownerStatus.ClearAlert(AlertType.Pulling); + } break; } } + + private void OnClickAlert(ClickAlertEventArgs args) + { + EntitySystem + .Get() + .GetPulled(args.Player)? + .GetComponentOrNull()? + .TryStopPull(); + } } } diff --git a/Content.Shared/Physics/Pull/PullController.cs b/Content.Shared/Physics/Pull/PullController.cs index 5ad3dff9df..be093ca55e 100644 --- a/Content.Shared/Physics/Pull/PullController.cs +++ b/Content.Shared/Physics/Pull/PullController.cs @@ -11,10 +11,15 @@ using Robust.Shared.Map; using Robust.Shared.Maths; using Robust.Shared.Physics; using Robust.Shared.Utility; +using Content.Shared.GameObjects.Components.Pulling; using static Content.Shared.GameObjects.EntitySystems.SharedInteractionSystem; namespace Content.Shared.Physics.Pull { + /// + /// This is applied upon a Pullable object when that object is being pulled. + /// It lives only to serve that Pullable object. + /// public class PullController : VirtualController { [Dependency] private readonly IEntityManager _entityManager = default!; @@ -24,12 +29,16 @@ namespace Content.Shared.Physics.Pull private const float StopMoveThreshold = 0.25f; - private IPhysicsComponent? _puller; + /// + /// The managing SharedPullableComponent of this PullController. + /// MUST BE SET! If you go attaching PullControllers yourself, YOU ARE DOING IT WRONG. + /// If you get a crash based on such, then, well, see previous note. + /// This is set by the SharedPullableComponent attaching the PullController. + /// + public SharedPullableComponent Manager = default!; private EntityCoordinates? _movingTo; - public IPhysicsComponent? Puller => _puller; - public EntityCoordinates? MovingTo { get => _movingTo; @@ -47,6 +56,7 @@ namespace Content.Shared.Physics.Pull private bool PullerMovingTowardsPulled() { + var _puller = Manager.PullerPhysics; if (_puller == null) { return false; @@ -74,93 +84,9 @@ namespace Content.Shared.Physics.Pull return rayResults.Any(); } - public bool StartPull(IEntity entity) - { - DebugTools.AssertNotNull(entity); - - if (!entity.TryGetComponent(out IPhysicsComponent? physics)) - { - return false; - } - - return StartPull(physics); - } - - public bool StartPull(IPhysicsComponent puller) - { - DebugTools.AssertNotNull(puller); - - if (_puller == puller) - { - return false; - } - - if (ControlledComponent == null) - { - return false; - } - - var pullAttempt = new PullAttemptMessage(puller, ControlledComponent); - - puller.Owner.SendMessage(null, pullAttempt); - - if (pullAttempt.Cancelled) - { - return false; - } - - ControlledComponent.Owner.SendMessage(null, pullAttempt); - - if (pullAttempt.Cancelled) - { - return false; - } - - _puller = puller; - - var message = new PullStartedMessage(this, _puller, ControlledComponent); - - _puller.Owner.SendMessage(null, message); - ControlledComponent.Owner.SendMessage(null, message); - - _puller.Owner.EntityManager.EventBus.RaiseEvent(EventSource.Local, message); - - ControlledComponent.WakeBody(); - - return true; - } - - public bool StopPull() - { - var oldPuller = _puller; - - if (oldPuller == null) - { - return false; - } - - _puller = null; - - if (ControlledComponent == null) - { - return false; - } - - var message = new PullStoppedMessage(oldPuller, ControlledComponent); - - oldPuller.Owner.SendMessage(null, message); - ControlledComponent.Owner.SendMessage(null, message); - - oldPuller.Owner.EntityManager.EventBus.RaiseEvent(EventSource.Local, message); - - ControlledComponent.WakeBody(); - ControlledComponent.TryRemoveController(); - - return true; - } - public bool TryMoveTo(EntityCoordinates from, EntityCoordinates to) { + var _puller = Manager.PullerPhysics; if (_puller == null || ControlledComponent == null) { return false; @@ -199,6 +125,7 @@ namespace Content.Shared.Physics.Pull public override void UpdateBeforeProcessing() { + var _puller = Manager.PullerPhysics; if (_puller == null || ControlledComponent == null) { return; @@ -206,7 +133,7 @@ namespace Content.Shared.Physics.Pull if (!_puller.Owner.IsInSameOrNoContainer(ControlledComponent.Owner)) { - StopPull(); + Manager.Puller = null; return; } @@ -214,7 +141,7 @@ namespace Content.Shared.Physics.Pull if (distance.Length > DistBeforeStopPull) { - StopPull(); + Manager.Puller = null; } else if (MovingTo.HasValue) { diff --git a/Content.Shared/Physics/Pull/PullStartedMessage.cs b/Content.Shared/Physics/Pull/PullStartedMessage.cs index 00d51bdbd3..ea043c2b4b 100644 --- a/Content.Shared/Physics/Pull/PullStartedMessage.cs +++ b/Content.Shared/Physics/Pull/PullStartedMessage.cs @@ -4,7 +4,7 @@ namespace Content.Shared.Physics.Pull { public class PullStartedMessage : PullMessage { - public PullStartedMessage(PullController controller, IPhysicsComponent puller, IPhysicsComponent pulled) : + public PullStartedMessage(IPhysicsComponent puller, IPhysicsComponent pulled) : base(puller, pulled) { }