Refactor/ecs to arch adapter#62
Merged
Merged
Conversation
- 将EcsModule类重命名为EcsServiceModule - 更新构造函数名称以匹配新的类名 - 修改ModuleName属性中的类型引用 - 在服务模块管理器中更新模块实例化调用
- 移除原有的ECS基础实现和接口定义 - 添加ArchEcsModule作为新的ECS模块实现 - 创建ArchSystemAdapter基类用于桥接Arch系统 - 修改MovementSystem继承ArchSystemAdapter适配新架构 - 更新ServiceModuleManager使用新的ArchECS模块 - 移除ArchitectureContext中的ECS相关方法 - 从项目中移除对Arch包的直接依赖引用
- 为 Position 结构体添加 StructLayout 特性以确保顺序布局 - 为 Velocity 结构体添加 StructLayout 特性以确保顺序布局 - 更新 Velocity 组件的 XML 文档注释，提供更详细的描述 - 优化 Velocity 组件属性的文档注释，明确单位信息 - 添加 System.Runtime.InteropServices 命名空间引用
- 将 MovementSystem 从继承 EcsSystemBase 改为继承 ArchSystemAdapter - 更新 MovementSystem 的初始化和更新方法以适配 Arch 架构 - 移除测试代码中的 ECS 相关接口实现和抽象层 - 将测试用例从 GFramework ECS API 迁移到 Arch 原生 API - 更新 ECS 测试类以使用 Arch World 和实体操作方法 - 重构 ECS 模块初始化流程以支持 Arch 系统注册和管理
审查者指南（Reviewer's Guide）将 ECS 集成从自定义的 使用 ArchEcsModule 和 ArchSystemAdapter 进行 ECS 更新的时序图sequenceDiagram
participant GL as GameLoop
participant AEM as ArchEcsModule
participant SYS as MovementSystem
participant W as World
GL->>AEM: Update(deltaTime)
loop for each ArchSystemAdapter<float>
AEM->>SYS: Update(deltaTime)
activate SYS
SYS->>SYS: OnUpdate(deltaTime)
SYS->>W: Query(Position, Velocity)
loop for each matching Entity
W-->>SYS: ref Position, ref Velocity
SYS->>SYS: pos.X += vel.X * deltaTime
SYS->>SYS: pos.Y += vel.Y * deltaTime
end
deactivate SYS
end
新的基于 Arch 的 ECS 集成类图classDiagram
direction LR
class IServiceModule {
<<interface>>
+string ModuleName
+int Priority
+bool IsEnabled
+void Register(IIocContainer container)
+void Initialize()
+ValueTask DestroyAsync()
}
class IIocContainer {
<<interface>>
+void Register(object instance)
+T Get~T~()
+List~T~ GetAll~T~()
}
class World {
+static World Create()
+static void Destroy(World world)
+int Size
+Entity Create()
+Entity Create(Position position, Velocity velocity)
+void Destroy(Entity entity)
+void Clear()
+bool IsAlive(Entity entity)
+bool Has~T~(Entity entity)
+ref T Get~T~(Entity entity)
+void Add~T~(Entity entity, T component)
+void Remove~T~(Entity entity)
}
class AbstractSystem {
<<abstract>>
+void Initialize()
+void Destroy()
#void OnInit()
#void OnDestroy()
}
namespace Arch_System {
class ISystem_T_ {
<<interface>>
+void Initialize()
+void BeforeUpdate(T t)
+void Update(T t)
+void AfterUpdate(T t)
}
}
class ArchSystemAdapter_T_ {
<<abstract>>
+World World
+void Initialize()
+void Destroy()
+void Update(T t)
#void OnInit()
#void OnDestroy()
#void OnArchInitialize()
#void OnBeforeUpdate(T t)
#void OnUpdate(T t)
#void OnAfterUpdate(T t)
#void OnArchDispose()
}
class MovementSystem {
<<sealed>>
-QueryDescription _query
#void OnArchInitialize()
#void OnUpdate(float deltaTime)
}
class ArchEcsModule {
<<sealed>>
-List~ArchSystemAdapter~float~~ _systems
-IIocContainer _container
-World _world
+string ModuleName
+int Priority
+bool IsEnabled
+ArchEcsModule(bool enabled)
+void Register(IIocContainer container)
+void Initialize()
+ValueTask DestroyAsync()
+void Update(float deltaTime)
}
class ServiceModuleManager {
-ILogger _logger
+void RegisterBuiltInModules(IIocContainer container, ArchitectureProperties properties)
}
class Position {
<<struct>>
+float X
+float Y
}
class Velocity {
<<struct>>
+float X
+float Y
}
IServiceModule <|.. ArchEcsModule
AbstractSystem <|-- ArchSystemAdapter_T_
Arch_System.ISystem_T_ <|.. ArchSystemAdapter_T_
ArchSystemAdapter_T_ --> World : uses
ArchSystemAdapter_float_ <|-- MovementSystem
ArchEcsModule o-- World : owns
ArchEcsModule o-- "*" ArchSystemAdapter_float_ : manages
ServiceModuleManager --> ArchEcsModule : registers
ServiceModuleManager --> IIocContainer : uses
World --> Position : component
World --> Velocity : component
文件级变更（File-Level Changes）
技巧和命令（Tips and commands）与 Sourcery 交互（Interacting with Sourcery）
自定义你的体验（Customizing Your Experience）访问你的 控制面板（dashboard） 以：
获取帮助（Getting Help）Original review guide in EnglishReviewer's GuideRefactors the ECS integration from a custom EcsWorld/EcsSystem* abstraction to an Arch.Core-based ArchEcsModule and ArchSystemAdapter, updating tests and service wiring to use the new architecture adapter and removing old ECS APIs from the architecture context. Sequence diagram for ECS update using ArchEcsModule and ArchSystemAdaptersequenceDiagram
participant GL as GameLoop
participant AEM as ArchEcsModule
participant SYS as MovementSystem
participant W as World
GL->>AEM: Update(deltaTime)
loop for each ArchSystemAdapter<float>
AEM->>SYS: Update(deltaTime)
activate SYS
SYS->>SYS: OnUpdate(deltaTime)
SYS->>W: Query(Position, Velocity)
loop for each matching Entity
W-->>SYS: ref Position, ref Velocity
SYS->>SYS: pos.X += vel.X * deltaTime
SYS->>SYS: pos.Y += vel.Y * deltaTime
end
deactivate SYS
end
Class diagram for new Arch-based ECS integrationclassDiagram
direction LR
class IServiceModule {
<<interface>>
+string ModuleName
+int Priority
+bool IsEnabled
+void Register(IIocContainer container)
+void Initialize()
+ValueTask DestroyAsync()
}
class IIocContainer {
<<interface>>
+void Register(object instance)
+T Get~T~()
+List~T~ GetAll~T~()
}
class World {
+static World Create()
+static void Destroy(World world)
+int Size
+Entity Create()
+Entity Create(Position position, Velocity velocity)
+void Destroy(Entity entity)
+void Clear()
+bool IsAlive(Entity entity)
+bool Has~T~(Entity entity)
+ref T Get~T~(Entity entity)
+void Add~T~(Entity entity, T component)
+void Remove~T~(Entity entity)
}
class AbstractSystem {
<<abstract>>
+void Initialize()
+void Destroy()
#void OnInit()
#void OnDestroy()
}
namespace Arch_System {
class ISystem_T_ {
<<interface>>
+void Initialize()
+void BeforeUpdate(T t)
+void Update(T t)
+void AfterUpdate(T t)
}
}
class ArchSystemAdapter_T_ {
<<abstract>>
+World World
+void Initialize()
+void Destroy()
+void Update(T t)
#void OnInit()
#void OnDestroy()
#void OnArchInitialize()
#void OnBeforeUpdate(T t)
#void OnUpdate(T t)
#void OnAfterUpdate(T t)
#void OnArchDispose()
}
class MovementSystem {
<<sealed>>
-QueryDescription _query
#void OnArchInitialize()
#void OnUpdate(float deltaTime)
}
class ArchEcsModule {
<<sealed>>
-List~ArchSystemAdapter~float~~ _systems
-IIocContainer _container
-World _world
+string ModuleName
+int Priority
+bool IsEnabled
+ArchEcsModule(bool enabled)
+void Register(IIocContainer container)
+void Initialize()
+ValueTask DestroyAsync()
+void Update(float deltaTime)
}
class ServiceModuleManager {
-ILogger _logger
+void RegisterBuiltInModules(IIocContainer container, ArchitectureProperties properties)
}
class Position {
<<struct>>
+float X
+float Y
}
class Velocity {
<<struct>>
+float X
+float Y
}
IServiceModule <|.. ArchEcsModule
AbstractSystem <|-- ArchSystemAdapter_T_
Arch_System.ISystem_T_ <|.. ArchSystemAdapter_T_
ArchSystemAdapter_T_ --> World : uses
ArchSystemAdapter_float_ <|-- MovementSystem
ArchEcsModule o-- World : owns
ArchEcsModule o-- "*" ArchSystemAdapter_float_ : manages
ServiceModuleManager --> ArchEcsModule : registers
ServiceModuleManager --> IIocContainer : uses
World --> Position : component
World --> Velocity : component
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
 |
