docs(config): 添加游戏内容配置系统文档和集成测试#186
Merged
Merged
Conversation
- 新增游戏内容配置系统完整文档，包含 YAML 配置、JSON Schema 结构描述 - 添加推荐目录结构和配置示例，支持怪物、物品、技能等静态内容管理 - 实现 Source Generator 自动生成配置类型、表包装和注册访问辅助功能 - 集成 VS Code 插件提供配置浏览、raw 编辑、schema 打开和校验功能 - 添加生成查询辅助，为顶层标量字段生成 FindBy* 与 TryFindFirstBy* 方法 - 实现开发期热重载功能，支持配置文件修改后自动刷新运行时表 - 添加跨表引用校验，支持 x-gframework-ref-table 声明的引用关系检查 - 新增集成测试验证生成器自动拾取 schema 并支持强类型访问入口 - 添加 IsExternalInit 类型支持低版本 .NET 框架的 init-only setter 功能
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| C# |  |
Apr 6, 2026 4:25a.m. | Review ↗ |
| Secrets | |
Apr 6, 2026 4:25a.m. | Review ↗ |
Contributor
📝 WalkthroughWalkthroughThis PR adds generated per-property query helpers (FindBy*/TryFindFirstBy*) for top-level scalar config fields, updates the monster schema/tests to exercise them, extends generator tests and docs, and includes minor whitespace/line-ending normalizations. Changes
Sequence Diagram(s)sequenceDiagram
participant Arch as ConsumerArchitecture
participant CReg as ConfigRegistry
participant Loader as YamlConfigLoader
participant FS as FileSystem
Arch->>CReg: Register ConfigRegistry (RegisterUtility)
Arch->>Loader: Construct with temp root
Arch->>Loader: LoadAsync(schema files)
Loader->>FS: Read monster.schema.json, slime.yaml, goblin.yaml
FS-->>Loader: Return schema and YAML contents
Loader->>CReg: Parse & Populate MonsterTable (calls generated Table wrappers)
CReg-->>Arch: MonsterTable available via Context.GetUtility<ConfigRegistry>()
Arch->>Arch: OnInitialize uses MonsterTable.FindByFaction(...)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
GFramework.Game.Tests/Config/ArchitectureConfigIntegrationTests.cs (1)
16-57: 建议复用现有 Architecture 测试基建，减少样板和生命周期分支。这里的初始化/销毁与目录治理逻辑较多，后续可考虑迁移到现有测试基类模式以提高一致性与可维护性。
Based on learnings: Reuse existing architecture test infrastructure when relevant:
ArchitectureTestsBase<T>,SyncTestArchitecture,AsyncTestArchitecture.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Game.Tests/Config/ArchitectureConfigIntegrationTests.cs` around lines 16 - 57, Replace the manual setup/teardown and temp-dir handling in ArchitectureConfigIntegrationTests with the shared test infrastructure: inherit from ArchitectureTestsBase<ConsumerArchitecture> (or use SyncTestArchitecture/AsyncTestArchitecture helper) so the base class manages lifecycle and temp config root; update the test to obtain a ConsumerArchitecture instance from the base (or create it via the provided Sync/Async factory), remove CreateTempConfigRoot/DeleteDirectoryIfExists, and call the base init/destroy helpers instead of directly calling InitializeAsync/DestroyAsync to eliminate duplicated boilerplate and lifecycle branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@GFramework.Game.Tests/Config/ArchitectureConfigIntegrationTests.cs`:
- Around line 81-88: 当前清理块只捕获 IOException，当调用 Directory.Delete(path, true)
在某些环境可能抛出 UnauthorizedAccessException，导致非预期失败；修改
ArchitectureConfigIntegrationTests.cs 中执行 Directory.Delete(path, true) 的
try-catch，使其同时捕获 UnauthorizedAccessException（或改为 catch (Exception)
但保留“best-effort”注释），确保删除失败被静默吞掉并保留现有注释和意图。
In `@GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs`:
- Around line 338-347: Add negative assertions to the test to ensure the
generator does not produce TryFindFirstBy* overloads for excluded fields: update
the Assert.Multiple block that currently checks Does.Not.Contain("FindById("),
"FindByDropItems(", "FindByTargetId(", "FindByReward(") to also assert
Does.Not.Contain("TryFindFirstById("),
Does.Not.Contain("TryFindFirstByDropItems("),
Does.Not.Contain("TryFindFirstByTargetId("), and
Does.Not.Contain("TryFindFirstByReward(") so the test verifies both FindBy* and
TryFindFirstBy* are not generated (referencing TryFindFirstByName and
TryFindFirstByHp as examples of the positive assertions to mirror).
---
Nitpick comments:
In `@GFramework.Game.Tests/Config/ArchitectureConfigIntegrationTests.cs`:
- Around line 16-57: Replace the manual setup/teardown and temp-dir handling in
ArchitectureConfigIntegrationTests with the shared test infrastructure: inherit
from ArchitectureTestsBase<ConsumerArchitecture> (or use
SyncTestArchitecture/AsyncTestArchitecture helper) so the base class manages
lifecycle and temp config root; update the test to obtain a ConsumerArchitecture
instance from the base (or create it via the provided Sync/Async factory),
remove CreateTempConfigRoot/DeleteDirectoryIfExists, and call the base
init/destroy helpers instead of directly calling InitializeAsync/DestroyAsync to
eliminate duplicated boilerplate and lifecycle branches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f8069136-12a1-4ccb-aad9-8bd8ef43c1d9
📒 Files selected for processing (8)
GFramework.Core.Abstractions/Internals/IsExternalInit.csGFramework.Game.Tests/Config/ArchitectureConfigIntegrationTests.csGFramework.Game.Tests/Config/GeneratedConfigConsumerIntegrationTests.csGFramework.Game.Tests/schemas/monster.schema.jsonGFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.SourceGenerators.Tests/Config/snapshots/SchemaConfigGenerator/MonsterTable.g.txtGFramework.SourceGenerators/Config/SchemaConfigGenerator.csdocs/zh-CN/game/config-system.md
- 添加 ArchitectureConfigIntegrationTests 测试验证配置注册表初始化功能 - 实现 SchemaConfigGenerator 源代码生成器解析JSON schema文件 - 生成配置类型类、配置表包装器和运行时绑定辅助代码 - 支持嵌套对象、对象数组、标量数组的类型安全访问 - 提供强类型的配置表查询方法如 FindByProperty 等 - 实现跨表引用元数据生成和验证机制
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
GFramework.Game.Tests/Config/ArchitectureConfigIntegrationTests.cs (1)
145-153: Blocking async call is acceptable here but worth documenting.The
.GetAwaiter().GetResult()at line 151 is necessary becauseOnInitialize()is synchronous (as shown in the Architecture lifecycle). This works in test code without aSynchronizationContext, but the pattern could cause deadlocks in other contexts (e.g., UI threads, ASP.NET Classic).Consider adding a brief inline comment explaining why synchronous blocking is used here:
📝 Optional: Add clarifying comment
protected override void OnInitialize() { RegisterUtility(Registry); var loader = new YamlConfigLoader(_configRoot) .RegisterMonsterTable(); + // OnInitialize is synchronous; block here since test runner has no SynchronizationContext. loader.LoadAsync(Registry).GetAwaiter().GetResult(); MonsterTable = Registry.GetMonsterTable(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Game.Tests/Config/ArchitectureConfigIntegrationTests.cs` around lines 145 - 153, Add a short inline comment in OnInitialize next to the loader.LoadAsync(Registry).GetAwaiter().GetResult() call explaining that synchronous blocking is used intentionally because OnInitialize is synchronous in the Architecture lifecycle and tests run without a SynchronizationContext; mention the risk of deadlocks in contexts with a SynchronizationContext (e.g., UI threads or classic ASP.NET) so future maintainers understand why GetAwaiter().GetResult() is necessary and to avoid using it outside test/setup code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@GFramework.Game.Tests/Config/ArchitectureConfigIntegrationTests.cs`:
- Around line 145-153: Add a short inline comment in OnInitialize next to the
loader.LoadAsync(Registry).GetAwaiter().GetResult() call explaining that
synchronous blocking is used intentionally because OnInitialize is synchronous
in the Architecture lifecycle and tests run without a SynchronizationContext;
mention the risk of deadlocks in contexts with a SynchronizationContext (e.g.,
UI threads or classic ASP.NET) so future maintainers understand why
GetAwaiter().GetResult() is necessary and to avoid using it outside test/setup
code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 952c07d8-1b9c-4223-b53b-2948282b92e5
📒 Files selected for processing (3)
GFramework.Game.Tests/Config/ArchitectureConfigIntegrationTests.csGFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.SourceGenerators/Config/SchemaConfigGenerator.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
- GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs
This was referenced Apr 6, 2026
This was referenced Apr 17, 2026
Merged
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 CodeRabbit
New Features
Documentation
Tests