Skip to content

Feature/game content config system#227

Merged
GeWuYou merged 5 commits into
mainfrom
feature/game-content-config-system
Apr 16, 2026
Merged

Feature/game content config system#227
GeWuYou merged 5 commits into
mainfrom
feature/game-content-config-system

Conversation

@GeWuYou

@GeWuYou GeWuYou commented Apr 16, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

发布说明

  • 新功能

    • 支持 durationtime 字符串格式的校验与文档输出。
    • 在配置校验与生成链中实现 JSON Schema 的 not 约束,拒绝匹配被禁止的子模式。
  • 文档

    • 更新配置系统文档,说明 not 语义与新增格式在运行时/编辑器/生成文档中的表现。
  • 测试

    • 扩展单元与集成测试,新增针对 duration/timenot 的全面覆盖。
  • 其他

    • 新增本地化校验消息键与中英文翻译用于 not 违规提示。

GeWuYou added 2 commits April 16, 2026 09:16
- 新增游戏内容配置系统详细介绍文档
- 包含 YAML 配置源文件支持说明
- 提供 JSON Schema 结构描述功能说明
- 说明一对象一文件的目录组织方式
- 介绍运行时只读查询功能特性
- 详细说明 Runtime / Generator / Tooling 共享支持的约束类型
- 提供 Source Generator 生成配置类型的完整说明
- 包含 VS Code 插件功能详细介绍
- 提供推荐目录结构和 Schema 示例
- 说明 YAML 示例格式和接入模板
- 详细说明 Godot 文本配置桥接功能
- 提供运行时读取模板和生成查询辅助说明
- 包含 Architecture 接入模板和热重载配置说明
- 详细说明运行时校验行为和跨表引用机制
- 提供开发期热重载功能完整配置指南
- 说明生成器接入约定和 VS Code 工具功能
- 包含当前限制和独立 Config Studio 评估说明
- 新增配置验证 JavaScript 工具实现
- 新增游戏内容配置系统详细介绍文档
- 包含 YAML 配置源文件和 JSON Schema 结构描述说明
- 提供推荐目录结构和 Schema 示例配置
- 添加官方启动帮助器 GameConfigBootstrap 使用指南
- 包含 Godot 文本配置桥接和运行时读取模板
- 提供 Architecture 推荐接入模板和热重载配置说明
- 添加运行时校验行为和开发期热重载功能说明
- 包含生成器接入约定和 VS Code 工具使用指南
- 新增 JavaScript 配置验证实现和格式校验模式
- 添加字符串格式校验包括 date、email、uuid 等类型
- 实现配置字段可编辑性检测和批量编辑功能支持
@coderabbitai

coderabbitai Bot commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 991156fd-f3e9-422a-a319-9851f1a8be95

📥 Commits

Reviewing files that changed from the base of the PR and between ebc5351 and a14e736.

📒 Files selected for processing (1)
  • GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs
🚧 Files skipped from review as they are similar to previous changes (1)
  • GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs
