Feature/architecture modularization and ecs #45
Merged
Conversation
- 将架构服务重构为模块化设计，引入ServiceModuleManager统一管理 - 新增EventBusModule、CommandExecutorModule、QueryExecutorModule等核心服务模块 - 实现ECS模块支持，可配置启用Entity Component System功能 - 在架构初始化过程中集成模块注册、初始化和销毁流程 - 更新架构属性配置，添加EnableEcs开关控制ECS功能启用 - 优化服务获取方式，从直接依赖改为通过容器动态获取 - 移除架构上下文中的ECS相关实现代码，统一由ECS模块管理
- 添加了对ArchitectureProperties和GFramework.Core.services的引用 - 实现了RegisterBuiltInServices方法用于注册内置服务 - 修改了构造函数测试以验证容器初始化而非所有服务 - 更新了EventBus、CommandExecutor和QueryExecutor的测试以验证注册后可用性 - 添加了AsyncQueryExecutor可用性测试 - 添加了未注册服务时EventBus为null的测试 - 在多个实例测试中添加了模块注册以确保独立性 - 添加了ModuleManager属性非空测试 - 实现了ECS配置开关控制模块注册的测试 - 移除了TestArchitectureContextV3中的硬编码服务实现 - 更新了ECS相关测试以直接注册EcsWorld到容器中 - 改进了ECS世界获取失败时的错误消息
Reviewer's GuideIntroduces a modular service architecture with a ServiceModuleManager and pluggable IServiceModule implementations (including an ECS module), wires Architecture and ArchitectureServices to use these modules based on ArchitectureProperties (notably EnableEcs), and updates tests to reflect container-based resolution and ECS configuration-driven behavior. Sequence diagram for architecture initialization with service modules and ECSsequenceDiagram
actor App
participant Architecture
participant ArchitectureServices
participant ServiceModuleManager
participant Container as IIocContainer
participant Environment
participant Module as IServiceModule
App->>Architecture: InitializeAsync()
activate Architecture
Architecture->>Architecture: InitializeInternalAsync(asyncMode)
activate Architecture
Architecture->>ArchitectureServices: get Services
Architecture->>ArchitectureServices: get Container
Architecture->>Architecture: get Configuration.ArchitectureProperties
Architecture->>ServiceModuleManager: RegisterBuiltInModules(Container, ArchitectureProperties)
activate ServiceModuleManager
ServiceModuleManager->>ServiceModuleManager: RegisterModule(EventBusModule)
ServiceModuleManager->>ServiceModuleManager: RegisterModule(CommandExecutorModule)
ServiceModuleManager->>ServiceModuleManager: RegisterModule(QueryExecutorModule)
ServiceModuleManager->>ServiceModuleManager: RegisterModule(AsyncQueryExecutorModule)
alt EnableEcs is true
ServiceModuleManager->>ServiceModuleManager: RegisterModule(EcsModule(enabled:true))
else EnableEcs is false
ServiceModuleManager->>ServiceModuleManager: skip EcsModule
end
ServiceModuleManager->>ServiceModuleManager: sort modules by Priority
loop for each enabled module
ServiceModuleManager->>Module: Register(Container)
Module->>Container: register services
end
deactivate ServiceModuleManager
Architecture->>Environment: Initialize()
Architecture->>Container: ExecuteServicesHook(Configurator)
Architecture->>ServiceModuleManager: InitializeAllAsync(asyncMode)
activate ServiceModuleManager
loop for each enabled module
alt asyncMode and Module is IAsyncInitializable
ServiceModuleManager->>Module: InitializeAsync()
Module-->>ServiceModuleManager: completed
else
ServiceModuleManager->>Module: Initialize()
end
end
deactivate ServiceModuleManager
Architecture-->>App: initialization completed
deactivate Architecture
Class diagram for modular service architecture and modulesclassDiagram
direction LR
class Architecture {
-ArchitectureServices Services
-IIocContainer Container
-ArchitectureConfiguration Configuration
-IEnvironment Environment
+ValueTask InitializeAsync()
-Task InitializeInternalAsync(bool asyncMode)
+ValueTask DestroyAsync()
}
class ArchitectureServices {
-IServiceModuleManager _moduleManager
-IArchitectureContext _context
+ArchitectureServices()
+IIocContainer Container
+IServiceModuleManager ModuleManager
+IEventBus EventBus
+ICommandExecutor CommandExecutor
+IQueryExecutor QueryExecutor
+IAsyncQueryExecutor AsyncQueryExecutor
+void SetContext(IArchitectureContext context)
+IArchitectureContext GetContext()
}
class IServiceModuleManager {
+void RegisterModule(IServiceModule module)
+void RegisterBuiltInModules(IIocContainer container, ArchitectureProperties properties)
+IReadOnlyList~IServiceModule~ GetModules()
+Task InitializeAllAsync(bool asyncMode)
+ValueTask DestroyAllAsync()
}
class ServiceModuleManager {
-ILogger _logger
-List~IServiceModule~ _modules
+void RegisterModule(IServiceModule module)
+void RegisterBuiltInModules(IIocContainer container, ArchitectureProperties properties)
+IReadOnlyList~IServiceModule~ GetModules()
+Task InitializeAllAsync(bool asyncMode)
+ValueTask DestroyAllAsync()
}
class IServiceModule {
<<interface>>
+string ModuleName
+int Priority
+bool IsEnabled
+void Register(IIocContainer container)
+void Initialize()
+ValueTask DestroyAsync()
}
class EventBusModule {
+string ModuleName
+int Priority
+bool IsEnabled
+void Register(IIocContainer container)
+void Initialize()
+ValueTask DestroyAsync()
}
class CommandExecutorModule {
+string ModuleName
+int Priority
+bool IsEnabled
+void Register(IIocContainer container)
+void Initialize()
+ValueTask DestroyAsync()
}
class QueryExecutorModule {
+string ModuleName
+int Priority
+bool IsEnabled
+void Register(IIocContainer container)
+void Initialize()
+ValueTask DestroyAsync()
}
class AsyncQueryExecutorModule {
+string ModuleName
+int Priority
+bool IsEnabled
+void Register(IIocContainer container)
+void Initialize()
+ValueTask DestroyAsync()
}
class EcsModule {
-EcsSystemRunner _ecsRunner
-EcsWorld _ecsWorld
+EcsModule(bool enabled)
+string ModuleName
+int Priority
+bool IsEnabled
+void Register(IIocContainer container)
+void Initialize()
+ValueTask DestroyAsync()
+IEcsWorld GetEcsWorld()
+EcsSystemRunner GetEcsRunner()
}
class ArchitectureProperties {
+bool AllowLateRegistration
+bool StrictPhaseValidation
+bool EnableEcs
}
class ArchitectureContext {
-IIocContainer _container
-Dictionary~Type,object~ _serviceCache
+ArchitectureContext(IIocContainer container)
+TService GetService~TService~()
+IMediator GetMediator()
+IEcsWorld GetEcsWorld()
+void RegisterEcsSystem~T~()
}
class IIocContainer {
<<interface>>
+void Register~TService~(TService instance)
+void RegisterPlurality~TService~(TService instance)
+TService Get~TService~()
+bool Contains~TService~()
+void Clear()
}
class IEventBus {
<<interface>>
}
class ICommandExecutor {
<<interface>>
}
class IQueryExecutor {
<<interface>>
}
class IAsyncQueryExecutor {
<<interface>>
}
class IEcsWorld {
<<interface>>
}
class EcsWorld {
+int EntityCount
+void Dispose()
}
class EcsSystemRunner {
}
Architecture --> ArchitectureServices : uses
ArchitectureServices --> IIocContainer : has
ArchitectureServices --> IServiceModuleManager : uses
ArchitectureServices ..> IEventBus : resolves from Container
ArchitectureServices ..> ICommandExecutor : resolves from Container
ArchitectureServices ..> IQueryExecutor : resolves from Container
ArchitectureServices ..> IAsyncQueryExecutor : resolves from Container
ServiceModuleManager ..|> IServiceModuleManager
ServiceModuleManager o--> IServiceModule : manages modules
EventBusModule ..|> IServiceModule
CommandExecutorModule ..|> IServiceModule
QueryExecutorModule ..|> IServiceModule
AsyncQueryExecutorModule ..|> IServiceModule
EcsModule ..|> IServiceModule
ServiceModuleManager --> ArchitectureProperties : uses
ServiceModuleManager --> IIocContainer : registers services
ServiceModuleManager --> EcsModule : conditionally creates
EcsModule --> EcsWorld : creates
EcsModule ..> IEcsWorld : registers
EcsModule --> EcsSystemRunner : resolves and manages
Architecture --> ArchitectureProperties : uses in Configuration
Architecture --> ServiceModuleManager : via Services.ModuleManager
ArchitectureContext --> IIocContainer : uses
ArchitectureContext ..> IEcsWorld : resolves
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 - I've found 1 issue, and left some high level feedback:
- Accessors like
EventBus,CommandExecutor,QueryExecutor, andAsyncQueryExecutornow callContainer.Get<T>()!and will throw if built-in modules were not registered; consider either enforcing module registration in theArchitectureServicesconstructor or failing with a clearer, intentional exception when the service is missing. ServiceModuleManager.RegisterBuiltInModulesis not idempotent with respect to the container: repeated calls will re-runmodule.Register(container)for already-registered modules; if this method can be invoked more than once per container, consider tracking whether registration has already occurred or guarding against duplicate registrations.- In
Architecture, theServicesfield has been narrowed fromIArchitectureServicesto the concreteArchitectureServicesto accessModuleManager; to preserve abstraction, consider exposingModuleManageronIArchitectureServicesinstead of downcasting to the concrete type.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Accessors like `EventBus`, `CommandExecutor`, `QueryExecutor`, and `AsyncQueryExecutor` now call `Container.Get<T>()!` and will throw if built-in modules were not registered; consider either enforcing module registration in the `ArchitectureServices` constructor or failing with a clearer, intentional exception when the service is missing.
- `ServiceModuleManager.RegisterBuiltInModules` is not idempotent with respect to the container: repeated calls will re-run `module.Register(container)` for already-registered modules; if this method can be invoked more than once per container, consider tracking whether registration has already occurred or guarding against duplicate registrations.
- In `Architecture`, the `Services` field has been narrowed from `IArchitectureServices` to the concrete `ArchitectureServices` to access `ModuleManager`; to preserve abstraction, consider exposing `ModuleManager` on `IArchitectureServices` instead of downcasting to the concrete type.
## Individual Comments
### Comment 1
<location> `GFramework.Core/architecture/Architecture.cs:71` </location>
<code_context>
- /// 获取架构服务对象
- /// </summary>
- private IArchitectureServices Services { get; } = services ?? new ArchitectureServices();
+ private ArchitectureServices Services { get; } = services as ArchitectureServices ?? new ArchitectureServices();
/// <summary>
</code_context>
<issue_to_address>
**issue (bug_risk):** Casting `services` to `ArchitectureServices` silently ignores non-concrete implementations of `IArchitectureServices`.
Because the constructor accepts `IArchitectureServices?` but the property is now `ArchitectureServices` and initialized via `services as ArchitectureServices ?? new ArchitectureServices()`, any custom `IArchitectureServices` implementation will be ignored and replaced with a new `ArchitectureServices`. This breaks the abstraction and can change caller behavior unexpectedly. Either change the parameter type to `ArchitectureServices?` if a concrete type is required, or keep the property as `IArchitectureServices` and store/use the instance as provided.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- 将 ArchitectureServices 类型改为 IArchitectureServices 接口 - 简化 IArchitectureServices 接口中属性声明的访问修饰符 - 为 IArchitectureServices 接口添加模块管理器属性
- 为接口添加总体功能描述注释 - 为RegisterModule方法添加参数说明注释 - 为RegisterBuiltInModules方法添加容器和属性参数说明注释 - 为GetModules方法添加返回值说明注释 - 为InitializeAllAsync方法添加异步模式参数和返回值说明注释 - 为DestroyAllAsync方法添加返回值说明注释
- 在 ServiceModuleManager 中添加 _builtInModulesRegistered 标志位 - 实现重复注册检测逻辑，避免内置模块被多次注册 - 更新 RegisterBuiltInModules 方法的文档注释 - 在销毁所有模块时重置注册标志位 - 优化方法参数命名和描述信息 - 改进代码结构和注释内容
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
Introduce a modular service module system for core architecture services and make ECS support configurable via architecture properties and modules.
New Features:
Enhancements:
Tests: