From 96d732d6ab6f6b8406236cb025cd0158acbc586e Mon Sep 17 00:00:00 2001 From: hypercross Date: Tue, 21 Jul 2026 09:15:27 +0800 Subject: [PATCH] refactor: simplify component iteration and singleton handling Update the ECS engine to support automatic mutation tracking for singletons during iteration, removing the need for manual `MarkModified` calls. - Update `ComponentDiscoveryGenerator` to detect singleton components. - Refactor `PlaceMarkCommand` and `WinCheckSystem` to leverage automatic mutation marking. - Replace `ForEach` usage with `Select` and `ItemN` access in tests and systems to align with the updated iterator API. --- Game.TicTacToe.Tests/GameFlowTests.cs | 33 +++++++-------- Game.TicTacToe.Tests/PlayTests.cs | 12 +++--- Game.TicTacToe.Tests/SnapshotTests.cs | 4 +- Game.TicTacToe/Commands/PlaceMarkCommand.cs | 23 +++++------ Game.TicTacToe/Systems/WinCheckSystem.cs | 20 +++++----- OECS.SourceGen/ComponentDiscoveryGenerator.cs | 40 ++++++++++++++----- 6 files changed, 76 insertions(+), 56 deletions(-) diff --git a/Game.TicTacToe.Tests/GameFlowTests.cs b/Game.TicTacToe.Tests/GameFlowTests.cs index 95931c5..ba861cb 100644 --- a/Game.TicTacToe.Tests/GameFlowTests.cs +++ b/Game.TicTacToe.Tests/GameFlowTests.cs @@ -12,9 +12,10 @@ public class GameFlowTests { var (world, _) = SetupGame(); - var query = world.Query().With().Without().Build(); + var query = new Query().Without(); var emptyCount = 0; - world.ForEach(query, (Entity e, ref Cell cell) => emptyCount++); + using var iter = world.Select(query); + while (iter.MoveNext()) emptyCount++; emptyCount.Should().Be(9); } @@ -27,15 +28,15 @@ public class GameFlowTests world.Commands.Enqueue(new PlaceMarkCommand { Row = 0, Col = 0 }); group.RunLogical(); - var query = world.Query().With().With().Build(); var markedCount = 0; - world.ForEach(query, (Entity e, ref Cell cell, ref Mark mark) => + using var iter = world.Select(); + while (iter.MoveNext()) { markedCount++; - cell.Row.Should().Be(0); - cell.Col.Should().Be(0); - mark.Player.Should().Be(Player.X); - }); + iter.Item1.Row.Should().Be(0); + iter.Item1.Col.Should().Be(0); + iter.Item2.Player.Should().Be(Player.X); + } markedCount.Should().Be(1); } @@ -65,12 +66,12 @@ public class GameFlowTests group.RunLogical(); // Only one mark should exist at (0,0), and it should still be X. - var query = world.Query().With().With().Build(); var marks = new List<(int Row, int Col, Player Player)>(); - world.ForEach(query, (Entity e, ref Cell cell, ref Mark mark) => + using var iter3 = world.Select(); + while (iter3.MoveNext()) { - marks.Add((cell.Row, cell.Col, mark.Player)); - }); + marks.Add((iter3.Item1.Row, iter3.Item1.Col, iter3.Item2.Player)); + } marks.Should().ContainSingle() .Which.Should().Be((0, 0, Player.X)); @@ -168,12 +169,12 @@ public class GameFlowTests state.CurrentPlayer.Should().Be(Player.X); state.MoveCount.Should().Be(2); - var query = world2.Query().With().With().Build(); var marks = new List<(int Row, int Col, Player Player)>(); - world2.ForEach(query, (Entity e, ref Cell cell, ref Mark mark) => + using var iter2 = world2.Select(); + while (iter2.MoveNext()) { - marks.Add((cell.Row, cell.Col, mark.Player)); - }); + marks.Add((iter2.Item1.Row, iter2.Item1.Col, iter2.Item2.Player)); + } marks.Should().BeEquivalentTo([(0, 0, Player.X), (1, 0, Player.O)]); } diff --git a/Game.TicTacToe.Tests/PlayTests.cs b/Game.TicTacToe.Tests/PlayTests.cs index 80837e2..115d103 100644 --- a/Game.TicTacToe.Tests/PlayTests.cs +++ b/Game.TicTacToe.Tests/PlayTests.cs @@ -163,8 +163,8 @@ public class PlayTests using (var iter = world.Select()) { while (iter.MoveNext()) - grid[iter.Current1.Row, iter.Current1.Col] = - iter.Current2.Player == Player.X ? 'X' : 'O'; + grid[iter.Item1.Row, iter.Item1.Col] = + iter.Item2.Player == Player.X ? 'X' : 'O'; } for (int r = 0; r < 3; r++) @@ -253,10 +253,10 @@ public class PlayTests private static List<(int Row, int Col)> GetEmptyCells(World world) { var empty = new List<(int, int)>(); - var query = world.Query().With().Without().Build(); - using var iter = world.Select(query); + var query = new Query().Without(); + using var iter = world.Select(query); while (iter.MoveNext()) - empty.Add((iter.Current1.Row, iter.Current1.Col)); + empty.Add((iter.Item1.Row, iter.Item1.Col)); return empty; } @@ -265,7 +265,7 @@ public class PlayTests var grid = new Player[3, 3]; using (var iter = world.Select()) while (iter.MoveNext()) - grid[iter.Current1.Row, iter.Current1.Col] = iter.Current2.Player; + grid[iter.Item1.Row, iter.Item1.Col] = iter.Item2.Player; grid[row, col] = player; for (int r = 0; r < 3; r++) diff --git a/Game.TicTacToe.Tests/SnapshotTests.cs b/Game.TicTacToe.Tests/SnapshotTests.cs index f32c34c..d30c5ce 100644 --- a/Game.TicTacToe.Tests/SnapshotTests.cs +++ b/Game.TicTacToe.Tests/SnapshotTests.cs @@ -212,8 +212,8 @@ public class SnapshotTests { while (iter.MoveNext()) { - grid[iter.Current1.Row, iter.Current1.Col] = - iter.Current2.Player == Player.X ? 'X' : 'O'; + grid[iter.Item1.Row, iter.Item1.Col] = + iter.Item2.Player == Player.X ? 'X' : 'O'; } } diff --git a/Game.TicTacToe/Commands/PlaceMarkCommand.cs b/Game.TicTacToe/Commands/PlaceMarkCommand.cs index dab1200..e6d9212 100644 --- a/Game.TicTacToe/Commands/PlaceMarkCommand.cs +++ b/Game.TicTacToe/Commands/PlaceMarkCommand.cs @@ -15,22 +15,23 @@ public struct PlaceMarkCommand : ICommand public void Execute(World world) { - ref var state = ref world.GetSingleton(); - - if (state.Status != GameStatus.Playing) + // Read-only check before iteration — never auto-marks. + if (world.ReadSingleton().Status != GameStatus.Playing) return; // Find the cell entity at (Row, Col) that has no Mark. - // Use the iterator API with early-exit via break. - var query = world.Query().With().Without().Build(); + var query = new Query().Without(); Entity? target = null; - using var iter = world.Select(query); + // Fetch singleton ref inside iteration so mutations are auto-marked. + using var iter = world.Select(query); + ref var state = ref world.GetSingleton(); + while (iter.MoveNext()) { - if (iter.Current1.Row == Row && iter.Current1.Col == Col) + if (iter.Item1.Row == Row && iter.Item1.Col == Col) { - target = iter.CurrentEntity; + target = iter.Entity; break; } } @@ -38,13 +39,11 @@ public struct PlaceMarkCommand : ICommand if (target == null) return; // Cell already occupied or invalid position. - // Place the mark. ComponentAdded is sufficient — MarkModified - // is only needed when mutating an existing component via GetComponent. + // Place the mark. world.AddComponent(target.Value, new Mark { Player = state.CurrentPlayer }); - // Advance turn. + // Advance turn. Mutations auto-marked via EndIteration — no MarkModified needed. state.MoveCount++; state.CurrentPlayer = state.CurrentPlayer == Player.X ? Player.O : Player.X; - world.MarkModified(World.SingletonEntity); } } diff --git a/Game.TicTacToe/Systems/WinCheckSystem.cs b/Game.TicTacToe/Systems/WinCheckSystem.cs index 66a744f..eade168 100644 --- a/Game.TicTacToe/Systems/WinCheckSystem.cs +++ b/Game.TicTacToe/Systems/WinCheckSystem.cs @@ -15,15 +15,17 @@ public class WinCheckSystem : ISystem if (world.ReadSingleton().Status != GameStatus.Playing) return; + // Get singleton ref inside iteration so all mutations are auto-marked + // by EndIteration — no MarkModified calls needed. + using var iter = world.Select(); ref var state = ref world.GetSingleton(); - // Build a 3×3 grid of marks using the iterator API. + // Build a 3×3 grid of marks. var grid = new Player[3, 3]; - using var iter = world.Select(); while (iter.MoveNext()) { - grid[iter.Current1.Row, iter.Current1.Col] = iter.Current2.Player; + grid[iter.Item1.Row, iter.Item1.Col] = iter.Item2.Player; } // Check rows. @@ -31,7 +33,7 @@ public class WinCheckSystem : ISystem { if (TryGetWinner(grid[r, 0], grid[r, 1], grid[r, 2], out var winner)) { - SetWinner(world, ref state, winner); + SetWinner(ref state, winner); return; } } @@ -41,7 +43,7 @@ public class WinCheckSystem : ISystem { if (TryGetWinner(grid[0, c], grid[1, c], grid[2, c], out var winner)) { - SetWinner(world, ref state, winner); + SetWinner(ref state, winner); return; } } @@ -49,12 +51,12 @@ public class WinCheckSystem : ISystem // Check diagonals. if (TryGetWinner(grid[0, 0], grid[1, 1], grid[2, 2], out var diag1)) { - SetWinner(world, ref state, diag1); + SetWinner(ref state, diag1); return; } if (TryGetWinner(grid[0, 2], grid[1, 1], grid[2, 0], out var diag2)) { - SetWinner(world, ref state, diag2); + SetWinner(ref state, diag2); return; } @@ -62,7 +64,6 @@ public class WinCheckSystem : ISystem if (state.MoveCount >= 9) { state.Status = GameStatus.Draw; - world.MarkModified(World.SingletonEntity); } } @@ -77,9 +78,8 @@ public class WinCheckSystem : ISystem return false; } - private static void SetWinner(World world, ref GameState state, Player winner) + private static void SetWinner(ref GameState state, Player winner) { state.Status = winner == Player.X ? GameStatus.XWon : GameStatus.OWon; - world.MarkModified(World.SingletonEntity); } } \ No newline at end of file diff --git a/OECS.SourceGen/ComponentDiscoveryGenerator.cs b/OECS.SourceGen/ComponentDiscoveryGenerator.cs index 795f828..21a5b45 100644 --- a/OECS.SourceGen/ComponentDiscoveryGenerator.cs +++ b/OECS.SourceGen/ComponentDiscoveryGenerator.cs @@ -44,6 +44,15 @@ public class ComponentDiscoveryGenerator : IIncrementalGenerator "GetSources", }; + private static readonly HashSet _singletonMethods = new() + { + "SetSingleton", + "GetSingleton", + "ReadSingleton", + "HasSingleton", + "RemoveSingleton", + }; + private static readonly HashSet _forEachNames = new() { "ForEach", @@ -57,7 +66,7 @@ public class ComponentDiscoveryGenerator : IIncrementalGenerator predicate: IsCandidateInvocation, transform: ExtractComponentType) .Where(t => t.FullyQualifiedName != null) - .Select((t, _) => (t.FullyQualifiedName!, t.AssemblyQualifiedName!, t.IsRelationship)); + .Select((t, _) => (t.FullyQualifiedName!, t.AssemblyQualifiedName!, t.IsRelationship, t.IsSingleton)); // Also collect ForEach type arguments from the World class. var forEachCalls = context.SyntaxProvider @@ -104,7 +113,7 @@ public class ComponentDiscoveryGenerator : IIncrementalGenerator return false; } - private static (string? FullyQualifiedName, string? AssemblyQualifiedName, bool IsRelationship) ExtractComponentType( + private static (string? FullyQualifiedName, string? AssemblyQualifiedName, bool IsRelationship, bool IsSingleton) ExtractComponentType( GeneratorSyntaxContext ctx, CancellationToken _) { if (ctx.Node is not InvocationExpressionSyntax invocation) @@ -140,7 +149,9 @@ public class ComponentDiscoveryGenerator : IIncrementalGenerator bool isRelationship = typeSymbol.AllInterfaces.Any(i => i.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat) == "global::OECS.IRelationship"); - return (fqn, aqn, isRelationship); + bool isSingleton = _singletonMethods.Contains(genericName.Identifier.Text); + + return (fqn, aqn, isRelationship, isSingleton); } private static ImmutableArray<(string FullyQualifiedName, string AssemblyQualifiedName, bool IsRelationship)> ExtractForEachTypes( @@ -177,21 +188,29 @@ public class ComponentDiscoveryGenerator : IIncrementalGenerator } private static void GenerateRegistry(SourceProductionContext context, - (ImmutableArray<(string FullyQualifiedName, string AssemblyQualifiedName, bool IsRelationship)> Left, + (ImmutableArray<(string FullyQualifiedName, string AssemblyQualifiedName, bool IsRelationship, bool IsSingleton)> Left, ImmutableArray<(string FullyQualifiedName, string AssemblyQualifiedName, bool IsRelationship)> Right) data) { var (invocations, forEachTypes) = data; // Collect unique types by fully-qualified name, deduplicating. - var typeMap = new Dictionary(); + var typeMap = new Dictionary(); foreach (var t in invocations) - typeMap[t.FullyQualifiedName] = (t.FullyQualifiedName, t.AssemblyQualifiedName, t.IsRelationship); + { + if (typeMap.TryGetValue(t.FullyQualifiedName, out var existing)) + typeMap[t.FullyQualifiedName] = (existing.Fqn, existing.Aqn, existing.IsRel || t.IsRelationship, existing.IsSingleton || t.IsSingleton); + else + typeMap[t.FullyQualifiedName] = (t.FullyQualifiedName, t.AssemblyQualifiedName, t.IsRelationship, t.IsSingleton); + } foreach (var t in forEachTypes) { if (!typeMap.ContainsKey(t.FullyQualifiedName)) - typeMap[t.FullyQualifiedName] = (t.FullyQualifiedName, t.AssemblyQualifiedName, t.IsRelationship); + typeMap[t.FullyQualifiedName] = (t.FullyQualifiedName, t.AssemblyQualifiedName, t.IsRelationship, false); else if (t.IsRelationship) - typeMap[t.FullyQualifiedName] = (t.FullyQualifiedName, t.AssemblyQualifiedName, true); + { + var existing = typeMap[t.FullyQualifiedName]; + typeMap[t.FullyQualifiedName] = (existing.Fqn, existing.Aqn, true, existing.IsSingleton); + } } if (typeMap.Count == 0) @@ -213,7 +232,7 @@ public class ComponentDiscoveryGenerator : IIncrementalGenerator sb.AppendLine(" {"); bool first = true; - foreach (var (fqn, aqn, isRel) in typeMap.Values.OrderBy(t => t.Fqn)) + foreach (var (fqn, aqn, isRel, isSingleton) in typeMap.Values.OrderBy(t => t.Fqn)) { if (!first) sb.AppendLine(","); @@ -225,7 +244,8 @@ public class ComponentDiscoveryGenerator : IIncrementalGenerator sb.AppendLine($" serialize: obj => MessagePackSerializer.Serialize(({fqn})obj),"); sb.AppendLine($" deserializeAndAdd: (world, entity, data) =>"); sb.AppendLine($" world.AddComponent(entity, MessagePackSerializer.Deserialize<{fqn}>(data)),"); - sb.AppendLine($" deserialize: data => MessagePackSerializer.Deserialize<{fqn}>(data)"); + sb.AppendLine($" deserialize: data => MessagePackSerializer.Deserialize<{fqn}>(data),"); + sb.AppendLine($" isSingleton: {(isSingleton ? "true" : "false")}"); sb.Append(" )"); }