Fix the pathfinding leak (#1647)

Off-grid entities were continually expanding the graph indefinitely which is... bad.

Co-authored-by: Metal Gear Sloth <metalgearsloth@gmail.com>
This commit is contained in:
metalgearsloth
2020-08-12 01:36:40 +10:00
committed by GitHub
parent 89fff7dab2
commit 452a67032f
5 changed files with 92 additions and 71 deletions

View File

@@ -1,15 +1,17 @@
using Robust.Shared.GameObjects;
using Robust.Shared.Interfaces.GameObjects;
namespace Content.Server.GameObjects.Components.Access
{
public sealed class AccessReaderChangeMessage : EntitySystemMessage
{
public EntityUid Uid { get; }
public IEntity Sender { get; }
public bool Enabled { get; }
public AccessReaderChangeMessage(EntityUid uid, bool enabled)
public AccessReaderChangeMessage(IEntity entity, bool enabled)
{
Uid = uid;
Sender = entity;
Enabled = enabled;
}
}

View File

@@ -188,7 +188,7 @@ namespace Content.Server.GameObjects
SetAppearance(DoorVisualState.Open);
}, _cancellationTokenSource.Token);
Owner.EntityManager.EventBus.RaiseEvent(EventSource.Local, new AccessReaderChangeMessage(Owner.Uid, false));
Owner.EntityManager.EventBus.RaiseEvent(EventSource.Local, new AccessReaderChangeMessage(Owner, false));
}
public virtual bool CanClose()
@@ -284,7 +284,7 @@ namespace Content.Server.GameObjects
State = DoorState.Closed;
SetAppearance(DoorVisualState.Closed);
}, _cancellationTokenSource.Token);
Owner.EntityManager.EventBus.RaiseEvent(EventSource.Local, new AccessReaderChangeMessage(Owner.Uid, true));
Owner.EntityManager.EventBus.RaiseEvent(EventSource.Local, new AccessReaderChangeMessage(Owner, true));
return true;
}

View File

@@ -25,17 +25,17 @@ namespace Content.Server.GameObjects.EntitySystems.Pathfinding
/// Whenever there's a change in the collision layers we update the mask as the graph has more reads than writes
/// </summary>
public int BlockedCollisionMask { get; private set; }
private readonly Dictionary<EntityUid, int> _blockedCollidables = new Dictionary<EntityUid, int>(0);
private readonly Dictionary<IEntity, int> _blockedCollidables = new Dictionary<IEntity, int>(0);
public IReadOnlyDictionary<EntityUid, int> PhysicsLayers => _physicsLayers;
private readonly Dictionary<EntityUid, int> _physicsLayers = new Dictionary<EntityUid, int>(0);
public IReadOnlyDictionary<IEntity, int> PhysicsLayers => _physicsLayers;
private readonly Dictionary<IEntity, int> _physicsLayers = new Dictionary<IEntity, int>(0);
/// <summary>
/// The entities on this tile that require access to traverse
/// </summary>
/// We don't store the ICollection, at least for now, as we'd need to replicate the access code here
public IReadOnlyCollection<AccessReader> AccessReaders => _accessReaders.Values;
private readonly Dictionary<EntityUid, AccessReader> _accessReaders = new Dictionary<EntityUid, AccessReader>(0);
private readonly Dictionary<IEntity, AccessReader> _accessReaders = new Dictionary<IEntity, AccessReader>(0);
public PathfindingNode(PathfindingChunk parent, TileRef tileRef)
{
@@ -44,6 +44,17 @@ namespace Content.Server.GameObjects.EntitySystems.Pathfinding
GenerateMask();
}
public static bool IsRelevant(IEntity entity, ICollidableComponent collidableComponent)
{
if (entity.Transform.GridID == GridId.Invalid ||
(PathfindingSystem.TrackedCollisionLayers & collidableComponent.CollisionLayer) == 0)
{
return false;
}
return true;
}
/// <summary>
/// Return our neighboring nodes (even across chunks)
/// </summary>
@@ -249,7 +260,7 @@ namespace Content.Server.GameObjects.EntitySystems.Pathfinding
/// <param name="entity"></param>
/// TODO: These 2 methods currently don't account for a bunch of changes (e.g. airlock unpowered, wrenching, etc.)
/// TODO: Could probably optimise this slightly more.
public void AddEntity(IEntity entity)
public void AddEntity(IEntity entity, ICollidableComponent collidableComponent)
{
// If we're a door
if (entity.HasComponent<AirlockComponent>() || entity.HasComponent<ServerDoorComponent>())
@@ -258,29 +269,27 @@ namespace Content.Server.GameObjects.EntitySystems.Pathfinding
// TODO: Check for powered I think (also need an event for when it's depowered
// AccessReader calls this whenever opening / closing but it can seem to get called multiple times
// Which may or may not be intended?
if (entity.TryGetComponent(out AccessReader accessReader) && !_accessReaders.ContainsKey(entity.Uid))
if (entity.TryGetComponent(out AccessReader accessReader) && !_accessReaders.ContainsKey(entity))
{
_accessReaders.Add(entity.Uid, accessReader);
_accessReaders.Add(entity, accessReader);
ParentChunk.Dirty();
}
return;
}
if (entity.TryGetComponent(out ICollidableComponent collidableComponent) &&
(PathfindingSystem.TrackedCollisionLayers & collidableComponent.CollisionLayer) != 0)
DebugTools.Assert((PathfindingSystem.TrackedCollisionLayers & collidableComponent.CollisionLayer) != 0);
if (!collidableComponent.Anchored)
{
if (entity.TryGetComponent(out IPhysicsComponent physicsComponent) && !physicsComponent.Anchored)
{
_physicsLayers.Add(entity.Uid, collidableComponent.CollisionLayer);
_physicsLayers.Add(entity, collidableComponent.CollisionLayer);
}
else
{
_blockedCollidables.TryAdd(entity.Uid, collidableComponent.CollisionLayer);
_blockedCollidables.Add(entity, collidableComponent.CollisionLayer);
GenerateMask();
ParentChunk.Dirty();
}
}
}
/// <summary>
/// Remove the entity from this node.
@@ -292,18 +301,18 @@ namespace Content.Server.GameObjects.EntitySystems.Pathfinding
// There's no guarantee that the entity isn't deleted
// 90% of updates are probably entities moving around
// Entity can't be under multiple categories so just checking each once is fine.
if (_physicsLayers.ContainsKey(entity.Uid))
if (_physicsLayers.ContainsKey(entity))
{
_physicsLayers.Remove(entity.Uid);
_physicsLayers.Remove(entity);
}
else if (_accessReaders.ContainsKey(entity.Uid))
else if (_accessReaders.ContainsKey(entity))
{
_accessReaders.Remove(entity.Uid);
_accessReaders.Remove(entity);
ParentChunk.Dirty();
}
else if (_blockedCollidables.ContainsKey(entity.Uid))
else if (_blockedCollidables.ContainsKey(entity))
{
_blockedCollidables.Remove(entity.Uid);
_blockedCollidables.Remove(entity);
GenerateMask();
ParentChunk.Dirty();
}

View File

@@ -1,7 +1,9 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Threading;
using Content.Server.GameObjects.Components.Access;
using Content.Server.GameObjects.Components.GUI;
using Content.Server.GameObjects.EntitySystems.AI.Pathfinding.Pathfinders;
using Content.Server.GameObjects.EntitySystems.JobQueues;
using Content.Server.GameObjects.EntitySystems.JobQueues.Queues;
@@ -20,9 +22,10 @@ using Robust.Shared.Utility;
namespace Content.Server.GameObjects.EntitySystems.AI.Pathfinding
{
/*
// TODO: IMO use rectangular symmetry reduction on the nodes with collision at all., or
// TODO: IMO use rectangular symmetry reduction on the nodes with collision at all. (currently planned to be implemented via AiReachableSystem and expanded later).
alternatively store all rooms and have an alternative graph for humanoid mobs (same collision mask, needs access etc). You could also just path from room to room as needed.
// TODO: Longer term -> Handle collision layer changes?
TODO: Handle container entities so they're not tracked.
*/
/// <summary>
/// This system handles pathfinding graph updates as well as dispatches to the pathfinder
@@ -30,10 +33,7 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Pathfinding
/// </summary>
public class PathfindingSystem : EntitySystem
{
#pragma warning disable 649
[Dependency] private readonly IEntityManager _entitymanager;
[Dependency] private readonly IMapManager _mapManager;
#pragma warning restore 649
[Dependency] private readonly IMapManager _mapManager = default!;
public IReadOnlyDictionary<GridId, Dictionary<MapIndices, PathfindingChunk>> Graph => _graph;
private readonly Dictionary<GridId, Dictionary<MapIndices, PathfindingChunk>> _graph = new Dictionary<GridId, Dictionary<MapIndices, PathfindingChunk>>();
@@ -47,7 +47,7 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Pathfinding
private readonly Queue<TileRef> _tileUpdateQueue = new Queue<TileRef>();
// Need to store previously known entity positions for collidables for when they move
private readonly Dictionary<EntityUid, PathfindingNode> _lastKnownPositions = new Dictionary<EntityUid, PathfindingNode>();
private readonly Dictionary<IEntity, PathfindingNode> _lastKnownPositions = new Dictionary<IEntity, PathfindingNode>();
public const int TrackedCollisionLayers = (int)
(CollisionGroup.Impassable |
@@ -86,7 +86,7 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Pathfinding
foreach (var update in _collidableUpdateQueue)
{
var entity = _entitymanager.GetEntity(update.Owner);
var entity = EntityManager.GetEntity(update.Owner);
if (update.CanCollide)
{
HandleEntityAdd(entity);
@@ -103,14 +103,13 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Pathfinding
foreach (var update in _accessReaderUpdateQueue)
{
var entity = _entitymanager.GetEntity(update.Uid);
if (update.Enabled)
{
HandleEntityAdd(entity);
HandleEntityAdd(update.Sender);
}
else
{
HandleEntityRemove(entity);
HandleEntityRemove(update.Sender);
}
totalUpdates++;
@@ -138,7 +137,7 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Pathfinding
for (var i = 0; i < moveUpdateCount; i++)
{
HandleCollidableMove(_moveUpdateQueue.Dequeue());
HandleEntityMove(_moveUpdateQueue.Dequeue());
}
DebugTools.Assert(_moveUpdateQueue.Count < 1000);
@@ -204,9 +203,6 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Pathfinding
public override void Initialize()
{
// TODO: Remove this once the memory leaks are solved.
return;
SubscribeLocalEvent<CollisionChangeMessage>(QueueCollisionChangeMessage);
SubscribeLocalEvent<MoveEvent>(QueueMoveEvent);
SubscribeLocalEvent<AccessReaderChangeMessage>(QueueAccessChangeMessage);
@@ -279,7 +275,10 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Pathfinding
/// <param name="entity"></param>
private void HandleEntityAdd(IEntity entity)
{
if (entity.Deleted || _lastKnownPositions.ContainsKey(entity.Uid))
if (entity.Deleted ||
_lastKnownPositions.ContainsKey(entity) ||
!entity.TryGetComponent(out ICollidableComponent collidableComponent) ||
!PathfindingNode.IsRelevant(entity, collidableComponent))
{
return;
}
@@ -289,19 +288,19 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Pathfinding
var chunk = GetChunk(tileRef);
var node = chunk.GetNode(tileRef);
node.AddEntity(entity);
_lastKnownPositions.Add(entity.Uid, node);
node.AddEntity(entity, collidableComponent);
_lastKnownPositions.Add(entity, node);
}
private void HandleEntityRemove(IEntity entity)
{
if (!_lastKnownPositions.TryGetValue(entity.Uid, out var node))
if (!_lastKnownPositions.TryGetValue(entity, out var node))
{
return;
}
node.RemoveEntity(entity);
_lastKnownPositions.Remove(entity.Uid);
_lastKnownPositions.Remove(entity);
}
private void QueueMoveEvent(MoveEvent moveEvent)
@@ -313,23 +312,35 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Pathfinding
/// When an entity moves around we'll remove it from its old node and add it to its new node (if applicable)
/// </summary>
/// <param name="moveEvent"></param>
private void HandleCollidableMove(MoveEvent moveEvent)
private void HandleEntityMove(MoveEvent moveEvent)
{
var entityUid = moveEvent.Sender.Uid;
if (!_lastKnownPositions.TryGetValue(entityUid, out var oldNode))
{
return;
}
// The pathfinding graph is tile-based so first we'll check if they're on a different tile and if we need to update.
// If you get entities bigger than 1 tile wide you'll need some other system so god help you.
if (moveEvent.Sender.Deleted)
// If we've moved to space or the likes then remove us.
if (moveEvent.Sender.Deleted ||
!moveEvent.Sender.TryGetComponent(out ICollidableComponent collidableComponent) ||
!PathfindingNode.IsRelevant(moveEvent.Sender, collidableComponent))
{
HandleEntityRemove(moveEvent.Sender);
return;
}
// Memory leak protection until grid parenting confirmed fix / you REALLY need the performance
var gridBounds = _mapManager.GetGrid(moveEvent.Sender.Transform.GridID).WorldBounds;
if (!gridBounds.Contains(moveEvent.Sender.Transform.WorldPosition))
{
HandleEntityRemove(moveEvent.Sender);
return;
}
// If we move from space to a grid we may need to start tracking it.
if (!_lastKnownPositions.TryGetValue(moveEvent.Sender, out var oldNode))
{
HandleEntityAdd(moveEvent.Sender);
return;
}
// The pathfinding graph is tile-based so first we'll check if they're on a different tile and if we need to update.
// If you get entities bigger than 1 tile wide you'll need some other system so god help you.
var newTile = _mapManager.GetGrid(moveEvent.NewPosition.GridID).GetTileRef(moveEvent.NewPosition);
if (oldNode == null || oldNode.TileRef == newTile)
@@ -338,10 +349,10 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Pathfinding
}
var newNode = GetNode(newTile);
_lastKnownPositions[entityUid] = newNode;
_lastKnownPositions[moveEvent.Sender] = newNode;
oldNode.RemoveEntity(moveEvent.Sender);
newNode.AddEntity(moveEvent.Sender);
newNode.AddEntity(moveEvent.Sender, collidableComponent);
}
private void QueueCollisionChangeMessage(CollisionChangeMessage collisionMessage)

View File

@@ -320,7 +320,7 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Steering
return SteeringStatus.Pending;
}
var ignoredCollision = new List<EntityUid>();
var ignoredCollision = new List<IEntity>();
// Check if the target entity has moved - If so then re-path
// TODO: Patch the path from the target's position back towards us, stopping if it ever intersects the current path
// Probably need a separate "PatchPath" job
@@ -339,7 +339,7 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Steering
RequestPath(entity, steeringRequest);
}
ignoredCollision.Add(entitySteer.Target.Uid);
ignoredCollision.Add(entitySteer.Target);
}
HandleStuck(entity);
@@ -597,7 +597,7 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Steering
/// <param name="direction">entity's travel direction</param>
/// <param name="ignoredTargets"></param>
/// <returns></returns>
private Vector2 CollisionAvoidance(IEntity entity, Vector2 direction, ICollection<EntityUid> ignoredTargets)
private Vector2 CollisionAvoidance(IEntity entity, Vector2 direction, ICollection<IEntity> ignoredTargets)
{
if (direction == Vector2.Zero || !entity.TryGetComponent(out ICollidableComponent collidableComponent))
{
@@ -627,26 +627,25 @@ namespace Content.Server.GameObjects.EntitySystems.AI.Steering
{
var node = _pathfindingSystem.GetNode(tile);
// Assume the immovables have already been checked
foreach (var (uid, layer) in node.PhysicsLayers)
foreach (var (physicsEntity, layer) in node.PhysicsLayers)
{
// Ignore myself / my target if applicable / if my mask doesn't collide
if (uid == entity.Uid || ignoredTargets.Contains(uid) || (entityCollisionMask & layer) == 0) continue;
if (physicsEntity == entity || ignoredTargets.Contains(physicsEntity) || (entityCollisionMask & layer) == 0) continue;
// God there's so many ways to do this
// err for now we'll just assume the first entity is the center and just add a vector for it
var collisionEntity = _entityManager.GetEntity(uid);
//Pathfinding updates are deferred so this may not be done yet.
if (collisionEntity.Deleted) continue;
if (physicsEntity.Deleted) continue;
// if we're moving in the same direction then ignore
// So if 2 entities are moving towards each other and both detect a collision they'll both move in the same direction
// i.e. towards the right
if (collisionEntity.TryGetComponent(out IPhysicsComponent physicsComponent) &&
if (physicsEntity.TryGetComponent(out IPhysicsComponent physicsComponent) &&
Vector2.Dot(physicsComponent.LinearVelocity, direction) > 0)
{
continue;
}
var centerGrid = collisionEntity.Transform.GridPosition;
var centerGrid = physicsEntity.Transform.GridPosition;
// Check how close we are to center of tile and get the inverse; if we're closer this is stronger
var additionalVector = (centerGrid.Position - entityGridCoords.Position);
var distance = additionalVector.Length;