From 76e13eed3595747135a8e0f4d668a9011d2ea06f Mon Sep 17 00:00:00 2001 From: Leon Friedrich <60421075+ElectroJr@users.noreply.github.com> Date: Fri, 24 May 2024 17:03:03 +1200 Subject: [PATCH] Improve InteractionSystem range & BUI checks (#27999) * Improve InteractionSystem range & BUI checks * Ghost fixes * AAA * Fix test * fix nullable * revert to broadcast event * Fixes for eengine PR * Ah buckle code * ) --- .../UI/InstrumentBoundUserInterface.cs | 10 +- .../Interactable/InteractionSystem.cs | 22 +- Content.IntegrationTests/Tests/EntityTest.cs | 4 + .../Configurable/ConfigurationSystem.cs | 1 + .../Interaction/InteractionSystem.cs | 29 +- .../StationEvents/Events/ImmovableRodRule.cs | 4 +- .../Weapons/Melee/MeleeWeaponSystem.cs | 9 +- Content.Shared/Actions/SharedActionsSystem.cs | 8 +- .../Buckle/SharedBuckleSystem.Buckle.cs | 2 +- .../Interaction/SharedInteractionSystem.cs | 254 ++++++++++++------ .../Inventory/InventorySystem.Equip.cs | 6 +- .../MagicMirror/SharedMagicMirrorSystem.cs | 10 +- .../Movement/Pulling/Systems/PullingSystem.cs | 13 +- .../EntitySystems/SharedStorageSystem.cs | 2 +- .../UserInterface/ActivatableUIComponent.cs | 2 +- .../UserInterface/ActivatableUISystem.cs | 88 ++---- Content.Shared/Verbs/SharedVerbSystem.cs | 14 +- .../Fun/Instruments/base_instruments.yml | 2 +- 18 files changed, 232 insertions(+), 248 deletions(-) diff --git a/Content.Client/Instruments/UI/InstrumentBoundUserInterface.cs b/Content.Client/Instruments/UI/InstrumentBoundUserInterface.cs index 2a846ff708..0f5729f55b 100644 --- a/Content.Client/Instruments/UI/InstrumentBoundUserInterface.cs +++ b/Content.Client/Instruments/UI/InstrumentBoundUserInterface.cs @@ -37,14 +37,8 @@ namespace Content.Client.Instruments.UI protected override void ReceiveMessage(BoundUserInterfaceMessage message) { - switch (message) - { - case InstrumentBandResponseBuiMessage bandRx: - _bandMenu?.Populate(bandRx.Nearby, EntMan); - break; - default: - break; - } + if (message is InstrumentBandResponseBuiMessage bandRx) + _bandMenu?.Populate(bandRx.Nearby, EntMan); } protected override void Open() diff --git a/Content.Client/Interactable/InteractionSystem.cs b/Content.Client/Interactable/InteractionSystem.cs index 0af8830e9a..ff0a607920 100644 --- a/Content.Client/Interactable/InteractionSystem.cs +++ b/Content.Client/Interactable/InteractionSystem.cs @@ -4,24 +4,6 @@ using Robust.Shared.Containers; namespace Content.Client.Interactable { - public sealed class InteractionSystem : SharedInteractionSystem - { - [Dependency] private readonly SharedContainerSystem _container = default!; - - public override bool CanAccessViaStorage(EntityUid user, EntityUid target) - { - if (!EntityManager.EntityExists(target)) - return false; - - if (!_container.TryGetContainingContainer(target, out var container)) - return false; - - if (!HasComp(container.Owner)) - return false; - - // we don't check if the user can access the storage entity itself. This should be handed by the UI system. - // Need to return if UI is open or not - return true; - } - } + // TODO Remove Shared prefix + public sealed class InteractionSystem : SharedInteractionSystem; } diff --git a/Content.IntegrationTests/Tests/EntityTest.cs b/Content.IntegrationTests/Tests/EntityTest.cs index d3b1fb4722..54af64122b 100644 --- a/Content.IntegrationTests/Tests/EntityTest.cs +++ b/Content.IntegrationTests/Tests/EntityTest.cs @@ -350,8 +350,12 @@ namespace Content.IntegrationTests.Tests "DebrisFeaturePlacerController", // Above. "LoadedChunk", // Worldgen chunk loading malding. "BiomeSelection", // Whaddya know, requires config. + "ActivatableUI", // Requires enum key }; + // TODO TESTS + // auto ignore any components that have a "required" data field. + await using var pair = await PoolManager.GetServerClient(); var server = pair.Server; var entityManager = server.ResolveDependency(); diff --git a/Content.Server/Configurable/ConfigurationSystem.cs b/Content.Server/Configurable/ConfigurationSystem.cs index 2683bf4e09..5f5f1ef7d1 100644 --- a/Content.Server/Configurable/ConfigurationSystem.cs +++ b/Content.Server/Configurable/ConfigurationSystem.cs @@ -24,6 +24,7 @@ public sealed class ConfigurationSystem : EntitySystem private void OnInteractUsing(EntityUid uid, ConfigurationComponent component, InteractUsingEvent args) { + // TODO use activatable ui system if (args.Handled) return; diff --git a/Content.Server/Interaction/InteractionSystem.cs b/Content.Server/Interaction/InteractionSystem.cs index 4eac7e9ef1..9ac82b2185 100644 --- a/Content.Server/Interaction/InteractionSystem.cs +++ b/Content.Server/Interaction/InteractionSystem.cs @@ -7,31 +7,6 @@ using Robust.Shared.Player; namespace Content.Server.Interaction { - /// - /// Governs interactions during clicking on entities - /// - [UsedImplicitly] - public sealed partial class InteractionSystem : SharedInteractionSystem - { - [Dependency] private readonly SharedContainerSystem _container = default!; - [Dependency] private readonly UserInterfaceSystem _uiSystem = default!; - - public override bool CanAccessViaStorage(EntityUid user, EntityUid target) - { - if (Deleted(target)) - return false; - - if (!_container.TryGetContainingContainer(target, out var container)) - return false; - - if (!TryComp(container.Owner, out StorageComponent? storage)) - return false; - - if (storage.Container?.ID != container.ID) - return false; - - // we don't check if the user can access the storage entity itself. This should be handed by the UI system. - return _uiSystem.IsUiOpen(container.Owner, StorageComponent.StorageUiKey.Key, user); - } - } + // TODO Remove Shared prefix + public sealed class InteractionSystem : SharedInteractionSystem; } diff --git a/Content.Server/StationEvents/Events/ImmovableRodRule.cs b/Content.Server/StationEvents/Events/ImmovableRodRule.cs index cacb839cd3..aa193f2f4c 100644 --- a/Content.Server/StationEvents/Events/ImmovableRodRule.cs +++ b/Content.Server/StationEvents/Events/ImmovableRodRule.cs @@ -28,7 +28,9 @@ public sealed class ImmovableRodRule : StationEventSystem(out var rod) && proto.TryGetComponent(out var despawn)) { - TryFindRandomTile(out _, out _, out _, out var targetCoords); + if (!TryFindRandomTile(out _, out _, out _, out var targetCoords)) + return; + var speed = RobustRandom.NextFloat(rod.MinSpeed, rod.MaxSpeed); var angle = RobustRandom.NextAngle(); var direction = angle.ToVec(); diff --git a/Content.Server/Weapons/Melee/MeleeWeaponSystem.cs b/Content.Server/Weapons/Melee/MeleeWeaponSystem.cs index 2612e99ec9..190a2d0263 100644 --- a/Content.Server/Weapons/Melee/MeleeWeaponSystem.cs +++ b/Content.Server/Weapons/Melee/MeleeWeaponSystem.cs @@ -187,15 +187,10 @@ public sealed class MeleeWeaponSystem : SharedMeleeWeaponSystem if (session is { } pSession) { (targetCoordinates, targetLocalAngle) = _lag.GetCoordinatesAngle(target, pSession); - } - else - { - var xform = Transform(target); - targetCoordinates = xform.Coordinates; - targetLocalAngle = xform.LocalRotation; + return Interaction.InRangeUnobstructed(user, target, targetCoordinates, targetLocalAngle, range); } - return Interaction.InRangeUnobstructed(user, target, targetCoordinates, targetLocalAngle, range); + return Interaction.InRangeUnobstructed(user, target, range); } protected override void DoDamageEffect(List targets, EntityUid? user, TransformComponent targetXform) diff --git a/Content.Shared/Actions/SharedActionsSystem.cs b/Content.Shared/Actions/SharedActionsSystem.cs index 30687c9322..c92d67ba81 100644 --- a/Content.Shared/Actions/SharedActionsSystem.cs +++ b/Content.Shared/Actions/SharedActionsSystem.cs @@ -502,13 +502,7 @@ public abstract class SharedActionsSystem : EntitySystem return distance <= action.Range; } - if (_interactionSystem.InRangeUnobstructed(user, target, range: action.Range) - && _containerSystem.IsInSameOrParentContainer(user, target)) - { - return true; - } - - return _interactionSystem.CanAccessViaStorage(user, target); + return _interactionSystem.InRangeAndAccessible(user, target, range: action.Range); } public bool ValidateWorldTarget(EntityUid user, EntityCoordinates coords, Entity action) diff --git a/Content.Shared/Buckle/SharedBuckleSystem.Buckle.cs b/Content.Shared/Buckle/SharedBuckleSystem.Buckle.cs index 4e94c6134b..00040211e3 100644 --- a/Content.Shared/Buckle/SharedBuckleSystem.Buckle.cs +++ b/Content.Shared/Buckle/SharedBuckleSystem.Buckle.cs @@ -64,7 +64,7 @@ public abstract partial class SharedBuckleSystem return; var strapPosition = Transform(strapUid).Coordinates; - if (ev.NewPosition.InRange(EntityManager, _transform, strapPosition, strapComp.MaxBuckleDistance)) + if (ev.NewPosition.EntityId.IsValid() && ev.NewPosition.InRange(EntityManager, _transform, strapPosition, strapComp.MaxBuckleDistance)) return; TryUnbuckle(uid, uid, true, component); diff --git a/Content.Shared/Interaction/SharedInteractionSystem.cs b/Content.Shared/Interaction/SharedInteractionSystem.cs index c82a749755..3324ce5b9b 100644 --- a/Content.Shared/Interaction/SharedInteractionSystem.cs +++ b/Content.Shared/Interaction/SharedInteractionSystem.cs @@ -1,11 +1,10 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using Content.Shared.ActionBlocker; -using Content.Shared.Administration; using Content.Shared.Administration.Logs; -using Content.Shared.Administration.Managers; using Content.Shared.CombatMode; using Content.Shared.Database; +using Content.Shared.Ghost; using Content.Shared.Hands; using Content.Shared.Hands.Components; using Content.Shared.Input; @@ -15,12 +14,13 @@ using Content.Shared.Inventory; using Content.Shared.Inventory.Events; using Content.Shared.Item; using Content.Shared.Movement.Components; -using Content.Shared.Movement.Pulling.Components; using Content.Shared.Movement.Pulling.Systems; using Content.Shared.Physics; using Content.Shared.Popups; +using Content.Shared.Storage; using Content.Shared.Tag; using Content.Shared.Timing; +using Content.Shared.UserInterface; using Content.Shared.Verbs; using Content.Shared.Wall; using JetBrains.Annotations; @@ -36,6 +36,7 @@ using Robust.Shared.Player; using Robust.Shared.Random; using Robust.Shared.Serialization; using Robust.Shared.Timing; +using Robust.Shared.Utility; #pragma warning disable 618 @@ -50,12 +51,11 @@ namespace Content.Shared.Interaction [Dependency] private readonly IGameTiming _gameTiming = default!; [Dependency] private readonly INetManager _net = default!; [Dependency] private readonly IMapManager _mapManager = default!; - [Dependency] private readonly ISharedAdminManager _adminManager = default!; [Dependency] private readonly ISharedAdminLogManager _adminLogger = default!; [Dependency] private readonly ActionBlockerSystem _actionBlockerSystem = default!; [Dependency] private readonly RotateToFaceSystem _rotateToFaceSystem = default!; [Dependency] private readonly SharedContainerSystem _containerSystem = default!; - [Dependency] private readonly SharedPhysicsSystem _sharedBroadphaseSystem = default!; + [Dependency] private readonly SharedPhysicsSystem _broadphase = default!; [Dependency] private readonly SharedTransformSystem _transform = default!; [Dependency] private readonly SharedVerbSystem _verbSystem = default!; [Dependency] private readonly SharedPopupSystem _popupSystem = default!; @@ -64,6 +64,18 @@ namespace Content.Shared.Interaction [Dependency] private readonly InventorySystem _inventory = default!; [Dependency] private readonly IRobustRandom _random = default!; [Dependency] private readonly TagSystem _tagSystem = default!; + [Dependency] private readonly SharedUserInterfaceSystem _ui = default!; + + private EntityQuery _ignoreUiRangeQuery; + private EntityQuery _fixtureQuery; + private EntityQuery _itemQuery; + private EntityQuery _physicsQuery; + private EntityQuery _handsQuery; + private EntityQuery _relayQuery; + private EntityQuery _combatQuery; + private EntityQuery _wallMountQuery; + private EntityQuery _delayQuery; + private EntityQuery _uiQuery; private const CollisionGroup InRangeUnobstructedMask = CollisionGroup.Impassable | CollisionGroup.InteractImpassable; @@ -76,6 +88,17 @@ namespace Content.Shared.Interaction public override void Initialize() { + _ignoreUiRangeQuery = GetEntityQuery(); + _fixtureQuery = GetEntityQuery(); + _itemQuery = GetEntityQuery(); + _physicsQuery = GetEntityQuery(); + _handsQuery = GetEntityQuery(); + _relayQuery = GetEntityQuery(); + _combatQuery = GetEntityQuery(); + _wallMountQuery = GetEntityQuery(); + _delayQuery = GetEntityQuery(); + _uiQuery = GetEntityQuery(); + SubscribeLocalEvent(HandleUserInterfaceRangeCheck); SubscribeLocalEvent(OnBoundInterfaceInteractAttempt); @@ -111,29 +134,57 @@ namespace Content.Shared.Interaction /// private void OnBoundInterfaceInteractAttempt(BoundUserInterfaceMessageAttempt ev) { - var user = ev.Actor; + _uiQuery.TryComp(ev.Target, out var uiComp); + if (!_actionBlockerSystem.CanInteract(ev.Actor, ev.Target)) + { + // We permit ghosts to open uis unless explicitly blocked + if (ev.Message is not OpenBoundInterfaceMessage || !HasComp(ev.Actor) || uiComp?.BlockSpectators == true) + { + ev.Cancel(); + return; + } + } + + var range = _ui.GetUiRange(ev.Target, ev.UiKey); - if (!_actionBlockerSystem.CanInteract(user, ev.Target)) + // As long as range>0, the UI frame updates should have auto-closed the UI if it is out of range. + DebugTools.Assert(range <= 0 || UiRangeCheck(ev.Actor, ev.Target, range)); + + if (range <= 0 && !IsAccessible(ev.Actor, ev.Target)) { ev.Cancel(); return; } - // Check if the bound entity is accessible. Note that we allow admins to ignore this restriction, so that - // they can fiddle with UI's that people can't normally interact with (e.g., placing things directly into - // other people's backpacks). - if (!_containerSystem.IsInSameOrParentContainer(user, ev.Target) - && !CanAccessViaStorage(user, ev.Target) - && !_adminManager.HasAdminFlag(user, AdminFlags.Admin)) + if (uiComp == null) + return; + + if (uiComp.SingleUser && uiComp.CurrentSingleUser != ev.Actor) { ev.Cancel(); return; } - if (!InRangeUnobstructed(user, ev.Target)) - { + if (!uiComp.RequireHands) + return; + + if (!_handsQuery.TryComp(ev.Actor, out var hands) || hands.Hands.Count == 0) ev.Cancel(); - } + } + + private bool UiRangeCheck(Entity user, Entity target, float range) + { + if (!Resolve(target, ref target.Comp)) + return false; + + if (user.Owner == target.Owner) + return true; + + // Fast check: if the user is the parent of the entity (e.g., holding it), we always assume that it is in range + if (target.Comp.ParentUid == user.Owner) + return true; + + return InRangeAndAccessible(user, target, range) || _ignoreUiRangeQuery.HasComp(user); } /// @@ -190,10 +241,7 @@ namespace Content.Shared.Interaction if (!InRangeUnobstructed(userEntity.Value, uid, popup: true)) return false; - if (!TryComp(uid, out PullableComponent? pull)) - return false; - - _pullSystem.TogglePull(uid, userEntity.Value, pull); + _pullSystem.TogglePull(uid, userEntity.Value); return false; } @@ -269,7 +317,7 @@ namespace Content.Shared.Interaction public bool CombatModeCanHandInteract(EntityUid user, EntityUid? target) { // Always allow attack in these cases - if (target == null || !TryComp(user, out var hands) || hands.ActiveHand?.HeldEntity is not null) + if (target == null || !_handsQuery.TryComp(user, out var hands) || hands.ActiveHand?.HeldEntity is not null) return false; // Only eat input if: @@ -277,7 +325,7 @@ namespace Content.Shared.Interaction // - Target doesn't cancel should-interact event // This is intended to allow items to be picked up in combat mode, // but to also allow items to force attacks anyway (like mobs which are items, e.g. mice) - if (!HasComp(target)) + if (!_itemQuery.HasComp(target)) return false; var combatEv = new CombatModeShouldHandInteractEvent(); @@ -307,7 +355,7 @@ namespace Content.Shared.Interaction bool checkAccess = true, bool checkCanUse = true) { - if (TryComp(user, out var relay) && relay.RelayEntity is not null) + if (_relayQuery.TryComp(user, out var relay) && relay.RelayEntity is not null) { // TODO this needs to be handled better. This probably bypasses many complex can-interact checks in weird roundabout ways. if (_actionBlockerSystem.CanInteract(user, target)) @@ -321,7 +369,7 @@ namespace Content.Shared.Interaction if (target != null && Deleted(target.Value)) return; - if (!altInteract && TryComp(user, out var combatMode) && combatMode.IsInCombatMode) + if (!altInteract && _combatQuery.TryComp(user, out var combatMode) && combatMode.IsInCombatMode) { if (!CombatModeCanHandInteract(user, target)) return; @@ -343,10 +391,7 @@ namespace Content.Shared.Interaction // Check if interacted entity is in the same container, the direct child, or direct parent of the user. // Also checks if the item is accessible via some storage UI (e.g., open backpack) - if (checkAccess - && target != null - && !_containerSystem.IsInSameOrParentContainer(user, target.Value) - && !CanAccessViaStorage(user, target.Value)) + if (checkAccess && target != null && !IsAccessible(user, target.Value)) return; var inRangeUnobstructed = target == null @@ -354,7 +399,7 @@ namespace Content.Shared.Interaction : !checkAccess || InRangeUnobstructed(user, target.Value); // permits interactions with wall mounted entities // Does the user have hands? - if (!TryComp(user, out var hands) || hands.ActiveHand == null) + if (!_handsQuery.TryComp(user, out var hands) || hands.ActiveHand == null) { var ev = new InteractNoHandEvent(user, target, coordinates); RaiseLocalEvent(user, ev); @@ -494,7 +539,7 @@ namespace Content.Shared.Interaction predicate ??= _ => false; var ray = new CollisionRay(origin.Position, dir.Normalized(), collisionMask); - var rayResults = _sharedBroadphaseSystem.IntersectRayWithPredicate(origin.MapId, ray, dir.Length(), predicate.Invoke, false).ToList(); + var rayResults = _broadphase.IntersectRayWithPredicate(origin.MapId, ray, dir.Length(), predicate.Invoke, false).ToList(); if (rayResults.Count == 0) return dir.Length(); @@ -557,23 +602,29 @@ namespace Content.Shared.Interaction } var ray = new CollisionRay(origin.Position, dir.Normalized(), (int) collisionMask); - var rayResults = _sharedBroadphaseSystem.IntersectRayWithPredicate(origin.MapId, ray, length, predicate.Invoke, false).ToList(); + var rayResults = _broadphase.IntersectRayWithPredicate(origin.MapId, ray, length, predicate.Invoke, false).ToList(); return rayResults.Count == 0; } public bool InRangeUnobstructed( - EntityUid origin, - EntityUid other, + Entity origin, + Entity other, float range = InteractionRange, CollisionGroup collisionMask = InRangeUnobstructedMask, Ignored? predicate = null, bool popup = false) { - if (!TryComp(other, out TransformComponent? otherXform)) + if (!Resolve(other, ref other.Comp)) return false; - return InRangeUnobstructed(origin, other, otherXform.Coordinates, otherXform.LocalRotation, range, collisionMask, predicate, + return InRangeUnobstructed(origin, + other, + other.Comp.Coordinates, + other.Comp.LocalRotation, + range, + collisionMask, + predicate, popup); } @@ -605,8 +656,8 @@ namespace Content.Shared.Interaction /// True if the two points are within a given range without being obstructed. /// public bool InRangeUnobstructed( - EntityUid origin, - EntityUid other, + Entity origin, + Entity other, EntityCoordinates otherCoordinates, Angle otherAngle, float range = InteractionRange, @@ -614,10 +665,10 @@ namespace Content.Shared.Interaction Ignored? predicate = null, bool popup = false) { - Ignored combinedPredicate = e => e == origin || (predicate?.Invoke(e) ?? false); + Ignored combinedPredicate = e => e == origin.Owner || (predicate?.Invoke(e) ?? false); var inRange = true; MapCoordinates originPos = default; - var targetPos = otherCoordinates.ToMap(EntityManager, _transform); + var targetPos = _transform.ToMapCoordinates(otherCoordinates); Angle targetRot = default; // So essentially: @@ -627,23 +678,30 @@ namespace Content.Shared.Interaction // Alternatively we could check centre distances first though // that means we wouldn't be able to easily check overlap interactions. if (range > 0f && - TryComp(origin, out var fixtureA) && + _fixtureQuery.TryComp(origin, out var fixtureA) && // These fixture counts are stuff that has the component but no fixtures for (e.g. buttons). // At least until they get removed. fixtureA.FixtureCount > 0 && - TryComp(other, out var fixtureB) && + _fixtureQuery.TryComp(other, out var fixtureB) && fixtureB.FixtureCount > 0 && - TryComp(origin, out TransformComponent? xformA)) + Resolve(origin, ref origin.Comp)) { - var (worldPosA, worldRotA) = xformA.GetWorldPositionRotation(); + var (worldPosA, worldRotA) = origin.Comp.GetWorldPositionRotation(); var xfA = new Transform(worldPosA, worldRotA); var parentRotB = _transform.GetWorldRotation(otherCoordinates.EntityId); var xfB = new Transform(targetPos.Position, parentRotB + otherAngle); // Different map or the likes. - if (!_sharedBroadphaseSystem.TryGetNearest(origin, other, - out _, out _, out var distance, - xfA, xfB, fixtureA, fixtureB)) + if (!_broadphase.TryGetNearest( + origin, + other, + out _, + out _, + out var distance, + xfA, + xfB, + fixtureA, + fixtureB)) { inRange = false; } @@ -665,15 +723,15 @@ namespace Content.Shared.Interaction else { // We'll still do the raycast from the centres but we'll bump the range as we know they're in range. - originPos = _transform.GetMapCoordinates(origin, xform: xformA); + originPos = _transform.GetMapCoordinates(origin, xform: origin.Comp); range = (originPos.Position - targetPos.Position).Length(); } } // No fixtures, e.g. wallmounts. else { - originPos = _transform.GetMapCoordinates(origin); - var otherParent = Transform(other).ParentUid; + originPos = _transform.GetMapCoordinates(origin, origin); + var otherParent = (other.Comp ?? Transform(other)).ParentUid; targetRot = otherParent.IsValid() ? Transform(otherParent).LocalRotation + otherAngle : otherAngle; } @@ -724,13 +782,13 @@ namespace Content.Shared.Interaction { HashSet ignored = new(); - if (HasComp(target) && TryComp(target, out PhysicsComponent? physics) && physics.CanCollide) + if (_itemQuery.HasComp(target) && _physicsQuery.TryComp(target, out var physics) && physics.CanCollide) { // If the target is an item, we ignore any colliding entities. Currently done so that if items get stuck // inside of walls, users can still pick them up. - ignored.UnionWith(_sharedBroadphaseSystem.GetEntitiesIntersectingBody(target, (int) collisionMask, false, physics)); + ignored.UnionWith(_broadphase.GetEntitiesIntersectingBody(target, (int) collisionMask, false, physics)); } - else if (TryComp(target, out WallMountComponent? wallMount)) + else if (_wallMountQuery.TryComp(target, out var wallMount)) { // wall-mount exemptions may be restricted to a specific angle range.da @@ -748,13 +806,7 @@ namespace Content.Shared.Interaction ignored.UnionWith(grid.GetAnchoredEntities(targetCoords)); } - Ignored combinedPredicate = e => - { - return e == target - || (predicate?.Invoke(e) ?? false) - || ignored.Contains(e); - }; - + Ignored combinedPredicate = e => e == target || (predicate?.Invoke(e) ?? false) || ignored.Contains(e); return combinedPredicate; } @@ -951,10 +1003,8 @@ namespace Content.Shared.Interaction bool checkUseDelay = true, bool checkAccess = true) { - UseDelayComponent? delayComponent = null; - if (checkUseDelay - && TryComp(used, out delayComponent) - && _useDelay.IsDelayed((used, delayComponent))) + _delayQuery.TryComp(used, out var delayComponent); + if (checkUseDelay && delayComponent != null && _useDelay.IsDelayed((used, delayComponent))) return false; if (checkCanInteract && !_actionBlockerSystem.CanInteract(user, used)) @@ -965,11 +1015,11 @@ namespace Content.Shared.Interaction // Check if interacted entity is in the same container, the direct child, or direct parent of the user. // This is bypassed IF the interaction happened through an item slot (e.g., backpack UI) - if (checkAccess && !_containerSystem.IsInSameOrParentContainer(user, used) && !CanAccessViaStorage(user, used)) + if (checkAccess && !IsAccessible(user, used)) return false; // Does the user have hands? - if (!HasComp(user)) + if (!_handsQuery.HasComp(user)) return false; var activateMsg = new ActivateInWorldEvent(user, used); @@ -979,7 +1029,9 @@ namespace Content.Shared.Interaction DoContactInteraction(user, used, activateMsg); // Still need to call this even without checkUseDelay in case this gets relayed from Activate. - _useDelay.TryResetDelay(used, component: delayComponent); + if (delayComponent != null) + _useDelay.TryResetDelay(used, component: delayComponent); + if (!activateMsg.WasLogged) _adminLogger.Add(LogType.InteractActivate, LogImpact.Low, $"{ToPrettyString(user):user} activated {ToPrettyString(used):used}"); return true; @@ -1000,11 +1052,8 @@ namespace Content.Shared.Interaction bool checkCanInteract = true, bool checkUseDelay = true) { - UseDelayComponent? delayComponent = null; - - if (checkUseDelay - && TryComp(used, out delayComponent) - && _useDelay.IsDelayed((used, delayComponent))) + _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. if (checkCanInteract && !_actionBlockerSystem.CanInteract(user, used)) @@ -1066,11 +1115,60 @@ namespace Content.Shared.Interaction } #endregion + /// + /// Check if a user can access a target (stored in the same containers) and is in range without obstructions. + /// + public bool InRangeAndAccessible( + Entity user, + Entity target, + float range = InteractionRange, + CollisionGroup collisionMask = InRangeUnobstructedMask, + Ignored? predicate = null) + { + if (user == target) + return true; + + if (!Resolve(user, ref user.Comp)) + return false; + + if (!Resolve(target, ref target.Comp)) + return false; + + return IsAccessible(user, target) && InRangeUnobstructed(user, target, range, collisionMask, predicate); + } + + /// + /// Check if a user can access a target or if they are stored in different containers. + /// + public bool IsAccessible(Entity user, Entity target) + { + if (_containerSystem.IsInSameOrParentContainer(user, target, out _, out var container)) + return true; + + return container != null && CanAccessViaStorage(user, target, container); + } + /// /// If a target is in range, but not in the same container as the user, it may be inside of a backpack. This /// checks if the user can access the item in these situations. /// - public abstract bool CanAccessViaStorage(EntityUid user, EntityUid target); + public bool CanAccessViaStorage(EntityUid user, EntityUid target) + { + if (!_containerSystem.TryGetContainingContainer(target, out var container)) + return false; + + return CanAccessViaStorage(user, target, container); + } + + /// + public bool CanAccessViaStorage(EntityUid user, EntityUid target, BaseContainer container) + { + if (StorageComponent.ContainerId != container.ID) + return false; + + // we don't check if the user can access the storage entity itself. This should be handed by the UI system. + return _ui.IsUiOpen(target, StorageComponent.StorageUiKey.Key, user); + } /// /// Checks whether an entity currently equipped by another player is accessible to some user. This shouldn't @@ -1151,19 +1249,15 @@ namespace Content.Shared.Interaction RaiseLocalEvent(uidB.Value, new ContactInteractionEvent(uidA)); } + private void HandleUserInterfaceRangeCheck(ref BoundUserInterfaceCheckRangeEvent ev) { if (ev.Result == BoundUserInterfaceRangeResult.Fail) return; - if (InRangeUnobstructed(ev.Actor, ev.Target, ev.Data.InteractionRange)) - { - ev.Result = BoundUserInterfaceRangeResult.Pass; - } - else - { - ev.Result = BoundUserInterfaceRangeResult.Fail; - } + ev.Result = UiRangeCheck(ev.Actor!, ev.Target, ev.Data.InteractionRange) + ? BoundUserInterfaceRangeResult.Pass + : BoundUserInterfaceRangeResult.Fail; } } diff --git a/Content.Shared/Inventory/InventorySystem.Equip.cs b/Content.Shared/Inventory/InventorySystem.Equip.cs index 400dfb0beb..7fd156213b 100644 --- a/Content.Shared/Inventory/InventorySystem.Equip.cs +++ b/Content.Shared/Inventory/InventorySystem.Equip.cs @@ -210,11 +210,7 @@ public abstract partial class InventorySystem return false; // Can the actor reach the item? - if (_interactionSystem.InRangeUnobstructed(actor, itemUid) && _containerSystem.IsInSameOrParentContainer(actor, itemUid)) - return true; - - // Is the item in an open storage UI, i.e., is the user quick-equipping from an open backpack? - if (_interactionSystem.CanAccessViaStorage(actor, itemUid)) + if (_interactionSystem.InRangeAndAccessible(actor, itemUid)) return true; // Is the actor currently stripping the target? Here we could check if the actor has the stripping UI open, but diff --git a/Content.Shared/MagicMirror/SharedMagicMirrorSystem.cs b/Content.Shared/MagicMirror/SharedMagicMirrorSystem.cs index 91059d60bf..433ad6b4fc 100644 --- a/Content.Shared/MagicMirror/SharedMagicMirrorSystem.cs +++ b/Content.Shared/MagicMirror/SharedMagicMirrorSystem.cs @@ -4,6 +4,7 @@ using Content.Shared.Humanoid.Markings; using Content.Shared.Interaction; using Content.Shared.UserInterface; using Robust.Shared.Serialization; +using Robust.Shared.Utility; namespace Content.Shared.MagicMirror; @@ -21,10 +22,13 @@ public abstract class SharedMagicMirrorSystem : EntitySystem private void OnMirrorRangeCheck(EntityUid uid, MagicMirrorComponent component, ref BoundUserInterfaceCheckRangeEvent args) { - if (!Exists(component.Target) || !_interaction.InRangeUnobstructed(uid, component.Target.Value)) - { + if (args.Result == BoundUserInterfaceRangeResult.Fail) + return; + + DebugTools.Assert(component.Target != null && Exists(component.Target)); + + if (!_interaction.InRangeUnobstructed(uid, component.Target.Value)) args.Result = BoundUserInterfaceRangeResult.Fail; - } } private void OnBeforeUIOpen(Entity ent, ref BeforeActivatableUIOpenEvent args) diff --git a/Content.Shared/Movement/Pulling/Systems/PullingSystem.cs b/Content.Shared/Movement/Pulling/Systems/PullingSystem.cs index 2781c49529..3de71172c7 100644 --- a/Content.Shared/Movement/Pulling/Systems/PullingSystem.cs +++ b/Content.Shared/Movement/Pulling/Systems/PullingSystem.cs @@ -362,14 +362,17 @@ public sealed class PullingSystem : EntitySystem return !startPull.Cancelled && !getPulled.Cancelled; } - public bool TogglePull(EntityUid pullableUid, EntityUid pullerUid, PullableComponent pullable) + public bool TogglePull(Entity pullable, EntityUid pullerUid) { - if (pullable.Puller == pullerUid) + if (!Resolve(pullable, ref pullable.Comp, false)) + return false; + + if (pullable.Comp.Puller == pullerUid) { - return TryStopPull(pullableUid, pullable); + return TryStopPull(pullable, pullable.Comp); } - return TryStartPull(pullerUid, pullableUid, pullableComp: pullable); + return TryStartPull(pullerUid, pullable, pullableComp: pullable); } public bool TogglePull(EntityUid pullerUid, PullerComponent puller) @@ -377,7 +380,7 @@ public sealed class PullingSystem : EntitySystem if (!TryComp(puller.Pulling, out var pullable)) return false; - return TogglePull(puller.Pulling.Value, pullerUid, pullable); + return TogglePull((puller.Pulling.Value, pullable), pullerUid); } public bool TryStartPull(EntityUid pullerUid, EntityUid pullableUid, diff --git a/Content.Shared/Storage/EntitySystems/SharedStorageSystem.cs b/Content.Shared/Storage/EntitySystems/SharedStorageSystem.cs index ea0c9632f0..778e2b2e74 100644 --- a/Content.Shared/Storage/EntitySystems/SharedStorageSystem.cs +++ b/Content.Shared/Storage/EntitySystems/SharedStorageSystem.cs @@ -1078,7 +1078,7 @@ public abstract class SharedStorageSystem : EntitySystem /// true if inserted, false otherwise public bool PlayerInsertEntityInWorld(Entity uid, EntityUid player, EntityUid toInsert) { - if (!Resolve(uid, ref uid.Comp) || !_interactionSystem.InRangeUnobstructed(player, uid)) + if (!Resolve(uid, ref uid.Comp) || !_interactionSystem.InRangeUnobstructed(player, uid.Owner)) return false; if (!Insert(uid, toInsert, out _, user: player, uid.Comp)) diff --git a/Content.Shared/UserInterface/ActivatableUIComponent.cs b/Content.Shared/UserInterface/ActivatableUIComponent.cs index 3f83816b7d..93f05acac0 100644 --- a/Content.Shared/UserInterface/ActivatableUIComponent.cs +++ b/Content.Shared/UserInterface/ActivatableUIComponent.cs @@ -57,7 +57,7 @@ namespace Content.Shared.UserInterface /// [ViewVariables(VVAccess.ReadWrite)] [DataField] - public bool AllowSpectator = true; + public bool BlockSpectators; /// /// Whether the item must be in the user's currently selected/active hand. diff --git a/Content.Shared/UserInterface/ActivatableUISystem.cs b/Content.Shared/UserInterface/ActivatableUISystem.cs index a6d27ac545..c1822c4ee3 100644 --- a/Content.Shared/UserInterface/ActivatableUISystem.cs +++ b/Content.Shared/UserInterface/ActivatableUISystem.cs @@ -8,7 +8,7 @@ using Content.Shared.Interaction; using Content.Shared.Interaction.Events; using Content.Shared.Popups; using Content.Shared.Verbs; -using Robust.Shared.Containers; +using Robust.Shared.Utility; namespace Content.Shared.UserInterface; @@ -19,15 +19,12 @@ public sealed partial class ActivatableUISystem : EntitySystem [Dependency] private readonly SharedUserInterfaceSystem _uiSystem = default!; [Dependency] private readonly SharedPopupSystem _popupSystem = default!; [Dependency] private readonly SharedHandsSystem _hands = default!; - [Dependency] private readonly SharedContainerSystem _container = default!; - [Dependency] private readonly SharedInteractionSystem _interaction = default!; - - private readonly List _toClose = new(); public override void Initialize() { base.Initialize(); + SubscribeLocalEvent(OnStartup); SubscribeLocalEvent(OnUseInHand); SubscribeLocalEvent(OnActivate); SubscribeLocalEvent(OnInteractUsing); @@ -37,28 +34,24 @@ public sealed partial class ActivatableUISystem : EntitySystem SubscribeLocalEvent>(GetActivationVerb); SubscribeLocalEvent>(GetVerb); - // TODO ActivatableUI - // Add UI-user component, and listen for user container changes. - // I.e., should lose a computer UI if a player gets shut into a locker. - SubscribeLocalEvent(OnGotInserted); - SubscribeLocalEvent(OnGotRemoved); - - SubscribeLocalEvent(OnBoundInterfaceInteractAttempt); SubscribeLocalEvent(OnActionPerform); InitializePower(); } - private void OnBoundInterfaceInteractAttempt(BoundUserInterfaceMessageAttempt ev) + private void OnStartup(Entity ent, ref ComponentStartup args) { - if (!TryComp(ev.Target, out ActivatableUIComponent? comp)) - return; - - if (!comp.RequireHands) + if (ent.Comp.Key == null) + { + Log.Error($"Missing UI Key for entity: {ToPrettyString(ent)}"); return; + } - if (!TryComp(ev.Actor, out HandsComponent? hands) || hands.Hands.Count == 0) - ev.Cancel(); + // TODO BUI + // set interaction range to zero to avoid constant range checks. + // + // if (ent.Comp.InHandsOnly && _uiSystem.TryGetInterfaceData(ent.Owner, ent.Comp.Key, out var data)) + // data.InteractionRange = 0; } private void OnActionPerform(EntityUid uid, UserInterfaceComponent component, OpenUiActionEvent args) @@ -77,9 +70,10 @@ public sealed partial class ActivatableUISystem : EntitySystem args.Verbs.Add(new ActivationVerb { - // TODO VERBS add "open UI" icon Act = () => InteractUI(args.User, uid, component), - Text = Loc.GetString(component.VerbText) + Text = Loc.GetString(component.VerbText), + // TODO VERB ICON find a better icon + Icon = new SpriteSpecifier.Texture(new ResPath("/Textures/Interface/VerbIcons/settings.svg.192dpi.png")), }); } @@ -90,9 +84,10 @@ public sealed partial class ActivatableUISystem : EntitySystem args.Verbs.Add(new Verb { - // TODO VERBS add "open UI" icon Act = () => InteractUI(args.User, uid, component), - Text = Loc.GetString(component.VerbText) + Text = Loc.GetString(component.VerbText), + // TODO VERB ICON find a better icon + Icon = new SpriteSpecifier.Texture(new ResPath("/Textures/Interface/VerbIcons/settings.svg.192dpi.png")), }); } @@ -119,7 +114,7 @@ public sealed partial class ActivatableUISystem : EntitySystem } } - return args.CanInteract || component.AllowSpectator && HasComp(args.User); + return args.CanInteract || HasComp(args.User) && !component.BlockSpectators; } private void OnUseInHand(EntityUid uid, ActivatableUIComponent component, UseInHandEvent args) @@ -191,7 +186,7 @@ public sealed partial class ActivatableUISystem : EntitySystem return true; } - if (!_blockerSystem.CanInteract(user, uiEntity) && (!aui.AllowSpectator || !HasComp(user))) + if (!_blockerSystem.CanInteract(user, uiEntity) && (!HasComp(user) || aui.BlockSpectators)) return false; if (aui.RequireHands) @@ -286,47 +281,4 @@ public sealed partial class ActivatableUISystem : EntitySystem if (ent.Comp.RequireHands && ent.Comp.InHandsOnly) CloseAll(ent, ent); } - - private void OnGotInserted(Entity ent, ref EntGotInsertedIntoContainerMessage args) - { - CheckAccess((ent, ent)); - } - - private void OnGotRemoved(Entity ent, ref EntGotRemovedFromContainerMessage args) - { - CheckAccess((ent, ent)); - } - - public void CheckAccess(Entity ent) - { - if (!Resolve(ent, ref ent.Comp)) - return; - - if (ent.Comp.Key == null) - { - Log.Error($"Encountered null key in activatable ui on entity {ToPrettyString(ent)}"); - return; - } - - foreach (var user in _uiSystem.GetActors(ent.Owner, ent.Comp.Key)) - { - if (!_container.IsInSameOrParentContainer(user, ent) - && !_interaction.CanAccessViaStorage(user, ent)) - { - _toClose.Add(user); - continue; - - } - - if (!_interaction.InRangeUnobstructed(user, ent)) - _toClose.Add(user); - } - - foreach (var user in _toClose) - { - _uiSystem.CloseUi(ent.Owner, ent.Comp.Key, user); - } - - _toClose.Clear(); - } } diff --git a/Content.Shared/Verbs/SharedVerbSystem.cs b/Content.Shared/Verbs/SharedVerbSystem.cs index 60714aea8f..e78fe98f4c 100644 --- a/Content.Shared/Verbs/SharedVerbSystem.cs +++ b/Content.Shared/Verbs/SharedVerbSystem.cs @@ -72,19 +72,7 @@ namespace Content.Shared.Verbs extraCategories = new(); // accessibility checks - bool canAccess = false; - if (force || target == user) - canAccess = true; - else if (_interactionSystem.InRangeUnobstructed(user, target)) - { - // Note that being in a container does not count as an obstruction for InRangeUnobstructed - // Therefore, we need extra checks to ensure the item is actually accessible: - if (ContainerSystem.IsInSameOrParentContainer(user, target)) - canAccess = true; - else - // the item might be in a backpack that the user has open - canAccess = _interactionSystem.CanAccessViaStorage(user, target); - } + var canAccess = force || _interactionSystem.InRangeAndAccessible(user, target); // A large number of verbs need to check action blockers. Instead of repeatedly having each system individually // call ActionBlocker checks, just cache it for the verb request. diff --git a/Resources/Prototypes/Entities/Objects/Fun/Instruments/base_instruments.yml b/Resources/Prototypes/Entities/Objects/Fun/Instruments/base_instruments.yml index 684260b737..122ff42eb2 100644 --- a/Resources/Prototypes/Entities/Objects/Fun/Instruments/base_instruments.yml +++ b/Resources/Prototypes/Entities/Objects/Fun/Instruments/base_instruments.yml @@ -29,7 +29,7 @@ components: - type: Instrument - type: ActivatableUI - allowSpectator: false # otherwise they can play client-side music + blockSpectators: true # otherwise they can play client-side music inHandsOnly: false singleUser: true requireHands: true -- 2.51.2