space-wizards/RobustToolbox
1
0
Fork 0
mirror of https://github.com/space-wizards/RobustToolbox.git synced 2026-02-14 19:29:36 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix terminating entity reparenting bug (#4549)

Browse Source
This commit is contained in:
Leon Friedrich
2023-11-08 15:39:08 +11:00
committed by GitHub
parent eecf834039
commit 773b87672b
9 changed files with 127 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
Robust.Shared/Containers/BaseContainer.cs
View File

@@ -125,16 +125,23 @@ namespace Robust.Shared.Containers
return false;
}
transform ??= transformQuery.GetComponent(toinsert);
//Verify we can insert into this container
if (!force && !containerSys.CanInsert(toinsert, this))
if (!force && !containerSys.CanInsert(toinsert, this, containerXform: ownerTransform))
return false;
// Please somebody ecs containers
var lookupSys = entMan.EntitySysManager.GetEntitySystem<EntityLookupSystem>();
var xformSys = entMan.EntitySysManager.GetEntitySystem<SharedTransformSystem>();
transform ??= transformQuery.GetComponent(toinsert);
meta ??= entMan.GetComponent<MetaDataComponent>(toinsert);
if (meta.EntityLifeStage >= EntityLifeStage.Terminating)
{
Logger.ErrorS("container",
$"Attempted to insert a terminating entity {entMan.ToPrettyString(toinsert)} into a container {ID} in entity: {entMan.ToPrettyString(Owner)}.");
return false;
}
// remove from any old containers.
if ((meta.Flags & MetaDataFlags.InContainer) != 0 &&

9
Robust.Shared/Containers/SharedContainerSystem.Insert.cs
View File

@@ -44,8 +44,8 @@ public abstract partial class SharedContainerSystem
public bool CanInsert(
EntityUid toInsert,
BaseContainer container,
TransformComponent? toInsertXform = null,
bool assumeEmpty = false)
bool assumeEmpty = false,
TransformComponent? containerXform = null)
{
if (container.Owner == toInsert)
return false;
@@ -56,15 +56,12 @@ public abstract partial class SharedContainerSystem
if (!container.CanInsert(toInsert, assumeEmpty, EntityManager))
return false;
if (!TransformQuery.Resolve(toInsert, ref toInsertXform))
return false;
// no, you can't put maps or grids into containers
if (_mapQuery.HasComponent(toInsert) || _gridQuery.HasComponent(toInsert))
return false;
// Prevent circular insertion.
if (_transform.ContainsEntity(toInsertXform, container.Owner))
if (_transform.ContainsEntity(toInsert, (container.Owner, containerXform)))
return false;
var insertAttemptEvent = new ContainerIsInsertingAttemptEvent(container, toInsert, assumeEmpty);

22
Robust.Shared/Containers/SharedContainerSystem.cs
View File

@@ -216,35 +216,27 @@ namespace Robust.Shared.Containers
}
/// <summary>
/// Recursively if the entity, or any parent entity, is inside of a container.
/// Recursively check if the entity or any parent is inside of a container.
/// </summary>
/// <returns>If the entity is inside of a container.</returns>
public bool IsEntityOrParentInContainer(
EntityUid uid,
MetaDataComponent? meta = null,
TransformComponent? xform = null,
EntityQuery<MetaDataComponent>? metas = null,
EntityQuery<TransformComponent>? xforms = null)
TransformComponent? xform = null)
{
if (meta == null)
{
metas ??= GetEntityQuery<MetaDataComponent>();
meta = metas.Value.GetComponent(uid);
}
if (!MetaQuery.Resolve(uid, ref meta))
return false;
if ((meta.Flags & MetaDataFlags.InContainer) == MetaDataFlags.InContainer)
return true;
if (xform == null)
{
xforms ??= GetEntityQuery<TransformComponent>();
xform = xforms.Value.GetComponent(uid);
}
if (!TransformQuery.Resolve(uid, ref xform))
return false;
if (!xform.ParentUid.Valid)
return false;
return IsEntityOrParentInContainer(xform.ParentUid, metas: metas, xforms: xforms);
return IsEntityOrParentInContainer(xform.ParentUid);
}
/// <summary>