📜 Recent review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Analyze (C#)
  • GitHub Check: Build and Test
  • GitHub Check: Code Quality & Security

📝 Walkthrough

Walkthrough

为 JSON Schema 验证器、代码生成器与配置工具添加对字符串格式 durationtime 的支持,并在解析/验证/文档生成/本地化中实现模式级别的 not 否定子模式(解析、严格匹配与运行时/工具链诊断均已实现并新增测试覆盖)。

Changes

Cohort / File(s) Summary
C# 验证器与测试
GFramework.Game/Config/YamlConfigSchemaValidator.cs, GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs, GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs
增加 duration/time 格式(枚举、白名单、正则与匹配方法);在 YamlConfigSchemaNode 中支持并存储 negated (not) 子模式(NegatedSchemaNodeWithNegatedSchemaNode);在标量/数组/对象验证路径中执行严格的 not 约束检查并抛出 ConstraintViolation;重构匹配为 TryMatchSchemaNode(..., allowUnknownObjectProperties);新增/扩展针对格式与 not 的单元测试(含负向测试夹具)。
源生成器与测试
GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs, GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
扩展生成器支持的格式白名单(加入 durationtime);在生成/验证过程中递归检测 not 并在诊断路径显示 ${displayPath}[not];统一内联子模式摘要生成(用于 containsnot);新增测试覆盖 not 文档输出与诊断场景,并更新格式相关断言。
配置工具验证 (JS) 与测试
tools/gframework-config-tool/src/configValidation.js, tools/gframework-config-tool/test/configValidation.test.js
duration/time 添加正则验证器并注册到 SupportedStringFormats;在解析的 SchemaNode 中加入可选 not 字段并解析对象值;实现 validateNotSchemaMatch,在验证时以严格对象属性匹配评估 not 并产生 notViolation 诊断;调整 matchesSchemaNode 以支持 allowUnknownObjectProperties 标志;更新并扩展测试覆盖与断言索引。
本地化
tools/gframework-config-tool/src/localizationKeys.js, tools/gframework-config-tool/src/localization.js, tools/gframework-config-tool/test/localization.test.js
新增本地化键 ValidationMessageKeys.notViolation 并为英文/中文字典添加 not 约束违反消息;新增测试验证本地化输出。
文档
docs/zh-CN/game/config-system.md
将支持关键字列表与运行时行为文档更新为包含 not;扩展并对齐 format 支持说明以包含 durationtime,并说明 not 的严格匹配语义。

Sequence Diagram(s)

sequenceDiagram
    actor Developer as 开发者
    participant Generator as 源生成器
    participant Validator as 验证器
    participant Tool as 配置工具
    participant Loader as YamlConfigLoader
    participant Localizer as 本地化

    rect rgba(200,200,255,0.5)
    Developer->>Generator: 提供 schema (含 format/not)
    Generator->>Validator: 嵌入/校验格式与 not 元数据 (diagnostics / doc)
    end

    rect rgba(200,255,200,0.5)
    Tool->>Validator: 解析 schema -> SchemaNode (包含 not)
    Tool->>Tool: 注册 format 验证器 (duration/time)
    Tool->>Localizer: 请求 notViolation 文本
    end

    rect rgba(255,200,200,0.5)
    Loader->>Validator: 验证 YAML 数据 对象/数组/标量
    Validator->>Validator: 严格匹配 not 子模式 (allowUnknownObjectProperties=false)
    Validator-->>Loader: 抛出 ConstraintViolation (或 通过)
    Loader->>Developer: 返回加载结果 / 诊断
    end
Loading

估算代码审查工作量

🎯 4 (复杂) | ⏱️ ~50 分钟

可能相关的 PR

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive PR标题"Feature/game content config system"过于宽泛且未准确反映核心变更,即对JSON Schema not约束和duration/time格式支持的实现。 建议将标题改为更具体的描述,如"Add JSON Schema 'not' constraint and 'duration'/'time' format support"或"Implement 'not' schema validation and format extensions",以准确反映实际变更内容。
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/game-content-config-system

Comment @coderabbitai help to get the list of available commands and usage tips.

- 新增 YAML 配置源文件支持说明
- 添加 JSON Schema 结构描述功能介绍
- 实现一对象一文件的目录组织方式
- 提供运行时只读查询机制说明
- 添加 Source Generator 类型生成功能文档
- 集成 VS Code 插件配置浏览功能说明
- 添加跨表引用和校验行为详细说明
- 提供热重载开发期工具使用指南
- 完善 Godot 引擎文本配置桥接文档
- 补充 Architecture 模块接入模板说明

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs (1)

673-705: ⚠️ Potential issue | 🟠 Major

not 里的 format 校验现在只覆盖数组节点。

这里的 return true; 会让非 array schema 提前退出,所以新增的 not 递归分支实际上只会在数组节点上运行。像 {"type":"string","not":{"type":"string","format":"ipv4"}} 这类 schema 仍然会被生成器放过,和这次变更目标不一致。建议把 not 的递归校验移到数组早退之前,或把 items / contains / not 分开按节点类型处理。

💡 可参考的修正方向
-        if (!string.Equals(schemaType, "array", StringComparison.Ordinal))
-        {
-            return true;
-        }
-
-        if (element.TryGetProperty("items", out var itemsElement) &&
+        if (element.TryGetProperty("not", out var notElement) &&
+            notElement.ValueKind == JsonValueKind.Object &&
+            !TryValidateStringFormatMetadataRecursively(
+                filePath,
+                $"{displayPath}[not]",
+                notElement,
+                out diagnostic))
+        {
+            return false;
+        }
+
+        if (!string.Equals(schemaType, "array", StringComparison.Ordinal))
+        {
+            return true;
+        }
+
+        if (element.TryGetProperty("items", out var itemsElement) &&
             itemsElement.ValueKind == JsonValueKind.Object &&
             !TryValidateStringFormatMetadataRecursively(filePath, $"{displayPath}[]", itemsElement, out diagnostic))
         {
             return false;
         }
@@
-        if (element.TryGetProperty("not", out var notElement) &&
-            notElement.ValueKind == JsonValueKind.Object &&
-            !TryValidateStringFormatMetadataRecursively(
-                filePath,
-                $"{displayPath}[not]",
-                notElement,
-                out diagnostic))
-        {
-            return false;
-        }
-
         return true;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs` around lines 673
- 705, The current logic in TryValidateStringFormatMetadataRecursively returns
early for non-array schemas, so the recursive check for "not" only runs for
array nodes; move the "not" recursive validation so it executes for all node
types (either by moving the if-block that checks element.TryGetProperty("not",
out var notElement) / TryValidateStringFormatMetadataRecursively(...) to before
the early "if (!string.Equals(schemaType, \"array\", ...)) return true;" or by
splitting handling so items/contains are checked only for arrays while "not" is
validated unconditionally), preserving the same parameters (filePath,
$"{displayPath}[not]", notElement, out diagnostic) and returning false if that
recursive call fails.
🧹 Nitpick comments (3)
tools/gframework-config-tool/test/configValidation.test.js (1)

1826-1857: 建议再补一个对象型 not 的“完整命中即报错”正例。

当前这条用例只验证了“不要按 contains 式子集匹配”。如果对象 not 分支整体失效,这个测试仍然会通过。再加一个 reward 只包含 gold 且应触发 notViolation 的用例,会更稳。

🧪 可补的测试形态
+test("validateParsedConfig should reject objects that fully match a forbidden not schema", () => {
+    const schema = parseSchemaContent(`
+        {
+          "type": "object",
+          "properties": {
+            "reward": {
+              "type": "object",
+              "not": {
+                "type": "object",
+                "required": ["gold"],
+                "properties": {
+                  "gold": { "type": "integer" }
+                }
+              },
+              "properties": {
+                "gold": { "type": "integer" },
+                "bonus": { "type": "integer" }
+              }
+            }
+          }
+        }
+    `);
+    const yaml = parseTopLevelYaml(`
+reward:
+  gold: 10
+`);
+
+    const diagnostics = validateParsedConfig(schema, yaml);
+    assert.equal(diagnostics.length, 1);
+    assert.match(diagnostics[0].message, /not/u);
+});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/gframework-config-tool/test/configValidation.test.js` around lines 1826
- 1857, Add a new positive test case that asserts an object-level "not" actually
triggers a notViolation: in the same test file add another test (or extend the
existing "validateParsedConfig should keep not object matching strict instead of
contains-style subset matching") that uses parseSchemaContent and
parseTopLevelYaml to build a YAML where reward contains only gold (e.g., reward:
gold: 10) and then call validateParsedConfig(schema, yaml) and assert the
diagnostics include a notViolation (non-empty, matching the not rule). Reference
parseSchemaContent, parseTopLevelYaml and validateParsedConfig to locate where
to create the schema/YAML and the assertion. Ensure the new case expects an
error for the object-level not (reward with only gold) rather than passing.
GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs (1)

1088-1183: 这个测试类该按主题拆了。

新增 not 用例本身没问题,但继续把字符串格式、否定约束、数组约束、跨表引用、热重载都堆在同一个 3800+ 行 fixture 里,后续定位失败和补覆盖都会越来越难。建议至少拆成 StringFormatsNegationArrayConstraintsCrossTableReferencesHotReload 这类子 fixture。

As per coding guidelines: Keep a single source file under roughly 800-1000 lines; if a file grows beyond that range, check whether responsibilities should be split before continuing.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs` around lines 1088 -
1183, The test file is too large and mixes unrelated test topics; extract the
negation-related tests into their own fixture. Create a new test class (e.g.,
NegationTests) and move LoadAsync_Should_Throw_When_Value_Matches_Not_Schema and
LoadAsync_Should_Throw_When_Not_Is_Not_An_Object into it, preserving the same
setup and assertions; if
CreateConfigFile/CreateSchemaFile/YamlConfigLoader/_rootPath are shared, factor
them into a shared base test class or test helper so both old and new fixtures
reuse that setup. Repeat this pattern to split other groups into
StringFormatsTests, ArrayConstraintsTests, CrossTableReferencesTests,
HotReloadTests, keeping each file under ~800–1000 lines. Ensure namespaces and
test attributes remain correct so tests still run.
GFramework.Game/Config/YamlConfigSchemaValidator.cs (1)

2520-2545: 时区偏移校验稍显宽松,但与现有模式保持一致。

当前实现允许 offsetHour <= 23,而实际 UTC 时区偏移最大为 ±14:00(太平洋线岛)。不过这与 date-time 格式的校验行为一致,且不太可能在实际配置场景中引发问题。

🔧 可选:收紧时区偏移范围校验
     var offsetHour = int.Parse(offset.AsSpan(1, 2), CultureInfo.InvariantCulture);
     var offsetMinute = int.Parse(offset.AsSpan(4, 2), CultureInfo.InvariantCulture);
-    return offsetHour <= 23 && offsetMinute <= 59;
+    return offsetHour <= 14 && offsetMinute <= 59;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game/Config/YamlConfigSchemaValidator.cs` around lines 2520 -
2545, The timezone-offset check in MatchesSupportedTimeFormat is too permissive
(allows offsetHour <= 23); update the validation to enforce real-world UTC
offset limits by parsing the offset part (still using SupportedTimeFormatRegex
and the existing offset AsSpan slices) and ensure the absolute offset hour is <=
14 and offset minute <= 59 (also ensure offset hour/minute are non-negative),
preserving the early return for "Z" and keeping other parsing logic in
MatchesSupportedTimeFormat unchanged.
🤖 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/YamlConfigLoaderTests.cs`:
- Around line 1091-1136: Add a complementary positive test to ensure values that
do NOT match the "not" subschema are accepted: create a new test (e.g.
LoadAsync_Should_Succeed_When_Value_Does_Not_Match_NotSchema) that uses the same
schema file but a config with name: "Allowed" (or any string other than
"Deprecated"), register the table with YamlConfigLoader.RegisterTable<int,
MonsterConfigStub>("monster", "monster", "schemas/monster.schema.json", static
config => config.Id), call await loader.LoadAsync(registry) and assert no
exception, registry.Count == 1, and the loaded MonsterConfigStub has the
expected Id/name; this locks the behavior of YamlConfigSchemaValidator (and the
test harness methods CreateConfigFile/CreateSchemaFile) so implementations that
erroneously reject non-matching "not" schemas will fail.

In `@GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs`:
- Around line 324-369: The test fails because the schema validator never
recurses into the "not" subschema, so the illegal "format" on hp[not] doesn't
emit GF_ConfigSchema_009; update the schema traversal/validation logic (the code
path invoked by SchemaGeneratorTestDriver.Run — e.g., the schema
walker/validator that emits diagnostics) to treat the JSON "not" keyword as a
nested schema node (like "properties"/"items"), recursively validate its
contents, and emit Diagnostic with Id "GF_ConfigSchema_009" (and the same
message text "Only 'string' properties can declare 'format'.") when a non-string
node inside "not" declares a "format". Ensure the location/target reported
includes the "not" path (hp[not]) so the existing test assertions succeed.

---

Outside diff comments:
In `@GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs`:
- Around line 673-705: The current logic in
TryValidateStringFormatMetadataRecursively returns early for non-array schemas,
so the recursive check for "not" only runs for array nodes; move the "not"
recursive validation so it executes for all node types (either by moving the
if-block that checks element.TryGetProperty("not", out var notElement) /
TryValidateStringFormatMetadataRecursively(...) to before the early "if
(!string.Equals(schemaType, \"array\", ...)) return true;" or by splitting
handling so items/contains are checked only for arrays while "not" is validated
unconditionally), preserving the same parameters (filePath,
$"{displayPath}[not]", notElement, out diagnostic) and returning false if that
recursive call fails.

---

Nitpick comments:
In `@GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs`:
- Around line 1088-1183: The test file is too large and mixes unrelated test
topics; extract the negation-related tests into their own fixture. Create a new
test class (e.g., NegationTests) and move
LoadAsync_Should_Throw_When_Value_Matches_Not_Schema and
LoadAsync_Should_Throw_When_Not_Is_Not_An_Object into it, preserving the same
setup and assertions; if
CreateConfigFile/CreateSchemaFile/YamlConfigLoader/_rootPath are shared, factor
them into a shared base test class or test helper so both old and new fixtures
reuse that setup. Repeat this pattern to split other groups into
StringFormatsTests, ArrayConstraintsTests, CrossTableReferencesTests,
HotReloadTests, keeping each file under ~800–1000 lines. Ensure namespaces and
test attributes remain correct so tests still run.

In `@GFramework.Game/Config/YamlConfigSchemaValidator.cs`:
- Around line 2520-2545: The timezone-offset check in MatchesSupportedTimeFormat
is too permissive (allows offsetHour <= 23); update the validation to enforce
real-world UTC offset limits by parsing the offset part (still using
SupportedTimeFormatRegex and the existing offset AsSpan slices) and ensure the
absolute offset hour is <= 14 and offset minute <= 59 (also ensure offset
hour/minute are non-negative), preserving the early return for "Z" and keeping
other parsing logic in MatchesSupportedTimeFormat unchanged.

In `@tools/gframework-config-tool/test/configValidation.test.js`:
- Around line 1826-1857: Add a new positive test case that asserts an
object-level "not" actually triggers a notViolation: in the same test file add
another test (or extend the existing "validateParsedConfig should keep not
object matching strict instead of contains-style subset matching") that uses
parseSchemaContent and parseTopLevelYaml to build a YAML where reward contains
only gold (e.g., reward: gold: 10) and then call validateParsedConfig(schema,
yaml) and assert the diagnostics include a notViolation (non-empty, matching the
not rule). Reference parseSchemaContent, parseTopLevelYaml and
validateParsedConfig to locate where to create the schema/YAML and the
assertion. Ensure the new case expects an error for the object-level not (reward
with only gold) rather than passing.
🪄 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: 6f0fbfe4-0109-4dba-8892-0dd3748b2e34

📥 Commits

Reviewing files that changed from the base of the PR and between c2ee220 and 0f5b3a9.

📒 Files selected for processing (10)
  • GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs
  • GFramework.Game/Config/YamlConfigSchemaValidator.cs
  • GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
  • GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs
  • docs/zh-CN/game/config-system.md
  • tools/gframework-config-tool/src/configValidation.js
  • tools/gframework-config-tool/src/localization.js
  • tools/gframework-config-tool/src/localizationKeys.js
  • tools/gframework-config-tool/test/configValidation.test.js
  • tools/gframework-config-tool/test/localization.test.js
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Analyze (C#)
  • GitHub Check: Code Quality & Security
🧰 Additional context used
📓 Path-based instructions (6)
docs/**/*

📄 CodeRabbit inference engine (CLAUDE.md)

Documentation must be located in docs/ directory with Chinese content in docs/zh-CN/

Files:

  • docs/zh-CN/game/config-system.md
docs/**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

docs/**/*.md: Update relevant README.md or docs/ page when behavior, setup steps, architecture guidance, or user-facing examples change
Keep code samples, package names, and command examples in documentation aligned with the current repository state
For integration-oriented features such as the AI-First config system, documentation MUST cover project directory layout, file conventions, required project/package wiring, minimal working usage example, and migration/compatibility notes

Files:

  • docs/zh-CN/game/config-system.md
docs/zh-CN/**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

When a feature is added, removed, renamed, or substantially refactored, contributors MUST update or create the corresponding user-facing integration documentation in docs/zh-CN/ in the same change

Files:

  • docs/zh-CN/game/config-system.md
**/*.cs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.cs: LoggerGenerator must automatically generate log fields and logging helper methods for classes decorated with [Log] attribute
PriorityGenerator must generate priority comparison implementations for classes decorated with [Priority] attribute
EnumExtensionsGenerator must generate enum extension capabilities for enums decorated with [GenerateEnumExtensions] attribute
ContextAwareGenerator must automatically implement IContextAware boilerplate logic for classes decorated with [ContextAware] attribute

**/*.cs: All public, protected, and internal types and members MUST include XML documentation comments (///) with <summary>, <param>, <returns>, <exception>, and <remarks> where applicable
Add inline comments for non-trivial logic, concurrency/threading behavior, performance-sensitive paths, workarounds, compatibility constraints, and edge cases
Core framework components (Architecture, Module, System, Context, Registry, Service Module, Lifecycle types) MUST include high-level explanations of responsibilities, lifecycle, interactions, and design rationale
Generated logic and source generator pipelines MUST explain what is generated, why it is generated, semantic assumptions, and diagnostic or fallback behavior
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling
Do not rely on implicit imports; declare every required using explicitly in C# files
Write null-safe code that respects nullable annotations instead of suppressing warnings by default
Use the namespace pattern GFramework.{Module}.{Feature} with PascalCase segments
Follow standard C# naming: Types, methods, properties, events, constants use PascalCase; interfaces use I prefix; parameters and locals use camelCase; private fields use _camelCase
Use 4 spaces for indentation (not tabs), use Allman braces, and keep using directives at the top of the file sorted consistently
Prefer one primary type per file unless the surrounding project alread...

Files:

  • GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs
  • GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
  • GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs
  • GFramework.Game/Config/YamlConfigSchemaValidator.cs
**/*.{cs,csproj}

📄 CodeRabbit inference engine (AGENTS.md)

Every non-trivial feature, bug fix, or behavior change MUST include tests or an explicit justification for why a test is not practical

Files:

  • GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs
  • GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
  • GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs
  • GFramework.Game/Config/YamlConfigSchemaValidator.cs
**/*SourceGenerators/**/*.cs

📄 CodeRabbit inference engine (AGENTS.md)

Keep source generators deterministic and free of hidden environment or network dependencies

Files:

  • GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs
🧠 Learnings (9)
📚 Learning: 2026-04-14T01:59:40.041Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T01:59:40.041Z
Learning: Applies to docs/zh-CN/**/*.md : When a feature is added, removed, renamed, or substantially refactored, contributors MUST update or create the corresponding user-facing integration documentation in `docs/zh-CN/` in the same change

Applied to files:

  • docs/zh-CN/game/config-system.md
📚 Learning: 2026-04-14T01:59:40.041Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T01:59:40.041Z
Learning: Applies to docs/**/*.md : For integration-oriented features such as the AI-First config system, documentation MUST cover project directory layout, file conventions, required project/package wiring, minimal working usage example, and migration/compatibility notes

Applied to files:

  • docs/zh-CN/game/config-system.md
📚 Learning: 2026-04-14T01:59:40.041Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T01:59:40.041Z
Learning: Applies to **/*.Tests/*.cs : Source generator changes MUST be covered by generator tests; preserve snapshot-based verification patterns already used in the repository

Applied to files:

  • GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs
  • GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
  • GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs
📚 Learning: 2026-04-14T01:59:40.041Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T01:59:40.041Z
Learning: Applies to **/*.Tests/*.cs : When generator behavior changes intentionally, update snapshots together with the implementation

Applied to files:

  • GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs
  • GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
  • GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs
📚 Learning: 2026-04-06T12:45:43.921Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 190
File: GFramework.Game/Config/GameConfigBootstrap.cs:1-3
Timestamp: 2026-04-06T12:45:43.921Z
Learning: In the GeWuYou/GFramework repository, C# files may omit explicit `using System*` imports because the project-wide `GlobalUsings.cs` (referenced via manual global `using` directives) supplies common namespaces (e.g., `System`, `System.Threading`, `System.Threading.Tasks`). During code review, do not flag missing `using System...` directives in `.cs` files as long as `GlobalUsings.cs` is present/used to provide those namespaces.

Applied to files:

  • GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs
  • GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
  • GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs
  • GFramework.Game/Config/YamlConfigSchemaValidator.cs
📚 Learning: 2026-04-14T01:59:40.041Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T01:59:40.041Z
Learning: Applies to **/*.cs : Public API changes must be covered by unit or integration tests

Applied to files:

  • GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
  • GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs
📚 Learning: 2026-04-14T01:59:40.041Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T01:59:40.041Z
Learning: Applies to **/*SourceGenerators/**/*.cs : Keep source generators deterministic and free of hidden environment or network dependencies

Applied to files:

  • GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
📚 Learning: 2026-04-14T01:59:40.041Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T01:59:40.041Z
Learning: Applies to **/*.Tests/*.cs : Keep tests focused on observable behavior, not implementation trivia

Applied to files:

  • GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
📚 Learning: 2026-04-14T01:59:40.041Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T01:59:40.041Z
Learning: Applies to **/*.Tests/*.cs : Regression fixes should include a test that fails before the fix and passes after it

Applied to files:

  • GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs
🪛 GitHub Check: Build and Test
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs

[failure] 360-360: Failed Test: Run_Should_Report_Diagnostic_When_Not_Schema_Uses_Format_On_Non_String_Node
Run_Should_Report_Diagnostic_When_Not_Schema_Uses_Format_On_Non_String_Node: System.InvalidOperationException : Sequence contains no elements - at System.Linq.ThrowHelper.ThrowNoElementsException()
at System.Linq.Enumerable.Single[TSource](IEnumerable`1 source)
at GFramework.SourceGenerators.Tests.Config.SchemaConfigGeneratorTests.Run_Should_Report_Diagnostic_When_Not_Schema_Uses_Format_On_Non_String_Node() in /home/runner/work/GFramework/GFramework/GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs:line 360
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

  1. at System.Linq.ThrowHelper.ThrowNoElementsException()
    at System.Linq.Enumerable.Single[TSource](IEnumerable`1 source)
    at GFramework.SourceGenerators.Tests.Config.SchemaConfigGeneratorTests.Run_Should_Report_Diagnostic_When_Not_Schema_Uses_Format_On_Non_String_Node() in /home/runner/work/GFramework/GFramework/GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs:line 360
    at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
    at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
🪛 LanguageTool
docs/zh-CN/game/config-system.md

[uncategorized] ~775-~775: 数词与名词之间一般应存在量词,可能缺少量词。
Context: ...int 和生成代码 XML 文档复用;当前优先按运行时与 JS 共用的十进制精确整倍数判定处理常见十进制步进,并在必要时退回浮点容差兜底 - minLength ...

(wa5)


[uncategorized] ~775-~775: 您的意思是“"不"进”?
Context: ...L 文档复用;当前优先按运行时与 JS 共用的十进制精确整倍数判定处理常见十进制步进,并在必要时退回浮点容差兜底 - minLength / `maxLeng...

(BU)


[uncategorized] ~780-~780: 您的意思是“"不"日期”?
Context: ...08:30:00+08:00),避免不同宿主对 time-only 文本隐式补日期或本地时区 - 对上述共享子集,运行时会拒绝不满足格式的值,VS Code ...

(BU)

🔇 Additional comments (6)
tools/gframework-config-tool/src/configValidation.js (1)

1601-1833: containsnot 的匹配语义拆分得对。

这里把对象子集匹配只放宽给 contains,而 not 仍然走严格匹配,能避免额外字段把否定判断带偏,也和这次新增的验证语义保持一致。

GFramework.Game/Config/YamlConfigSchemaValidator.cs (5)

39-45: 新增的 durationtime 格式正则表达式实现正确。

Duration 正则配合 MatchesSupportedDurationFormat 的校验逻辑,正确实现了仅支持 day-time duration 的设计意图,拒绝了 Y/M(月)/W 等日历语义部分。Time 正则也符合 RFC 3339 规范要求显式时区偏移。


2810-2851: ValidateNegatedSchemaConstraint 正确使用严格匹配模式。

contains 使用宽松匹配(允许额外字段)不同,not 约束使用 allowUnknownObjectProperties: false 进行严格匹配,这在语义上是正确的——只有当 YAML 节点完全匹配被禁止的 schema 时才会触发违规。


563-599: ParseNegatedSchemaNode 实现规范,复用了现有的节点解析逻辑。

该方法正确检查 not 关键字的存在性和类型,并通过递归调用 ParseNode 保证 Runtime / Generator / Tooling 三端对 not 子 schema 的解释一致。


2755-2807: TryMatchSchemaNode 重构设计良好,正确处理引用收集的隔离。

通过临时集合收集引用,仅在匹配成功时才写回输出集合,避免了失败路径的引用泄漏。allowUnknownObjectProperties 参数使该方法能同时服务于 contains(宽松)和 not(严格)两种不同的匹配语义。


3196-3223: CollectReferencedTableNames 中缺少对 NegatedSchemaNode 的递归处理。

当前实现递归收集 ReferenceTableNamePropertiesItemNodeContainsConstraints.ContainsNode 中的引用,但未处理 NegatedSchemaNode。虽然在语义上,被否定(not)的 schema 中的引用不会被实际使用(因为匹配该 schema 的值会被拒绝),但为了代码完整性和后续维护,建议:

  1. 如果这是有意为之,请在方法上添加注释说明为什么跳过 NegatedSchemaNode 递归
  2. 如果应该递归处理,请添加对应逻辑和测试用例

当前没有测试用例覆盖"negated schema 包含 ref-table"的场景。

Comment thread GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs Outdated
- 实现了对嵌套对象和数组元数据解析的测试
- 添加了常量值比较和对象排序一致性的验证测试
- 创建了对YAML解析器嵌套映射和复杂键名的支持测试
- 开发了针对缺失和未知属性的验证诊断测试
- 实现了对象数组项目验证和深度枚举匹配测试
- 添加了标量、对象、数组、整数和布尔型常量验证测试
- 创建了数字范围和字符串长度限制验证测试
- 实现了独占边界、模式匹配和数组项目计数验证
- 添加了支持的字符串格式验证(日期、时间、邮箱等)
- 创建了多重约束和唯一性检查验证测试
- 实现了包含匹配计数约束验证测试
- 添加了科学记数法和大数精度处理验证测试
- 创建了Unicode模式匹配和唯一项重复检测测试

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs (1)

70-74: 可选:抽取重复的 Loader/Registry 初始化,降低后续维护成本。

这几处初始化逻辑重复度较高;提取私有辅助方法后,测试主体会更聚焦在 schema 行为断言。

♻️ 可选重构示例
@@
-        var loader = new YamlConfigLoader(_rootPath)
-            .RegisterTable<int, MonsterConfigStub>("monster", "monster", "schemas/monster.schema.json",
-                static config => config.Id);
-        var registry = new ConfigRegistry();
+        var loader = CreateMonsterLoader();
+        var registry = CreateRegistry();
@@
-        var loader = new YamlConfigLoader(_rootPath)
-            .RegisterTable<int, MonsterRewardConfigStub>("monster", "monster", "schemas/monster.schema.json",
-                static config => config.Id);
-        var registry = new ConfigRegistry();
+        var loader = CreateMonsterRewardLoader();
+        var registry = CreateRegistry();
@@
+    private YamlConfigLoader CreateMonsterLoader()
+    {
+        return new YamlConfigLoader(_rootPath)
+            .RegisterTable<int, MonsterConfigStub>("monster", "monster", "schemas/monster.schema.json",
+                static config => config.Id);
+    }
+
+    private YamlConfigLoader CreateMonsterRewardLoader()
+    {
+        return new YamlConfigLoader(_rootPath)
+            .RegisterTable<int, MonsterRewardConfigStub>("monster", "monster", "schemas/monster.schema.json",
+                static config => config.Id);
+    }
+
+    private static ConfigRegistry CreateRegistry()
+    {
+        return new ConfigRegistry();
+    }

Also applies to: 120-123, 176-180, 233-237, 280-284

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs` around lines
70 - 74, Several tests repeat the same initialization of a YamlConfigLoader and
ConfigRegistry; extract a private helper (e.g., CreateLoaderAndRegistry or
SetupLoaderRegistry) that returns the configured YamlConfigLoader and new
ConfigRegistry to reduce duplication. Update each test that currently calls new
YamlConfigLoader(_rootPath).RegisterTable<int, MonsterConfigStub>("monster",
"monster", "schemas/monster.schema.json", static config => config.Id) and new
ConfigRegistry() (lines referenced) to call the helper instead, keeping the
RegisterTable invocation inside the helper so tests focus only on schema
assertions; ensure the helper returns the same types used by the tests
(YamlConfigLoader and ConfigRegistry).
🤖 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/YamlConfigLoaderNegationTests.cs`:
- Around line 70-74: Several tests repeat the same initialization of a
YamlConfigLoader and ConfigRegistry; extract a private helper (e.g.,
CreateLoaderAndRegistry or SetupLoaderRegistry) that returns the configured
YamlConfigLoader and new ConfigRegistry to reduce duplication. Update each test
that currently calls new YamlConfigLoader(_rootPath).RegisterTable<int,
MonsterConfigStub>("monster", "monster", "schemas/monster.schema.json", static
config => config.Id) and new ConfigRegistry() (lines referenced) to call the
helper instead, keeping the RegisterTable invocation inside the helper so tests
focus only on schema assertions; ensure the helper returns the same types used
by the tests (YamlConfigLoader and ConfigRegistry).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a2d5670d-f403-4060-ba90-a96ef2ca69fb

📥 Commits

Reviewing files that changed from the base of the PR and between 0f5b3a9 and ebc5351.

📒 Files selected for processing (4)
  • GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs
  • GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs
  • GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs
  • tools/gframework-config-tool/test/configValidation.test.js
🚧 Files skipped from review as they are similar to previous changes (3)
  • tools/gframework-config-tool/test/configValidation.test.js
  • GFramework.Game.Tests/Config/YamlConfigLoaderTests.cs
  • GFramework.SourceGenerators/Config/SchemaConfigGenerator.cs
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Analyze (C#)
  • GitHub Check: Code Quality & Security
🧰 Additional context used
📓 Path-based instructions (2)
**/*.cs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.cs: LoggerGenerator must automatically generate log fields and logging helper methods for classes decorated with [Log] attribute
PriorityGenerator must generate priority comparison implementations for classes decorated with [Priority] attribute
EnumExtensionsGenerator must generate enum extension capabilities for enums decorated with [GenerateEnumExtensions] attribute
ContextAwareGenerator must automatically implement IContextAware boilerplate logic for classes decorated with [ContextAware] attribute

**/*.cs: All public, protected, and internal types and members MUST include XML documentation comments (///) with <summary>, <param>, <returns>, <exception>, and <remarks> where applicable
Add inline comments for non-trivial logic, concurrency/threading behavior, performance-sensitive paths, workarounds, compatibility constraints, and edge cases
Core framework components (Architecture, Module, System, Context, Registry, Service Module, Lifecycle types) MUST include high-level explanations of responsibilities, lifecycle, interactions, and design rationale
Generated logic and source generator pipelines MUST explain what is generated, why it is generated, semantic assumptions, and diagnostic or fallback behavior
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling
Do not rely on implicit imports; declare every required using explicitly in C# files
Write null-safe code that respects nullable annotations instead of suppressing warnings by default
Use the namespace pattern GFramework.{Module}.{Feature} with PascalCase segments
Follow standard C# naming: Types, methods, properties, events, constants use PascalCase; interfaces use I prefix; parameters and locals use camelCase; private fields use _camelCase
Use 4 spaces for indentation (not tabs), use Allman braces, and keep using directives at the top of the file sorted consistently
Prefer one primary type per file unless the surrounding project alread...

Files:

  • GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs
**/*.{cs,csproj}

📄 CodeRabbit inference engine (AGENTS.md)

Every non-trivial feature, bug fix, or behavior change MUST include tests or an explicit justification for why a test is not practical

Files:

  • GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs
🧠 Learnings (6)
📚 Learning: 2026-04-14T01:59:40.041Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T01:59:40.041Z
Learning: Applies to **/*.Tests/*.cs : Source generator changes MUST be covered by generator tests; preserve snapshot-based verification patterns already used in the repository

Applied to files:

  • GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs
📚 Learning: 2026-04-14T01:59:40.041Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T01:59:40.041Z
Learning: Applies to **/*.Tests/*.cs : Regression fixes should include a test that fails before the fix and passes after it

Applied to files:

  • GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs
📚 Learning: 2026-04-14T01:59:40.041Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T01:59:40.041Z
Learning: Applies to **/*.Tests/*.cs : When generator behavior changes intentionally, update snapshots together with the implementation

Applied to files:

  • GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs
📚 Learning: 2026-04-14T01:59:40.041Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T01:59:40.041Z
Learning: Applies to **/*.{cs,csproj} : Every non-trivial feature, bug fix, or behavior change MUST include tests or an explicit justification for why a test is not practical

Applied to files:

  • GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs
📚 Learning: 2026-04-14T01:59:40.041Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-14T01:59:40.041Z
Learning: Applies to **/*.cs : Public API changes must be covered by unit or integration tests

Applied to files:

  • GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs
📚 Learning: 2026-04-06T12:45:43.921Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 190
File: GFramework.Game/Config/GameConfigBootstrap.cs:1-3
Timestamp: 2026-04-06T12:45:43.921Z
Learning: In the GeWuYou/GFramework repository, C# files may omit explicit `using System*` imports because the project-wide `GlobalUsings.cs` (referenced via manual global `using` directives) supplies common namespaces (e.g., `System`, `System.Threading`, `System.Threading.Tasks`). During code review, do not flag missing `using System...` directives in `.cs` files as long as `GlobalUsings.cs` is present/used to provide those namespaces.

Applied to files:

  • GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs
🔇 Additional comments (1)
GFramework.Game.Tests/Config/YamlConfigLoaderNegationTests.cs (1)

37-295: 这组 not 回归测试覆盖面和断言粒度都很扎实。

5 个场景同时覆盖成功/失败路径,并且校验了失败类型、DisplayPath、错误消息片段以及失败后注册表不落表,回归保护强度足够。

Based on learnings: "Regression fixes should include a test that fails before the fix and passes after it".

- 实现标量值命中not子schema时的异常抛出验证
- 验证值未命中not子schema时的正常加载行为
- 测试对象完整命中禁用schema时的约束失败触发
- 验证对象仅命中not schema属性子集时的正确处理
- 添加not声明为非对象值时的解析阶段拒绝验证
- 创建临时目录隔离测试环境避免用例间污染
- 实现配置文件和schema文件的动态创建功能
- 提供标量和对象两种not约束场景的测试加载器
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant