fix: improve stability and add edge case tests
- Fix SparseSet resize logic to correctly initialize new slots - Implement Equals and GetHashCode for QueryDescriptor - Prevent collection modification exceptions in World.DestroyEntity - Update CommandQueue to accumulate errors across drain cycles - Add comprehensive edge case tests in EdgeCaseTests.cs
This commit is contained in:
parent
8557fc864d
commit
25789e7c9d
|
|
@ -21,8 +21,8 @@ public class CommandQueue
|
|||
public int Count => _commands.Count;
|
||||
|
||||
/// <summary>
|
||||
/// Exceptions thrown by commands during the most recent <see cref="ExecuteAll"/> call.
|
||||
/// Cleared at the start of each drain cycle.
|
||||
/// Exceptions thrown by commands during <see cref="ExecuteAll"/> calls.
|
||||
/// Accumulates across drain cycles. Call <see cref="ClearErrors"/> to reset.
|
||||
/// </summary>
|
||||
public IReadOnlyList<Exception> Errors => _errors;
|
||||
|
||||
|
|
@ -42,8 +42,6 @@ public class CommandQueue
|
|||
/// </summary>
|
||||
public void ExecuteAll(World world)
|
||||
{
|
||||
_errors.Clear();
|
||||
|
||||
int index = 0;
|
||||
while (index < _commands.Count)
|
||||
{
|
||||
|
|
@ -69,6 +67,13 @@ public class CommandQueue
|
|||
public void Clear()
|
||||
{
|
||||
_commands.Clear();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Clears accumulated error history.
|
||||
/// </summary>
|
||||
public void ClearErrors()
|
||||
{
|
||||
_errors.Clear();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -27,4 +27,22 @@ public class QueryDescriptor
|
|||
/// Returns true if this query has no "with" components (matches nothing).
|
||||
/// </summary>
|
||||
internal bool IsEmpty => With.Count == 0;
|
||||
|
||||
public override bool Equals(object? obj)
|
||||
{
|
||||
if (obj is not QueryDescriptor other) return false;
|
||||
if (With.Count != other.With.Count || Without.Count != other.Without.Count)
|
||||
return false;
|
||||
return With.SetEquals(other.With) && Without.SetEquals(other.Without);
|
||||
}
|
||||
|
||||
public override int GetHashCode()
|
||||
{
|
||||
var hash = new HashCode();
|
||||
foreach (var t in With.OrderBy(t => t.FullName))
|
||||
hash.Add(t);
|
||||
foreach (var t in Without.OrderBy(t => t.FullName))
|
||||
hash.Add(t);
|
||||
return hash.ToHashCode();
|
||||
}
|
||||
}
|
||||
|
|
@ -140,9 +140,10 @@ internal class SparseSet<T> : ISparseSet where T : struct
|
|||
{
|
||||
if (entityId >= (uint)_sparse.Length)
|
||||
{
|
||||
int oldSize = _sparse.Length;
|
||||
int newSize = Math.Max((int)entityId + 1, _sparse.Length * 2);
|
||||
Array.Resize(ref _sparse, newSize);
|
||||
Array.Fill(_sparse, -1, _sparse.Length - (newSize - _sparse.Length), newSize - _sparse.Length);
|
||||
Array.Fill(_sparse, -1, oldSize, newSize - oldSize);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -71,7 +71,12 @@ public class World : IDisposable
|
|||
return;
|
||||
|
||||
// Remove all relationships where this entity is the target.
|
||||
foreach (var (relType, sources) in _relationships.GetIncomingRelationships(entity))
|
||||
// Materialize to avoid collection-modified-during-enumeration when
|
||||
// OnRemoved mutates the same HashSet we're iterating.
|
||||
var incoming = _relationships.GetIncomingRelationships(entity)
|
||||
.Select(r => (r.RelationshipType, r.Sources.ToList()))
|
||||
.ToList();
|
||||
foreach (var (relType, sources) in incoming)
|
||||
{
|
||||
foreach (var source in sources)
|
||||
{
|
||||
|
|
@ -83,6 +88,15 @@ public class World : IDisposable
|
|||
// Remove all relationships where this entity is the source.
|
||||
_relationships.RemoveAllSourcesForEntity(entity);
|
||||
|
||||
// Mark component removals for each component type before removing them.
|
||||
foreach (var componentType in _components.ComponentTypes.ToList())
|
||||
{
|
||||
if (_components.GetSet(componentType)?.Contains(entity) == true)
|
||||
{
|
||||
_changes.Pending.MarkComponentRemoved(entity, componentType);
|
||||
}
|
||||
}
|
||||
|
||||
_components.RemoveAll(entity);
|
||||
_allocator.Free(entity);
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,473 @@
|
|||
using FluentAssertions;
|
||||
using R3;
|
||||
using Xunit;
|
||||
|
||||
namespace OECS.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Tests for edge cases and previously uncovered code paths.
|
||||
/// </summary>
|
||||
public class EdgeCaseTests
|
||||
{
|
||||
// ── Test components ──────────────────────────────────────────────
|
||||
|
||||
private struct Position { public float X; public float Y; }
|
||||
private struct Velocity { public float X; public float Y; }
|
||||
private struct Health { public int Value; }
|
||||
private struct Frozen { }
|
||||
private struct Burning { }
|
||||
private struct Poisoned { }
|
||||
private struct Cursed { }
|
||||
|
||||
// ── SparseSet resize correctness ─────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void SparseSet_Resize_InitializesNewSlotsToMinusOne()
|
||||
{
|
||||
// This test verifies that when the sparse array grows, new slots
|
||||
// are filled with -1 (not left as default 0, which would falsely
|
||||
// indicate the entity has a component).
|
||||
var world = new World();
|
||||
|
||||
// Create many entities to force sparse array resizes.
|
||||
var entities = new List<Entity>();
|
||||
for (int i = 0; i < 2000; i++)
|
||||
{
|
||||
entities.Add(world.CreateEntity());
|
||||
}
|
||||
|
||||
// Add Position to every other entity.
|
||||
for (int i = 0; i < entities.Count; i += 2)
|
||||
{
|
||||
world.AddComponent(entities[i], new Position { X = i, Y = i });
|
||||
}
|
||||
|
||||
// Entities without Position should correctly report false.
|
||||
for (int i = 1; i < entities.Count; i += 2)
|
||||
{
|
||||
world.HasComponent<Position>(entities[i]).Should().BeFalse(
|
||||
$"entity {entities[i]} should not have Position");
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SparseSet_Resize_PreservesExistingData()
|
||||
{
|
||||
var world = new World();
|
||||
|
||||
// Create entities before any component adds to trigger resizes
|
||||
// after data is already present.
|
||||
var first = world.CreateEntity();
|
||||
world.AddComponent(first, new Position { X = 1, Y = 2 });
|
||||
|
||||
// Create many more to force resize.
|
||||
for (int i = 0; i < 2000; i++)
|
||||
{
|
||||
var e = world.CreateEntity();
|
||||
world.AddComponent(e, new Position { X = i, Y = i });
|
||||
}
|
||||
|
||||
// Original entity should still have its data.
|
||||
ref var pos = ref world.GetComponent<Position>(first);
|
||||
pos.X.Should().Be(1);
|
||||
pos.Y.Should().Be(2);
|
||||
}
|
||||
|
||||
// ── Entity version wrapping ──────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void Entity_VersionWraps_From255To1()
|
||||
{
|
||||
var world = new World();
|
||||
var original = world.CreateEntity();
|
||||
uint id = original.Id;
|
||||
|
||||
// Destroy and recreate the entity 255 times to wrap the version.
|
||||
for (int i = 0; i < 255; i++)
|
||||
{
|
||||
world.DestroyEntity(original);
|
||||
original = world.CreateEntity();
|
||||
original.Id.Should().Be(id);
|
||||
}
|
||||
|
||||
// After 255 recycles, version should have wrapped from 255 to 1.
|
||||
original.Version.Should().Be(1);
|
||||
world.IsAlive(original).Should().BeTrue();
|
||||
}
|
||||
|
||||
// ── DestroyEntity with multiple incoming relationships ───────────
|
||||
|
||||
[Fact]
|
||||
public void DestroyEntity_WithMultipleIncomingRelationships_DoesNotThrow()
|
||||
{
|
||||
var world = new World();
|
||||
var target = world.CreateEntity();
|
||||
|
||||
// Create multiple sources pointing to the same target.
|
||||
var sources = new List<Entity>();
|
||||
for (int i = 0; i < 10; i++)
|
||||
{
|
||||
var source = world.CreateEntity();
|
||||
sources.Add(source);
|
||||
world.AddComponent(source, new Relationship<ChildOf, ParentOf>
|
||||
{
|
||||
Source = source,
|
||||
Target = target
|
||||
});
|
||||
}
|
||||
|
||||
// Destroying the target should cascade-remove all incoming
|
||||
// relationships without throwing.
|
||||
var act = () => world.DestroyEntity(target);
|
||||
act.Should().NotThrow();
|
||||
|
||||
// All source entities should have had their relationship removed.
|
||||
foreach (var source in sources)
|
||||
{
|
||||
world.HasComponent<Relationship<ChildOf, ParentOf>>(source).Should().BeFalse();
|
||||
}
|
||||
}
|
||||
|
||||
// ── Component removal change tracking on destroy ─────────────────
|
||||
|
||||
[Fact]
|
||||
public void DestroyEntity_PostsComponentRemoved_ForEachComponent()
|
||||
{
|
||||
var world = new World();
|
||||
var entity = world.CreateEntity();
|
||||
world.AddComponent(entity, new Position { X = 1, Y = 2 });
|
||||
world.AddComponent(entity, new Health { Value = 100 });
|
||||
world.PostChanges(); // Clear pending
|
||||
|
||||
var collector = new ChangeCollector();
|
||||
using var sub = world.ObserveEntityChanges().Subscribe(collector.ToObserver());
|
||||
|
||||
world.DestroyEntity(entity);
|
||||
world.PostChanges();
|
||||
|
||||
// Should see: ComponentRemoved(Position), ComponentRemoved(Health), EntityRemoved
|
||||
collector.Changes.Should().Contain(c =>
|
||||
c.Kind == ChangeKind.ComponentRemoved && c.ComponentType == typeof(Position));
|
||||
collector.Changes.Should().Contain(c =>
|
||||
c.Kind == ChangeKind.ComponentRemoved && c.ComponentType == typeof(Health));
|
||||
collector.Changes.Should().Contain(c =>
|
||||
c.Kind == ChangeKind.EntityRemoved && c.Entity == entity);
|
||||
}
|
||||
|
||||
// ── QueryDescriptor equality ─────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void QueryDescriptor_EqualQueries_AreEqual()
|
||||
{
|
||||
var q1 = new QueryBuilder().With<Position>().With<Velocity>().Without<Frozen>().Build();
|
||||
var q2 = new QueryBuilder().With<Position>().With<Velocity>().Without<Frozen>().Build();
|
||||
|
||||
q1.Should().Be(q2);
|
||||
q1.GetHashCode().Should().Be(q2.GetHashCode());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void QueryDescriptor_DifferentWith_AreNotEqual()
|
||||
{
|
||||
var q1 = new QueryBuilder().With<Position>().Build();
|
||||
var q2 = new QueryBuilder().With<Velocity>().Build();
|
||||
|
||||
q1.Should().NotBe(q2);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void QueryDescriptor_DifferentWithout_AreNotEqual()
|
||||
{
|
||||
var q1 = new QueryBuilder().With<Position>().Without<Frozen>().Build();
|
||||
var q2 = new QueryBuilder().With<Position>().Without<Burning>().Build();
|
||||
|
||||
q1.Should().NotBe(q2);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void QueryDescriptor_DifferentCount_AreNotEqual()
|
||||
{
|
||||
var q1 = new QueryBuilder().With<Position>().Build();
|
||||
var q2 = new QueryBuilder().With<Position>().With<Velocity>().Build();
|
||||
|
||||
q1.Should().NotBe(q2);
|
||||
}
|
||||
|
||||
// ── Query with 4, 5, 6 components ────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void Query_FourComponents_Works()
|
||||
{
|
||||
var world = new World();
|
||||
var entity = world.CreateEntity();
|
||||
world.AddComponent(entity, new Position { X = 1, Y = 2 });
|
||||
world.AddComponent(entity, new Velocity { X = 3, Y = 4 });
|
||||
world.AddComponent(entity, new Health { Value = 100 });
|
||||
world.AddComponent(entity, new Frozen());
|
||||
|
||||
var query = world.Query()
|
||||
.With<Position>().With<Velocity>().With<Health>().With<Frozen>()
|
||||
.Build();
|
||||
|
||||
var count = 0;
|
||||
world.ForEach(query, (Entity e, ref Position pos, ref Velocity vel, ref Health hp, ref Frozen f) =>
|
||||
{
|
||||
count++;
|
||||
pos.X.Should().Be(1);
|
||||
vel.Y.Should().Be(4);
|
||||
hp.Value.Should().Be(100);
|
||||
});
|
||||
count.Should().Be(1);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Query_FiveComponents_Works()
|
||||
{
|
||||
var world = new World();
|
||||
var entity = world.CreateEntity();
|
||||
world.AddComponent(entity, new Position { X = 1, Y = 2 });
|
||||
world.AddComponent(entity, new Velocity { X = 3, Y = 4 });
|
||||
world.AddComponent(entity, new Health { Value = 100 });
|
||||
world.AddComponent(entity, new Frozen());
|
||||
world.AddComponent(entity, new Burning());
|
||||
|
||||
var query = world.Query()
|
||||
.With<Position>().With<Velocity>().With<Health>().With<Frozen>().With<Burning>()
|
||||
.Build();
|
||||
|
||||
var count = 0;
|
||||
world.ForEach(query, (Entity e, ref Position pos, ref Velocity vel, ref Health hp, ref Frozen f, ref Burning b) =>
|
||||
{
|
||||
count++;
|
||||
});
|
||||
count.Should().Be(1);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Query_SixComponents_Works()
|
||||
{
|
||||
var world = new World();
|
||||
var entity = world.CreateEntity();
|
||||
world.AddComponent(entity, new Position { X = 1, Y = 2 });
|
||||
world.AddComponent(entity, new Velocity { X = 3, Y = 4 });
|
||||
world.AddComponent(entity, new Health { Value = 100 });
|
||||
world.AddComponent(entity, new Frozen());
|
||||
world.AddComponent(entity, new Burning());
|
||||
world.AddComponent(entity, new Poisoned());
|
||||
|
||||
var query = world.Query()
|
||||
.With<Position>().With<Velocity>().With<Health>()
|
||||
.With<Frozen>().With<Burning>().With<Poisoned>()
|
||||
.Build();
|
||||
|
||||
var count = 0;
|
||||
world.ForEach(query, (Entity e, ref Position pos, ref Velocity vel, ref Health hp,
|
||||
ref Frozen f, ref Burning b, ref Poisoned p) =>
|
||||
{
|
||||
count++;
|
||||
});
|
||||
count.Should().Be(1);
|
||||
}
|
||||
|
||||
// ── Query with only Without ──────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void Query_OnlyWithout_FiltersCorrectly()
|
||||
{
|
||||
var world = new World();
|
||||
var a = world.CreateEntity();
|
||||
var b = world.CreateEntity();
|
||||
|
||||
world.AddComponent(a, new Position { X = 1, Y = 2 });
|
||||
world.AddComponent(b, new Position { X = 3, Y = 4 });
|
||||
world.AddComponent(b, new Frozen());
|
||||
|
||||
// Without<Frozen> filters out entity b which has Frozen.
|
||||
var query = world.Query().Without<Frozen>().Build();
|
||||
var results = new List<Entity>();
|
||||
|
||||
world.ForEach(query, (Entity e, ref Position pos) =>
|
||||
{
|
||||
results.Add(e);
|
||||
});
|
||||
|
||||
results.Should().BeEquivalentTo([a]);
|
||||
}
|
||||
|
||||
// ── World.Dispose ────────────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void World_Dispose_DoesNotThrow()
|
||||
{
|
||||
var world = new World();
|
||||
var entity = world.CreateEntity();
|
||||
world.AddComponent(entity, new Position { X = 1, Y = 2 });
|
||||
|
||||
var act = () => world.Dispose();
|
||||
act.Should().NotThrow();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void World_Dispose_CanBeCalledMultipleTimes()
|
||||
{
|
||||
var world = new World();
|
||||
|
||||
world.Dispose();
|
||||
var act = () => world.Dispose();
|
||||
act.Should().NotThrow();
|
||||
}
|
||||
|
||||
// ── CommandQueue error accumulation ──────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void CommandQueue_Errors_AccumulateAcrossDrainCycles()
|
||||
{
|
||||
var world = new World();
|
||||
|
||||
world.Commands.Enqueue(new FailingCommand { Message = "first" });
|
||||
world.ExecuteCommands();
|
||||
world.Commands.Errors.Should().HaveCount(1);
|
||||
|
||||
world.Commands.Enqueue(new FailingCommand { Message = "second" });
|
||||
world.ExecuteCommands();
|
||||
world.Commands.Errors.Should().HaveCount(2);
|
||||
|
||||
world.Commands.Errors[0].Message.Should().Be("first");
|
||||
world.Commands.Errors[1].Message.Should().Be("second");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void CommandQueue_ClearErrors_ResetsErrorList()
|
||||
{
|
||||
var world = new World();
|
||||
|
||||
world.Commands.Enqueue(new FailingCommand { Message = "boom" });
|
||||
world.ExecuteCommands();
|
||||
world.Commands.Errors.Should().HaveCount(1);
|
||||
|
||||
world.Commands.ClearErrors();
|
||||
world.Commands.Errors.Should().BeEmpty();
|
||||
}
|
||||
|
||||
// ── Multiple World instances ──────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void MultipleWorlds_AreIndependent()
|
||||
{
|
||||
var world1 = new World();
|
||||
var world2 = new World();
|
||||
|
||||
var e1 = world1.CreateEntity();
|
||||
var e2 = world2.CreateEntity();
|
||||
|
||||
world1.AddComponent(e1, new Position { X = 1, Y = 2 });
|
||||
world2.AddComponent(e2, new Position { X = 3, Y = 4 });
|
||||
|
||||
// Each world sees its own entity's components.
|
||||
ref var pos1 = ref world1.GetComponent<Position>(e1);
|
||||
pos1.X.Should().Be(1);
|
||||
|
||||
ref var pos2 = ref world2.GetComponent<Position>(e2);
|
||||
pos2.X.Should().Be(3);
|
||||
|
||||
// Worlds are independent: destroying in one doesn't affect the other.
|
||||
world1.DestroyEntity(e1);
|
||||
world1.IsAlive(e1).Should().BeFalse();
|
||||
world2.IsAlive(e2).Should().BeTrue();
|
||||
}
|
||||
|
||||
// ── EntityAllocator large scale ──────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void EntityAllocator_HandlesManyEntities()
|
||||
{
|
||||
var world = new World();
|
||||
var entities = new List<Entity>();
|
||||
|
||||
for (int i = 0; i < 5000; i++)
|
||||
{
|
||||
var e = world.CreateEntity();
|
||||
entities.Add(e);
|
||||
world.IsAlive(e).Should().BeTrue();
|
||||
}
|
||||
|
||||
// Destroy half.
|
||||
for (int i = 0; i < 2500; i++)
|
||||
{
|
||||
world.DestroyEntity(entities[i]);
|
||||
world.IsAlive(entities[i]).Should().BeFalse();
|
||||
}
|
||||
|
||||
// Create more — should reuse freed IDs.
|
||||
for (int i = 0; i < 2500; i++)
|
||||
{
|
||||
var e = world.CreateEntity();
|
||||
world.IsAlive(e).Should().BeTrue();
|
||||
}
|
||||
}
|
||||
|
||||
// ── Structural mutation during ForEach ────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void ForEach_AddingComponent_DoesNotAffectCurrentIteration()
|
||||
{
|
||||
var world = new World();
|
||||
var a = world.CreateEntity();
|
||||
var b = world.CreateEntity();
|
||||
world.AddComponent(a, new Position { X = 1, Y = 2 });
|
||||
world.AddComponent(b, new Position { X = 3, Y = 4 });
|
||||
|
||||
var query = world.Query().With<Position>().Build();
|
||||
var seen = new List<Entity>();
|
||||
|
||||
world.ForEach(query, (Entity e, ref Position pos) =>
|
||||
{
|
||||
seen.Add(e);
|
||||
// Add a component to the other entity during iteration.
|
||||
// This should not affect the current iteration.
|
||||
world.AddComponent(e, new Velocity { X = 1, Y = 1 });
|
||||
});
|
||||
|
||||
seen.Should().BeEquivalentTo([a, b]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ForEach_DestroyingEntity_DoesNotThrow()
|
||||
{
|
||||
var world = new World();
|
||||
var a = world.CreateEntity();
|
||||
var b = world.CreateEntity();
|
||||
world.AddComponent(a, new Position { X = 1, Y = 2 });
|
||||
world.AddComponent(b, new Position { X = 3, Y = 4 });
|
||||
|
||||
var query = world.Query().With<Position>().Build();
|
||||
|
||||
// Destroying an entity during iteration: the sparse set uses
|
||||
// swap-remove, so the destroyed entity's slot is replaced by
|
||||
// the last element. This is safe as long as we don't re-iterate
|
||||
// the destroyed entity (which we won't since it's swap-removed).
|
||||
var act = () =>
|
||||
{
|
||||
world.ForEach(query, (Entity e, ref Position pos) =>
|
||||
{
|
||||
world.DestroyEntity(e);
|
||||
});
|
||||
};
|
||||
act.Should().NotThrow();
|
||||
}
|
||||
|
||||
// ── Helpers ──────────────────────────────────────────────────────
|
||||
|
||||
private sealed class ChangeCollector : IObserver<EntityChange>
|
||||
{
|
||||
public List<EntityChange> Changes = new();
|
||||
|
||||
public void OnNext(EntityChange value) => Changes.Add(value);
|
||||
public void OnError(Exception error) { }
|
||||
public void OnCompleted() { }
|
||||
}
|
||||
|
||||
// Phantom types for relationship tests.
|
||||
public struct ChildOf { }
|
||||
public struct ParentOf { }
|
||||
}
|
||||
Loading…
Reference in New Issue