9
Robust.Shared/GameObjects/EntitySystem.Proxy.cs
View File

@@ -159,6 +159,15 @@ public partial class EntitySystem
EntityManager.Dirty(ent.Owner, ent.Comp, meta);
}
/// <summary>
/// Marks a component as dirty. This also implicitly dirties the entity this component belongs to.
/// </summary>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
protected void Dirty<T>(Entity<T, MetaDataComponent> ent) where T : IComponent
{
EntityManager.Dirty(ent.Owner, ent.Comp1, ent.Comp2);
}
/// <summary>
/// Retrieves the name of an entity.
/// </summary>

15
Robust.Shared/GameObjects/Systems/EntityLookup.Queries.cs
View File

@@ -171,11 +171,16 @@ public sealed partial class EntityLookupSystem
private void RecursiveAdd(EntityUid uid, ref ValueList<EntityUid> toAdd)
{
var childEnumerator = _xformQuery.GetComponent(uid).ChildEnumerator;
if (!_xformQuery.TryGetComponent(uid, out var xform))
{
Log.Error($"Encountered deleted entity {uid} while performing entity lookup.");
return;
}
toAdd.Add(uid);
var childEnumerator = xform.ChildEnumerator;
while (childEnumerator.MoveNext(out var child))
{
toAdd.Add(child.Value);
RecursiveAdd(child.Value, ref toAdd);
}
}
@@ -185,6 +190,11 @@ public sealed partial class EntityLookupSystem
if ((flags & LookupFlags.Contained) == 0x0 || intersecting.Count == 0)
return;
// TODO PERFORMANCE.
// toAdd only exists because we can't add directly to intersecting w/o enumeration issues.
// If we assume that there are more entities in containers than there are entities in the intersecting set, then
// we would be better off creating a fixed-size EntityUid array and coping all intersecting entities into that
// instead of creating a value list here that needs to be resized.
var toAdd = new ValueList<EntityUid>();
foreach (var uid in intersecting)
@@ -196,7 +206,6 @@ public sealed partial class EntityLookupSystem
{
foreach (var contained in con.ContainedEntities)
{
toAdd.Add(contained);
RecursiveAdd(contained, ref toAdd);
}
}

4
Robust.Shared/GameObjects/Systems/EntityLookupSystem.ComponentQueries.cs
View File

@@ -270,7 +270,7 @@ public sealed partial class EntityLookupSystem
if (xform.MapID != mapId ||
!worldAABB.Contains(_transform.GetWorldPosition(xform)) ||
((flags & LookupFlags.Contained) == 0x0 &&
_container.IsEntityOrParentInContainer(uid, _metaQuery.GetComponent(uid), xform, _metaQuery, _xformQuery)))
_container.IsEntityOrParentInContainer(uid, null, xform)))
{
continue;
}
@@ -332,7 +332,7 @@ public sealed partial class EntityLookupSystem
if (xform.MapID != mapId ||
!worldAABB.Contains(_transform.GetWorldPosition(xform)) ||
((flags & LookupFlags.Contained) == 0x0 &&
_container.IsEntityOrParentInContainer(uid, _metaQuery.GetComponent(uid), xform, _metaQuery, _xformQuery)))
_container.IsEntityOrParentInContainer(uid, _metaQuery.GetComponent(uid), xform)))
{
continue;
}

4
Robust.Shared/GameObjects/Systems/EntityLookupSystem.cs
View File

@@ -368,7 +368,7 @@ public sealed partial class EntityLookupSystem : EntitySystem
FixturesComponent manager,
EntityQuery<TransformComponent> xformQuery)
{
DebugTools.Assert(!_container.IsEntityOrParentInContainer(body.Owner, null, xform, null, xformQuery));
DebugTools.Assert(!_container.IsEntityOrParentInContainer(body.Owner, null, xform));
DebugTools.Assert(xform.Broadphase == null || xform.Broadphase == new BroadphaseData(broadphase.Owner, physicsMap.Owner, body.CanCollide, body.BodyType == BodyType.Static));
DebugTools.Assert(broadphase.Owner == broadUid);
@@ -843,7 +843,7 @@ public sealed partial class EntityLookupSystem : EntitySystem
TransformComponent xform,
[NotNullWhen(true)] out BroadphaseComponent? broadphase)
{
if (xform.MapID == MapId.Nullspace || _container.IsEntityOrParentInContainer(xform.Owner, null, xform, null, _xformQuery))
if (xform.MapID == MapId.Nullspace || _container.IsEntityOrParentInContainer(xform.Owner, null, xform))
{
broadphase = null;
return false;

132
Robust.Shared/GameObjects/Systems/SharedTransformSystem.Component.cs
View File

@@ -1,11 +1,8 @@
using JetBrains.Annotations;
using Robust.Shared.GameStates;
using Robust.Shared.Log;
using Robust.Shared.Map;
using Robust.Shared.Maths;
using Robust.Shared.Physics;
using Robust.Shared.Physics.Systems;
using Robust.Shared.Timing;
using Robust.Shared.Utility;
using System;
using System.Linq;
@@ -65,12 +62,7 @@ public abstract partial class SharedTransformSystem
RaiseLocalEvent(uid, ref ev);
}
[Obsolete("Use overload that takes an explicit EntityUid for the grid instead.")]
public bool AnchorEntity(EntityUid uid, TransformComponent xform, MapGridComponent grid, Vector2i tileIndices)
{
return AnchorEntity(uid, xform, grid.Owner, grid, tileIndices);
}
[Obsolete("Use Entity<T> variant")]
public bool AnchorEntity(
EntityUid uid,
TransformComponent xform,
@@ -78,12 +70,22 @@ public abstract partial class SharedTransformSystem
MapGridComponent grid,
Vector2i tileIndices)
{
if (!_map.AddToSnapGridCell(gridUid, grid, tileIndices, uid))
return AnchorEntity((uid, xform), (gridUid, grid), tileIndices);
}
public bool AnchorEntity(
Entity<TransformComponent> entity,
Entity<MapGridComponent> grid,
Vector2i tileIndices)
{
var (uid, xform) = entity;
if (!_map.AddToSnapGridCell(grid, grid, tileIndices, uid))
return false;
var wasAnchored = xform._anchored;
Dirty(uid, xform);
var wasAnchored = entity.Comp._anchored;
xform._anchored = true;
var meta = MetaData(uid);
Dirty(entity, meta);
// Mark as static before doing position changes, to avoid the velocity change on parent change.
_physics.TrySetBodyType(uid, BodyType.Static, xform: xform);
@@ -95,22 +97,36 @@ public abstract partial class SharedTransformSystem
}
// Anchor snapping. If there is a coordinate change, it will dirty the component for us.
var pos = new EntityCoordinates(gridUid, _map.GridTileToLocal(gridUid, grid, tileIndices).Position);
SetCoordinates(uid, xform, pos, unanchor: false);
var pos = new EntityCoordinates(grid, _map.GridTileToLocal(grid, grid, tileIndices).Position);
SetCoordinates((uid, xform, meta), pos, unanchor: false);
return true;
}
[Obsolete("Use Entity<T> variants")]
public bool AnchorEntity(EntityUid uid, TransformComponent xform, MapGridComponent grid)
{
var tileIndices = _map.TileIndicesFor(grid.Owner, grid, xform.Coordinates);
return AnchorEntity(uid, xform, grid, tileIndices);
return AnchorEntity(uid, xform, grid.Owner, grid, tileIndices);
}
public bool AnchorEntity(EntityUid uid, TransformComponent xform)
{
return _mapManager.TryGetGrid(xform.GridUid, out var grid)
&& AnchorEntity(uid, xform, grid, _map.TileIndicesFor(xform.GridUid.Value, grid, xform.Coordinates));
return AnchorEntity((uid, xform));
}
public bool AnchorEntity(Entity<TransformComponent> entity, Entity<MapGridComponent>? grid = null)
{
DebugTools.Assert(grid == null || grid.Value.Owner == entity.Comp.GridUid);
if (grid == null)
{
if (!TryComp(entity.Comp.GridUid, out MapGridComponent? gridComp))
return false;
grid = (entity.Comp.GridUid.Value, gridComp);
}
var tileIndices = _map.TileIndicesFor(grid.Value, grid.Value, entity.Comp.Coordinates);
return AnchorEntity(entity, grid.Value, tileIndices);
}
public void Unanchor(EntityUid uid, TransformComponent xform, bool setPhysics = true)
@@ -145,39 +161,23 @@ public abstract partial class SharedTransformSystem
#region Contains
/// <summary>
/// Returns whether the given entity is a child of this transform or one of its descendants.
/// Checks whether the first entity or one of it's children is the parent of some other entity.
/// </summary>
public bool ContainsEntity(TransformComponent xform, EntityUid entity)
public bool ContainsEntity(EntityUid parent, Entity<TransformComponent?> child)
{
return ContainsEntity(xform, entity, XformQuery);
}
/// <inheritdoc cref="ContainsEntity(Robust.Shared.GameObjects.TransformComponent,Robust.Shared.GameObjects.EntityUid)"/>
public bool ContainsEntity(TransformComponent xform, EntityUid entity, EntityQuery<TransformComponent> xformQuery)
{
return ContainsEntity(xform, xformQuery.GetComponent(entity), xformQuery);
}
/// <inheritdoc cref="ContainsEntity(Robust.Shared.GameObjects.TransformComponent,Robust.Shared.GameObjects.EntityUid)"/>
public bool ContainsEntity(TransformComponent xform, TransformComponent entityTransform)
{
return ContainsEntity(xform, entityTransform, XformQuery);
}
/// <inheritdoc cref="ContainsEntity(Robust.Shared.GameObjects.TransformComponent,Robust.Shared.GameObjects.EntityUid)"/>
public bool ContainsEntity(TransformComponent xform, TransformComponent entityTransform, EntityQuery<TransformComponent> xformQuery)
{
// Is the entity the scene root
if (!entityTransform.ParentUid.IsValid())
if (!Resolve(child.Owner, ref child.Comp))
return false;
// Is this the direct parent of the entity
if (xform.Owner == entityTransform.ParentUid)
if (!child.Comp.ParentUid.IsValid())
return false;
if (parent == child.Comp.ParentUid)
return true;
// Recursively search up the parents for this object
var parentXform = xformQuery.GetComponent(entityTransform.ParentUid);
return ContainsEntity(xform, parentXform, xformQuery);
if (!XformQuery.TryGetComponent(child.Comp.ParentUid, out var parentXform))
return false;
return ContainsEntity(parent, (child.Comp.ParentUid, parentXform));
}
#endregion
@@ -237,7 +237,7 @@ public abstract partial class SharedTransformSystem
{
var msg = $"Attempted to re-parent to a terminating object. Entity: {ToPrettyString(component.ParentUid)}, new parent: {ToPrettyString(uid)}";
#if EXCEPTION_TOLERANCE
Logger.Error(msg);
Log.Error(msg);
Del(uid);
#else
throw new InvalidOperationException(msg);
@@ -432,7 +432,7 @@ public abstract partial class SharedTransformSystem
public void SetCoordinates(EntityUid uid, EntityCoordinates value)
{
SetCoordinates(uid, Transform(uid), value);
SetCoordinates((uid, Transform(uid), MetaData(uid)), value);
}
/// <summary>
@@ -443,8 +443,15 @@ public abstract partial class SharedTransformSystem
/// <param name="unanchor">Whether or not to unanchor the entity before moving. Note that this will still move the
/// entity even when false. If you set this to false, you need to manually manage the grid lookup changes and ensure
/// the final position is valid</param>
public void SetCoordinates(EntityUid uid, TransformComponent xform, EntityCoordinates value, Angle? rotation = null, bool unanchor = true, TransformComponent? newParent = null, TransformComponent? oldParent = null)
public void SetCoordinates(
Entity<TransformComponent, MetaDataComponent> entity,
EntityCoordinates value,
Angle? rotation = null,
bool unanchor = true,
TransformComponent? newParent = null,
TransformComponent? oldParent = null)
{
var (uid, xform, meta) = entity;
// NOTE: This setter must be callable from before initialize.
if (xform.ParentUid == value.EntityId
@@ -460,8 +467,23 @@ public abstract partial class SharedTransformSystem
if (xform.Anchored && unanchor)
Unanchor(uid, xform);
if (value.EntityId != xform.ParentUid && value.EntityId.IsValid())
{
if (meta.EntityLifeStage >= EntityLifeStage.Terminating)
{
Log.Error($"{ToPrettyString(uid)} is attempting to move while terminating. New parent: {ToPrettyString(value.EntityId)}. Trace: {Environment.StackTrace}");
return;
}
if (TerminatingOrDeleted(value.EntityId))
{
Log.Error($"{ToPrettyString(uid)} is attempting to attach itself to a terminating entity {ToPrettyString(value.EntityId)}. Trace: {Environment.StackTrace}");
return;
}
}
// Set new values
Dirty(uid, xform);
Dirty(uid, xform, meta);
xform.MatricesDirty = true;
xform._localPosition = value.Position;
@@ -583,6 +605,18 @@ public abstract partial class SharedTransformSystem
RaiseLocalEvent(uid, ref moveEvent, true);
}
public void SetCoordinates(
EntityUid uid,
TransformComponent xform,
EntityCoordinates value,
Angle? rotation = null,
bool unanchor = true,
TransformComponent? newParent = null,
TransformComponent? oldParent = null)
{
SetCoordinates((uid, xform, MetaData(uid)), value, rotation, unanchor, newParent, oldParent);
}
#endregion
#region Parent

15
Robust.Shared/GameObjects/Systems/SharedTransformSystem.cs
View File

@@ -49,21 +49,6 @@ namespace Robust.Shared.GameObjects
SubscribeLocalEvent<TransformComponent, ComponentGetState>(OnGetState);
SubscribeLocalEvent<TransformComponent, ComponentHandleState>(OnHandleState);
SubscribeLocalEvent<TransformComponent, GridAddEvent>(OnGridAdd);
SubscribeLocalEvent<EntParentChangedMessage>(OnParentChange);
}
private void OnParentChange(ref EntParentChangedMessage ev)
{
// TODO: when PVS errors on live servers get fixed, wrap this whole subscription in an #if DEBUG block to speed up parent changes & entity deletion.
if (ev.Transform.ParentUid == EntityUid.Invalid)
return;
if (LifeStage(ev.Entity) >= EntityLifeStage.Terminating)
Log.Error($"Entity {ToPrettyString(ev.Entity)} is getting attached to a new parent while terminating. New parent: {ToPrettyString(ev.Transform.ParentUid)}. Trace: {Environment.StackTrace}");
if (LifeStage(ev.Transform.ParentUid) >= EntityLifeStage.Terminating)
Log.Error($"Entity {ToPrettyString(ev.Entity)} is attaching itself to a terminating entity {ToPrettyString(ev.Transform.ParentUid)}. Trace: {Environment.StackTrace}");
}
private void MapManagerOnTileChanged(ref TileChangedEvent e)