|
Overall Grade Focus Area: Reliability |
Security  Reliability  Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| C# |  |
Mar 2, 2026 1:20p.m. | Review ↗ |
| Secrets |  |
Mar 2, 2026 1:20p.m. | Review ↗ |
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
你好，我发现了两个问题，并给出了一些整体性的反馈：
- 在 ArchSystemAdapter 中，OnUpdate 的 XML 注释说明它是抽象方法，但该方法实际上是带有默认空实现的 virtual；建议要么将其改为 abstract，以强制在派生系统中重写，要么更新注释以反映当前的行为。
- ArchEcsModule.Initialize() 在每次调用时都会向 _systems 追加内容；如果 Initialize 在应用程序生命周期中可能被调用多次，那么最好增加防重初始化的保护（例如在重新初始化前清空 _systems，或者增加一个已初始化的标记）。
给 AI 代理的提示
请根据本次代码评审中的评论进行修改：
## 总体说明
- 在 ArchSystemAdapter<T> 中，OnUpdate 的 XML 注释说明它是抽象方法，但该方法实际上是带有默认空实现的 virtual；建议要么将其改为 abstract，以强制在派生系统中重写，要么更新注释以反映当前的行为。
- ArchEcsModule.Initialize() 在每次调用时都会向 _systems 追加内容；如果 Initialize 在应用程序生命周期中可能被调用多次，那么最好增加防重初始化的保护（例如在重新初始化前清空 _systems，或者增加一个已初始化的标记）。
## 具体评论
### Comment 1
<location path="GFramework.Core/ecs/ArchSystemAdapter.cs" line_range="8-17" />
<code_context>
-
- // === ECS 支持 ===
-
- /// <summary>
- /// 获取ECS世界实例
- /// </summary>
</code_context>
<issue_to_address>
**suggestion:** XML 注释将 OnUpdate 描述为抽象方法，但该方法实际上是一个带有默认空实现的 virtual 方法。
注释中使用了“受保护抽象方法”和“子类必须重写此方法”的表述，但 `OnUpdate` 实际上是 `protected virtual` 且方法体为空。建议要么将其改为 `abstract`，以强制子类必须重写；要么更新注释，说明重写是可选的，并且默认实现不执行任何操作。
建议实现如下：
```csharp
/// <summary>
/// 受保护虚方法，在系统更新时被调用。
/// 默认实现不执行任何操作，子类可以根据需要选择性地重写此方法以实现具体的系统逻辑。
/// </summary>
```
如果该文件（或相关基类）中还有其他 XML 注释仍然将 `OnUpdate` 描述为“抽象”或说明子类“必须重写此方法”，也应当一并更新，以反映 `OnUpdate` 实际上是一个 `protected virtual` 方法，重写是可选的，且默认实现为空操作。
</issue_to_address>
### Comment 2
<location path="GFramework.Core/ecs/ArchEcsModule.cs" line_range="75-99" />
<code_context>
+ /// <summary>
+ /// 异步销毁
+ /// </summary>
+ public async ValueTask DestroyAsync()
+ {
+ if (!IsEnabled) return;
+
+ // 销毁所有系统
+ foreach (var system in _systems)
+ {
+ system.Destroy();
+ }
+
+ _systems.Clear();
+
+ // 销毁 World
+ if (_world != null)
+ {
+ World.Destroy(_world);
+ _world = null;
+ }
+
+ await ValueTask.CompletedTask;
+ }
+
</code_context>
<issue_to_address>
**suggestion:** DestroyAsync 被标记为 async，但内部只执行同步工作并等待一个已完成的任务。
由于这里的所有工作都是同步的，并且最后只是执行 `await ValueTask.CompletedTask;`，`async` 状态机会带来额外开销而没有实际收益。可以考虑将该方法改为非 async，在清理完成后直接返回 `ValueTask.CompletedTask`；或者只有在未来预期会有真正的异步操作时才保留 async 签名。
```suggestion
/// <summary>
/// 异步销毁
/// </summary>
+ public ValueTask DestroyAsync()
+ {
+ if (!IsEnabled) return ValueTask.CompletedTask;
+
+ // 销毁所有系统
+ foreach (var system in _systems)
+ {
+ system.Destroy();
+ }
+
+ _systems.Clear();
+
+ // 销毁 World
+ if (_world != null)
+ {
+ World.Destroy(_world);
+ _world = null;
+ }
+
+ return ValueTask.CompletedTask;
+ }
+
```
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈进一步改进评审质量。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- In ArchSystemAdapter, the XML comment for OnUpdate says it is abstract but the method is virtual with a default empty implementation; either make it abstract to enforce overriding in derived systems or adjust the comment to reflect the current behavior.
- ArchEcsModule.Initialize() will append to _systems each time it is called; if Initialize can be invoked more than once in the application lifecycle it would be safer to guard against double-initialization (e.g., by clearing _systems or tracking an initialized flag).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In ArchSystemAdapter<T>, the XML comment for OnUpdate says it is abstract but the method is virtual with a default empty implementation; either make it abstract to enforce overriding in derived systems or adjust the comment to reflect the current behavior.
- ArchEcsModule.Initialize() will append to _systems each time it is called; if Initialize can be invoked more than once in the application lifecycle it would be safer to guard against double-initialization (e.g., by clearing _systems or tracking an initialized flag).
## Individual Comments
### Comment 1
<location path="GFramework.Core/ecs/ArchSystemAdapter.cs" line_range="8-17" />
<code_context>
-
- // === ECS 支持 ===
-
- /// <summary>
- /// 获取ECS世界实例
- /// </summary>
</code_context>
<issue_to_address>
**suggestion:** The XML comment describes OnUpdate as abstract, but the method is virtual with a default empty implementation.
The summary says "受保护抽象方法" and "子类必须重写此方法", but `OnUpdate` is `protected virtual` with a no-op body. Either change it to `abstract` to enforce overriding, or update the comment to indicate that overriding is optional and the default implementation does nothing.
Suggested implementation:
```csharp
/// <summary>
/// 受保护虚方法，在系统更新时被调用。
/// 默认实现不执行任何操作，子类可以根据需要选择性地重写此方法以实现具体的系统逻辑。
/// </summary>
```
If there are other XML comments in this file (or related base classes) that still describe `OnUpdate` as “抽象” or state that subclasses “必须重写此方法”, they should be updated similarly to reflect that `OnUpdate` is a `protected virtual` method with an optional override and a no-op default implementation.
</issue_to_address>
### Comment 2
<location path="GFramework.Core/ecs/ArchEcsModule.cs" line_range="75-99" />
<code_context>
+ /// <summary>
+ /// 异步销毁
+ /// </summary>
+ public async ValueTask DestroyAsync()
+ {
+ if (!IsEnabled) return;
+
+ // 销毁所有系统
+ foreach (var system in _systems)
+ {
+ system.Destroy();
+ }
+
+ _systems.Clear();
+
+ // 销毁 World
+ if (_world != null)
+ {
+ World.Destroy(_world);
+ _world = null;
+ }
+
+ await ValueTask.CompletedTask;
+ }
+
</code_context>
<issue_to_address>
**suggestion:** DestroyAsync is marked async but only performs synchronous work and awaits a completed task.
Since all work here is synchronous and you end with `await ValueTask.CompletedTask;`, the `async` state machine adds overhead without value. Consider either making this method non-async and returning `ValueTask.CompletedTask` directly after the cleanup, or keeping an async signature only if you expect real async operations later.
```suggestion
/// <summary>
/// 异步销毁
/// </summary>
+ public ValueTask DestroyAsync()
+ {
+ if (!IsEnabled) return ValueTask.CompletedTask;
+
+ // 销毁所有系统
+ foreach (var system in _systems)
+ {
+ system.Destroy();
+ }
+
+ _systems.Clear();
+
+ // 销毁 World
+ if (_world != null)
+ {
+ World.Destroy(_world);
+ _world = null;
+ }
+
+ return ValueTask.CompletedTask;
+ }
+
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- 添加 _isInitialized 标志防止 ArchEcsModule 重复初始化 - 在 Initialize 方法中检查是否已初始化避免重复执行 - 设置初始化标志在所有系统初始化完成后 - 修改 DestroyAsync 方法确保仅在已初始化时执行销毁 - 重置 _isInitialized 标志为 false 在销毁时 - 修正 ArchSystemAdapter 注释描述 OnUpdate 为虚方法而非抽象方法 - 简化 EcsAdvancedTests 中的断言语法移除不必要的大括号
- 移除 DestroyAsync 方法中的 async 关键字 - 修改条件判断逻辑，确保返回正确的 ValueTask.CompletedTask - 删除不必要的 await 关键字，直接返回 ValueTask.CompletedTask - 保持方法的异步操作正确性
- 修改了核心更新逻辑方法的文档描述 - 明确说明该方法为受保护虚方法，默认不执行任何操作 - 详细说明子类可以按需选择性重写此方法来实现具体系统逻辑 - 更新了参数说明文档
This was referenced Mar 15, 2026
This was referenced May 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
使用基于 Arch.Core 的 ECS 模块和适配器替换自定义的 ECS world 与系统基础设施，并更新测试和服务以采用新架构。
New Features:
ArchEcsModule，用于在服务模块框架中管理 Arch.CoreWorld的生命周期，并协调系统更新。ArchSystemAdapter基类，以将Arch.System系统集成到现有的框架生命周期和依赖注入（DI）容器中。Enhancements:
MovementSystem，使其继承自ArchSystemAdapter，并使用原生查询直接在 Arch.CoreWorld上运行。Position和VelocityECS 组件标记为顺序结构体布局，以改善互操作性并控制内存布局。ArchEcsModule，而不是旧的 ECS 模块。World和ArchEcsModule，而不再依赖已移除的自定义 ECS world 和 runner 抽象。IArchitectureContext及其测试实现中移除与 ECS 相关的方法，使该抽象与新的基于 Arch 的 ECS 集成保持一致。Tests:
ArchEcsModule和 Arch.CoreWorld验证行为，包括创建、销毁、组件操作、系统更新以及性能场景。Chores:
IEcsWorld、IEcsSystem、EcsWorld、EcsSystemBase、EcsSystemRunner以及旧的EcsModule。Original summary in English
Summary by Sourcery
Replace the custom ECS world and system infrastructure with an Arch.Core-based ECS module and adapters, and update tests and services to use the new architecture.
New Features:
Enhancements:
Tests:
Chores: