Refactor/extract ecs to independent module#89
Merged
Conversation
- 删除Position组件结构体定义 - 删除Velocity组件结构体定义 - 删除MovementSystem移动系统实现 - 移除ArchEcsModule ECS模块管理器 - 删除ArchSystemAdapter适配器基类 - 从ServiceModuleManager中移除ECS模块注册逻辑 - 从ArchitectureProperties中移除EnableEcs配置选项 - 删除ECS相关的单元测试文件 - 从项目文件中移除Arch和Arch.System包引用 - 从解决方案文件中移除ECS相关项目引用 - 更新项目配置文件中的目标框架和测试项目属性
- 移除自动初始化器，改用显式注册方式 - 修改 UseArch 扩展方法支持可选配置参数 - 更新文档说明新的集成方式和配置选项 - 删除自动注册相关的测试代码 - 调整 README 中的使用示例和架构说明
 |
|
Overall Grade Focus Area: Hygiene |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| C# |  |
Mar 8, 2026 1:01p.m. | Review ↗ |
| Secrets | |
Mar 8, 2026 1:01p.m. | Review ↗ |
审阅者指南将基于 Arch 的 ECS 支持从 GFramework.Core 中抽离出来，放入一个独立的 GFramework.Ecs.Arch 模块中，该模块拥有自己的抽象、配置选项、基于扩展方法的注册方式以及测试，用显式的模块注册机制替代原有的 ArchitectureProperties.EnableEcs 开关。 架构初始化与 ECS Arch 模块使用的时序图sequenceDiagram
actor Dev
participant GA as GameArchitecture
participant AE as ArchExtensions
participant AMR as ArchitectureModuleRegistry
participant SM as ServiceModuleManager
participant AEM as ArchEcsModule
participant IOC as IIocContainer
Dev->>GA: new GameArchitecture(config)
Dev->>GA: UseArch(configure)
GA->>AE: UseArch(GA, configure)
AE->>AE: create ArchOptions
AE-->>AE: apply configure
AE->>AMR: Register(() => new ArchEcsModule(enabled: true))
AMR-->>AE: factory stored
AE-->>GA: return architecture
Dev->>GA: Initialize()
GA->>SM: RegisterBuiltInModules(IOC, properties)
SM->>SM: Register built-in core modules
SM->>AMR: CreateModules()
AMR-->>SM: list of IServiceModule (includes ArchEcsModule)
loop for each module
SM->>SM: RegisterModule(module)
end
SM->>AEM: Configure(IOC)
AEM->>IOC: register World and ECS services
SM->>AEM: Initialize()
note over Dev,IOC: Runtime usage
Dev->>IOC: Resolve<IArchEcsModule>()
IOC-->>Dev: ArchEcsModule
Dev->>AEM: Update(deltaTime)
AEM->>AEM: Update all ArchSystemAdapter<float> systems
新 ECS Arch 抽象和模块的类图classDiagram
direction LR
class ISystem {
}
class IServiceModule {
<<interface>>
+int Priority
+bool IsEnabled
+string ModuleName
+void Configure(IIocContainer container)
+void Initialize()
+void Shutdown()
}
class IArchitecture {
<<interface>>
+IIocContainer Container
}
class IArchSystemAdapter_T_ {
<<interface>>
+void Update(in T t)
}
class IArchEcsModule {
<<interface>>
+void Update(float deltaTime)
}
class ArchOptions {
+int WorldCapacity
+bool EnableStatistics
+int Priority
}
class ArchSystemAdapter_T_ {
<<abstract>>
+World World
+void Update(in T t)
#void OnBeforeUpdate(in T t)
#void OnUpdate(in T t)
#void OnAfterUpdate(in T t)
#void OnArchInitialize()
}
class ArchEcsModule {
+bool IsEnabled
+int Priority
+string ModuleName
+void Configure(IIocContainer container)
+void Initialize()
+void Shutdown()
+void Update(float deltaTime)
}
class MovementSystem {
+QueryDescription _query
+void OnArchInitialize()
+void OnUpdate(in float deltaTime)
}
class World {
}
class QueryDescription {
+QueryDescription WithAll_Position_Velocity_()
}
class Position {
+float X
+float Y
}
class Velocity {
+float X
+float Y
}
IArchSystemAdapter_T_ --|> ISystem
ArchSystemAdapter_T_ --|> AbstractSystem
ArchSystemAdapter_T_ --|> IArchSystemAdapter_T_
ArchSystemAdapter_T_ ..> World
IArchEcsModule --|> IServiceModule
ArchEcsModule --|> IArchEcsModule
MovementSystem --|> ArchSystemAdapter_float_
MovementSystem ..> Position
MovementSystem ..> Velocity
MovementSystem ..> World
MovementSystem ..> QueryDescription
Position ..> Velocity
ArchOptions ..> ArchEcsModule
IArchitecture ..> ArchEcsModule
class AbstractSystem {
}
class IIocContainer {
<<interface>>
+void Register_T_()
+T Resolve_T_()
}
ArchEcsModule ..> IIocContainer
文件级改动
技巧和命令与 Sourcery 交互
自定义你的体验访问你的控制面板 以：
获取帮助Original review guide in EnglishReviewer's GuideRefactors Arch-based ECS support out of GFramework.Core into a dedicated GFramework.Ecs.Arch module with its own abstractions, options, extension-based registration, and tests, replacing the old ArchitectureProperties.EnableEcs toggle with an explicit module registration mechanism. Sequence diagram for architecture initialization and ECS Arch module usagesequenceDiagram
actor Dev
participant GA as GameArchitecture
participant AE as ArchExtensions
participant AMR as ArchitectureModuleRegistry
participant SM as ServiceModuleManager
participant AEM as ArchEcsModule
participant IOC as IIocContainer
Dev->>GA: new GameArchitecture(config)
Dev->>GA: UseArch(configure)
GA->>AE: UseArch(GA, configure)
AE->>AE: create ArchOptions
AE-->>AE: apply configure
AE->>AMR: Register(() => new ArchEcsModule(enabled: true))
AMR-->>AE: factory stored
AE-->>GA: return architecture
Dev->>GA: Initialize()
GA->>SM: RegisterBuiltInModules(IOC, properties)
SM->>SM: Register built-in core modules
SM->>AMR: CreateModules()
AMR-->>SM: list of IServiceModule (includes ArchEcsModule)
loop for each module
SM->>SM: RegisterModule(module)
end
SM->>AEM: Configure(IOC)
AEM->>IOC: register World and ECS services
SM->>AEM: Initialize()
note over Dev,IOC: Runtime usage
Dev->>IOC: Resolve<IArchEcsModule>()
IOC-->>Dev: ArchEcsModule
Dev->>AEM: Update(deltaTime)
AEM->>AEM: Update all ArchSystemAdapter<float> systems
Class diagram for new ECS Arch abstractions and moduleclassDiagram
direction LR
class ISystem {
}
class IServiceModule {
<<interface>>
+int Priority
+bool IsEnabled
+string ModuleName
+void Configure(IIocContainer container)
+void Initialize()
+void Shutdown()
}
class IArchitecture {
<<interface>>
+IIocContainer Container
}
class IArchSystemAdapter_T_ {
<<interface>>
+void Update(in T t)
}
class IArchEcsModule {
<<interface>>
+void Update(float deltaTime)
}
class ArchOptions {
+int WorldCapacity
+bool EnableStatistics
+int Priority
}
class ArchSystemAdapter_T_ {
<<abstract>>
+World World
+void Update(in T t)
#void OnBeforeUpdate(in T t)
#void OnUpdate(in T t)
#void OnAfterUpdate(in T t)
#void OnArchInitialize()
}
class ArchEcsModule {
+bool IsEnabled
+int Priority
+string ModuleName
+void Configure(IIocContainer container)
+void Initialize()
+void Shutdown()
+void Update(float deltaTime)
}
class MovementSystem {
+QueryDescription _query
+void OnArchInitialize()
+void OnUpdate(in float deltaTime)
}
class World {
}
class QueryDescription {
+QueryDescription WithAll_Position_Velocity_()
}
class Position {
+float X
+float Y
}
class Velocity {
+float X
+float Y
}
IArchSystemAdapter_T_ --|> ISystem
ArchSystemAdapter_T_ --|> AbstractSystem
ArchSystemAdapter_T_ --|> IArchSystemAdapter_T_
ArchSystemAdapter_T_ ..> World
IArchEcsModule --|> IServiceModule
ArchEcsModule --|> IArchEcsModule
MovementSystem --|> ArchSystemAdapter_float_
MovementSystem ..> Position
MovementSystem ..> Velocity
MovementSystem ..> World
MovementSystem ..> QueryDescription
Position ..> Velocity
ArchOptions ..> ArchEcsModule
IArchitecture ..> ArchEcsModule
class AbstractSystem {
}
class IIocContainer {
<<interface>>
+void Register_T_()
+T Resolve_T_()
}
ArchEcsModule ..> IIocContainer
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了两个问题，并给出了一些总体反馈：
- 新的 ArchitectureModuleRegistry 使用了一个静态可变的 List，没有任何同步或重复处理；如果模块可能在不同上下文中被注册，建议让每种模块类型的注册是幂等的，并/或增加基础的线程安全措施。
- ArchExtensions.UseArch 创建了一个 ArchOptions 实例，但从未将这些选项传递给 ArchEcsModule 或其他任何地方，因此配置实际上被忽略了；建议要么在构造模块（以及模块行为）时接入 ArchOptions，要么移除这些未使用的选项以避免混淆。
- ServiceModuleManager.RegisterBuiltInModules 仍然接受 ArchitectureProperties，但在移除 EnableEcs 之后已经不再使用它；建议移除该参数或重新赋予它用途，以避免误导性的 API 设计。
给 AI Agent 的提示
请根据下面的代码审查意见进行修改：
## 总体意见
- 新的 ArchitectureModuleRegistry 使用了一个静态可变的 List，没有任何同步或重复处理；如果模块可能在不同上下文中被注册，建议让每种模块类型的注册是幂等的，并/或增加基础的线程安全措施。
- ArchExtensions.UseArch 创建了一个 ArchOptions 实例，但从未将这些选项传递给 ArchEcsModule 或其他任何地方，因此配置实际上被忽略了；建议要么在构造模块（以及模块行为）时接入 ArchOptions，要么移除这些未使用的选项以避免混淆。
- ServiceModuleManager.RegisterBuiltInModules 仍然接受 ArchitectureProperties，但在移除 EnableEcs 之后已经不再使用它；建议移除该参数或重新赋予它用途，以避免误导性的 API 设计。
## 单独评论
### 评论 1
<location path="GFramework.Core/services/ServiceModuleManager.cs" line_range="50-51" />
<code_context>
/// </summary>
/// <param name="container">IoC容器实例，用于模块服务注册。</param>
- /// <param name="properties">架构属性配置，用于判断是否启用ECS模块。</param>
+ /// <param name="properties">架构属性配置。</param>
public void RegisterBuiltInModules(IIocContainer container, ArchitectureProperties properties)
{
if (_builtInModulesRegistered)
</code_context>
<issue_to_address>
**suggestion:** ArchitectureProperties 现在在 RegisterBuiltInModules 中未被使用，可以移除或重新利用。
由于此处已不再使用 `ArchitectureProperties`，建议移除该参数，以保持 API 精简并避免混淆。如果你预计很快会用它做额外配置，可以加一个简短的 TODO 或占位分支来向调用方说明它的用途。
建议实现：
```csharp
/// <summary>
/// 注册内置服务模块，并根据优先级排序后完成服务注册。
/// 内置模块包括事件总线、命令执行器、查询执行器等核心模块。
/// 同时注册通过 ModuleInitializer 自动注册的外部模块。
/// </summary>
/// <param name="container">IoC容器实例，用于模块服务注册。</param>
public void RegisterBuiltInModules(IIocContainer container)
{
if (_builtInModulesRegistered)
return;
// 注册内置模块
RegisterModule(new EventBusModule());
RegisterModule(new CommandExecutorModule());
RegisterModule(new QueryExecutorModule());
RegisterModule(new AsyncQueryExecutorModule());
```
1. 更新所有对 `RegisterBuiltInModules` 的调用点，移除 `ArchitectureProperties` 参数，仅传入 `IIocContainer` 实例。
2. 如果 `ArchitectureProperties` 之前只在这个方法中使用，考虑移除本文件中与之相关、现在已未使用的 `using` 指令或引用。
</issue_to_address>
### 评论 2
<location path="GFramework.Ecs.Arch/extensions/ArchExtensions.cs" line_range="17-26" />
<code_context>
+ /// <param name="architecture">架构实例</param>
+ /// <param name="configure">可选的配置委托</param>
+ /// <returns>架构实例，支持链式调用</returns>
+ public static TArchitecture UseArch<TArchitecture>(
+ this TArchitecture architecture,
+ Action<ArchOptions>? configure = null)
+ where TArchitecture : IArchitecture
+ {
+ // 配置选项
+ var options = new ArchOptions();
+ configure?.Invoke(options);
+
+ // 注册模块
+ ArchitectureModuleRegistry.Register(() => new ArchEcsModule(enabled: true));
+
+ return architecture;
</code_context>
<issue_to_address>
**issue:** ArchOptions 被配置了，但其值未被使用，这可能会误导使用者。
`UseArch` 构建了一个 `ArchOptions` 实例（WorldCapacity、EnableStatistics、Priority），但从未将其传递给 `ArchEcsModule` 或其他消费者，因此当前配置委托实际上不起任何作用。建议要么将这些选项接入 `ArchEcsModule`（例如通过构造函数或配置接口），要么暂时移除这些配置选项，以避免暴露一个“没有效果”的配置 API。
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据这些反馈改进以后的评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The new ArchitectureModuleRegistry uses a static mutable List without any synchronization or duplicate-handling; consider making registration idempotent per module type and/or adding basic thread-safety if modules can be registered from different contexts.
- ArchExtensions.UseArch creates an ArchOptions instance but never passes those options to ArchEcsModule or anywhere else, so the configuration is effectively ignored; either wire ArchOptions into module construction (and module behavior) or remove the unused options to avoid confusion.
- ServiceModuleManager.RegisterBuiltInModules still takes ArchitectureProperties but no longer uses it after removing EnableEcs; consider removing this parameter or repurposing it to avoid a misleading API surface.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new ArchitectureModuleRegistry uses a static mutable List without any synchronization or duplicate-handling; consider making registration idempotent per module type and/or adding basic thread-safety if modules can be registered from different contexts.
- ArchExtensions.UseArch creates an ArchOptions instance but never passes those options to ArchEcsModule or anywhere else, so the configuration is effectively ignored; either wire ArchOptions into module construction (and module behavior) or remove the unused options to avoid confusion.
- ServiceModuleManager.RegisterBuiltInModules still takes ArchitectureProperties but no longer uses it after removing EnableEcs; consider removing this parameter or repurposing it to avoid a misleading API surface.
## Individual Comments
### Comment 1
<location path="GFramework.Core/services/ServiceModuleManager.cs" line_range="50-51" />
<code_context>
/// </summary>
/// <param name="container">IoC容器实例，用于模块服务注册。</param>
- /// <param name="properties">架构属性配置，用于判断是否启用ECS模块。</param>
+ /// <param name="properties">架构属性配置。</param>
public void RegisterBuiltInModules(IIocContainer container, ArchitectureProperties properties)
{
if (_builtInModulesRegistered)
</code_context>
<issue_to_address>
**suggestion:** ArchitectureProperties is now unused in RegisterBuiltInModules and could be removed or repurposed.
Since `ArchitectureProperties` is no longer used here, consider removing the parameter to keep the API minimal and avoid confusion. If you expect to use it soon for additional configuration, adding a brief TODO or placeholder branch would help clarify its purpose to callers.
Suggested implementation:
```csharp
/// <summary>
/// 注册内置服务模块，并根据优先级排序后完成服务注册。
/// 内置模块包括事件总线、命令执行器、查询执行器等核心模块。
/// 同时注册通过 ModuleInitializer 自动注册的外部模块。
/// </summary>
/// <param name="container">IoC容器实例，用于模块服务注册。</param>
public void RegisterBuiltInModules(IIocContainer container)
{
if (_builtInModulesRegistered)
return;
// 注册内置模块
RegisterModule(new EventBusModule());
RegisterModule(new CommandExecutorModule());
RegisterModule(new QueryExecutorModule());
RegisterModule(new AsyncQueryExecutorModule());
```
1. Update all call sites of `RegisterBuiltInModules` to remove the `ArchitectureProperties` argument and pass only the `IIocContainer` instance.
2. If `ArchitectureProperties` was only used for this method, consider removing related `using` directives or references that are now unused in this file.
</issue_to_address>
### Comment 2
<location path="GFramework.Ecs.Arch/extensions/ArchExtensions.cs" line_range="17-26" />
<code_context>
+ /// <param name="architecture">架构实例</param>
+ /// <param name="configure">可选的配置委托</param>
+ /// <returns>架构实例，支持链式调用</returns>
+ public static TArchitecture UseArch<TArchitecture>(
+ this TArchitecture architecture,
+ Action<ArchOptions>? configure = null)
+ where TArchitecture : IArchitecture
+ {
+ // 配置选项
+ var options = new ArchOptions();
+ configure?.Invoke(options);
+
+ // 注册模块
+ ArchitectureModuleRegistry.Register(() => new ArchEcsModule(enabled: true));
+
+ return architecture;
</code_context>
<issue_to_address>
**issue:** ArchOptions is configured but its values are not used, which can be misleading for consumers.
`UseArch` builds an `ArchOptions` instance (WorldCapacity, EnableStatistics, Priority) but never passes it to `ArchEcsModule` or any other consumer, so the configuration delegate currently does nothing. Either wire these options into `ArchEcsModule` (e.g., via constructor/config interface) or remove the options for now to avoid exposing a configuration API that has no effect.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- 在 ArchEcsModule 初始化时注册模块自身到容器 - 移除不必要的空行以保持代码整洁
- 将 _systems 字段类型从 List 改为 IReadOnlyList - 使用 GetAllByPriority 方法按优先级获取适配器 - 移除手动 AddRange 操作，直接赋值适配器列表 - 销毁时将 _systems 设置为空数组而非 Clear() 操作
- 移除 RegisterBuiltInModules 方法中的 ArchitectureProperties 参数 - 更新 ArchitectureModuleRegistry 使用 ConcurrentDictionary 存储模块工厂 - 实现模块注册的幂等性检查，相同模块名只注册一次 - 为 ArchEcsModule 添加 ArchOptions 配置类支持 - 更新 UseArch 扩展方法传递配置选项给 ArchEcsModule - 移除废弃的 properties 命名空间引用 - 添加显式注册集成测试验证模块配置功能
- 将 ECS 相关菜单项从 Core 部分独立出来 - 新增 ECS 概述页面和 Arch ECS 集成文档 - 删除旧的 Core/ECS 集成文档文件 - 更新侧边栏配置以支持新的 ECS 导航结构
- 为 CI 工作流添加新的测试步骤 - 配置 GFramework.Ecs.Arch.Tests 的测试运行命令 - 设置测试结果输出目录为 TestResults - 使用 trx 格式记录测试日志文件 - 保持与现有测试配置一致的结构和格式
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 的 ECS 抽取到独立的
GFramework.Ecs.Arch模块中，并提供显式、按需启用（opt‑in）的注册与集成 API。New Features:
GFramework.Ecs.Arch和GFramework.Ecs.Arch.Abstractions包，将 Arch ECS 集成从核心中分离出来单独托管。ArchOptions配置对象和UseArch扩展方法，用于在架构实例上显式启用并配置 Arch ECS。IArchSystemAdapter<T>和IArchEcsModule抽象，以规范 ECS 集成契约。Enhancements:
ArchSystemAdapter，使其实现新的IArchSystemAdapter<T>接口，并将其移动到新的GFramework.Ecs.Arch命名空间下。ArchitectureModuleRegistry，以支持发现并自动注册外部服务模块。ServiceModuleManager，改为始终注册核心模块，然后通过ArchitectureModuleRegistry注册外部模块，而不再使用EnableEcs标志。GFramework.Ecs.Arch项目与命名空间中，从而将核心与 Arch 依赖解耦。GFramework.Ecs.Arch包新增文档，包括用法、配置以及生命周期细节。Build:
GFramework.Ecs.Arch、GFramework.Ecs.Arch.Abstractions和GFramework.Ecs.Arch.Tests项目，并将其接入解决方案及顶层项目引用中。Documentation:
GFramework.Ecs.Arch包新增 README，记录特性、配置方式和集成模式。Tests:
GFramework.Ecs.Arch.Tests项目，并将 ECS 基础、高级和集成测试迁移至此，与新的命名空间保持一致。EnableEcs配置开关相关测试，因为在显式模块注册后已不再适用。Chores:
ArchitectureProperties中移除EnableEcs属性，因为 ECS 现已通过显式模块注册启用。Original summary in English
Summary by Sourcery
Extract Arch-based ECS into a dedicated GFramework.Ecs.Arch module with explicit, opt‑in registration and integration APIs.
New Features:
Enhancements:
Build:
Documentation:
Tests:
Chores: