Skip to content

feat(ui): 添加UI路由基类和相关接口定义#249

Merged
GeWuYou merged 6 commits into
mainfrom
feat/ui-router-base
Apr 17, 2026
Merged

feat(ui): 添加UI路由基类和相关接口定义#249
GeWuYou merged 6 commits into
mainfrom
feat/ui-router-base

Conversation

@GeWuYou

@GeWuYou GeWuYou commented Apr 17, 2026

Copy link
Copy Markdown
Owner
  • 实现UiRouterBase基类,提供页面栈管理和层级UI管理功能
  • 定义IUiPageBehavior接口,规范UI页面生命周期方法和状态管理
  • 定义IUiRouter接口,统一UI界面导航和切换操作规范
  • 实现页面栈操作功能,包括Push、Pop、Replace、Clear等方法
  • 实现层级UI管理功能,支持Overlay、Modal、Toast等浮层显示
  • 集成UI过渡管道,支持UI切换动画和逻辑处理
  • 添加暂停令牌管理,实现页面可见性驱动的暂停控制
  • 实现UI动作捕获和分发机制,支持语义动作处理

Summary by CodeRabbit

  • 新功能

    • 引入统一的 UI 语义动作(None/取消/确认)与位掩码,支持页面声明捕获与路由
    • 新的交互配置对象与暂停模式,页面可在运行时提供交互语义并控制是否阻挡世界输入与暂停行为
    • 路由器新增按优先级查询与派发语义动作,并报告世界指针/动作输入阻塞;旧派发接口被标记为过期
  • 页面基类

    • 基类尊重交互配置、切换处理模式并支持语义动作处理委托
  • 测试

    • 新增回归测试覆盖交互配置、路由派发与页面恢复生命周期

- 实现UiRouterBase基类,提供页面栈管理和层级UI管理功能
- 定义IUiPageBehavior接口,规范UI页面生命周期方法和状态管理
- 定义IUiRouter接口,统一UI界面导航和切换操作规范
- 实现页面栈操作功能,包括Push、Pop、Replace、Clear等方法
- 实现层级UI管理功能,支持Overlay、Modal、Toast等浮层显示
- 集成UI过渡管道,支持UI切换动画和逻辑处理
- 添加暂停令牌管理,实现页面可见性驱动的暂停控制
- 实现UI动作捕获和分发机制,支持语义动作处理
@coderabbitai

coderabbitai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

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: a107f8a3-0f5a-4183-95f5-2d337440a7e3

📥 Commits

Reviewing files that changed from the base of the PR and between ef8530a and f711725.

📒 Files selected for processing (1)
  • GFramework.Game.Tests/UI/UiRouterInteractionTests.cs
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.cs: All public, protected, and internal types and members in C# MUST include XML documentation comments (///) using <summary>, <param>, <returns>, <exception>, and <remarks> tags where applicable
XML documentation comments must explain intent, contract, and usage constraints instead of restating syntax
If a member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly in XML documentation
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds and edge cases, and registration order or lifecycle sequencing
Avoid obvious comments such as // increment i
Core framework components such as Architecture, Module, System, Context, Registry, Service Module, and Lifecycle types MUST include high-level explanations of responsibilities, lifecycle, interaction with other components, why the abstraction exists, and when to use it instead of alternatives
Generated logic and generator pipelines MUST explain what is generated, why it is generated, the semantic assumptions the generator relies on, and any diagnostics or fallback behavior
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling, if any
Comments MUST NOT be trivial, redundant, or misleading. Prefer explaining why and when, not just what
Code should remain understandable without requiring external context. Prefer slightly more explanation over too little for framework code
Missing required documentation is a coding standards violation. Code that does not meet the documentation rules is considered incomplete
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, and con...

Files:

  • GFramework.Game.Tests/UI/UiRouterInteractionTests.cs
🧠 Learnings (2)
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
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/UI/UiRouterInteractionTests.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/UI/UiRouterInteractionTests.cs
🔇 Additional comments (4)
GFramework.Game.Tests/UI/UiRouterInteractionTests.cs (4)

22-42: 默认交互配置的回归断言很扎实。

这里把 CapturedActionsBlocksWorldPointerInputBlocksWorldActionInput 一起锁住了,后续如果 Modal/Topmost 的默认 profile 发生语义漂移,这个用例会很快报出来。


44-72: 这条用例把“捕获即返回成功”的分发契约固定住了。

既校验返回值,又校验 TryHandleUiAction 只调用一次,能有效防止后续把 TryDispatchUiAction() 误改回“只有 handled 才返回 true”的语义。


74-107: 实例序号位宽溢出的排序回归也覆盖到了。

先把计数器推进到边界附近,再断言最新页面拿到捕获权,这条测试对 GetUiActionOwner() 的数值排序语义很有保护作用。


109-157: 恢复去重的两条回归路径覆盖得很好。

Resume()PopAsync() 都验证了 ResumeFromShow 页面只额外恢复一次,这正是最容易被后续改动重新引入的行为回归。 Based on learnings: Regression fixes should include a test that fails before the fix and passes after it


📝 Walkthrough

Walkthrough

新增统一 UI 语义输入与交互契约:引入语义动作枚举/掩码、交互配置与暂停模式;为页面添加交互配置与动作处理接口;扩展路由器以仲裁并分发语义动作、查询对世界输入的阻断并与暂停栈同步;更新 Godot 页面基类并新增单元测试覆盖。

Changes

Cohort / File(s) Summary
UI 语义与掩码
GFramework.Game.Abstractions/UI/UiInputAction.cs, GFramework.Game.Abstractions/UI/UiInputActionMask.cs
新增 UiInputAction 枚举(None/Cancel/Confirm)与标志位掩码 UiInputActionMask([Flags])。
交互配置类型
GFramework.Game.Abstractions/UI/UiInteractionProfile.cs, GFramework.Game.Abstractions/UI/UiPauseMode.cs
新增 UiInteractionProfile DTO(CapturedActions、阻止世界输入、Pause 配置等)与 UiPauseMode 枚举(None/WhileVisible)。
页面接口与提供者
GFramework.Game.Abstractions/UI/IUiPageBehavior.cs, GFramework.Game.Abstractions/UI/IUiActionHandler.cs, GFramework.Game.Abstractions/UI/IUiInteractionProfileProvider.cs
为页面行为添加 InteractionProfile 属性与 TryHandleUiAction 方法;新增独立的动作处理器接口 IUiActionHandler 与配置提供者 IUiInteractionProfileProvider
路由器接口
GFramework.Game.Abstractions/UI/IUiRouter.cs
接口新增 GetUiActionOwnerTryDispatchUiActionBlocksWorldPointerInputBlocksWorldActionInput;将旧 TryHandleUiAction 标记为 Obsolete。
路由器实现
GFramework.Game/UI/UiRouterBase.cs
实现可见页面优先级枚举与动作所有者选择、TryDispatchUiAction 分发逻辑与捕获日志、世界输入阻止查询;集成暂停栈(管理 PauseToken)、用 Pause 同步页面可见性/生命周期,新增清理逻辑;添加可见页面排序与实例序列解析。
运行时快捷配置
GFramework.Game/UI/UiInteractionProfiles.cs
新增常用配置 DefaultBlockingCancelCreateDefault(UiLayer)Captures(profile, action) 帮助方法。
Godot 页面基类
GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.cs
增加对 IUiInteractionProfileProvider/IUiActionHandler 的可选支持,公开 InteractionProfile,实现 TryHandleUiAction 转发;在 Pause/Resume 中同步未处理输入与 ProcessMode(支持暂停时继续处理)。
测试与全局 using
GFramework.Game.Tests/UI/UiRouterInteractionTests.cs, GFramework.Game.Tests/GlobalUsings.cs, GFramework.Game/GlobalUsings.cs, GFramework.Godot/GlobalUsings.cs
新增回归测试覆盖默认配置、分发/所有者选择与 resume 去重场景;为测试与模块添加若干 global using。

Sequence Diagram(s)

sequenceDiagram
  participant Input as InputSource
  participant Router as UiRouterBase
  participant Page as UiPage (IUiPageBehavior)
  participant Pause as PauseStackManager

  Input->>Router: Submit UiInputAction
  Router->>Router: GetUiActionOwner(action)
  alt owner found
    Router->>Page: TryHandleUiAction(action)
    Page-->>Router: true / false
    Router->>Pause: Query BlocksWorldPointerInput()/BlocksWorldActionInput()
    Router-->>Input: Dispatch result (consumed)
  else no owner
    Router-->>Input: Dispatch result (not consumed)
  end
Loading

预估代码审查工作量

🎯 4 (Complex) | ⏱️ ~45 分钟

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了PR的主要目标:实现UI路由基类和相关接口定义,与改动内容高度相关。
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 feat/ui-router-base

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

@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: 1

🧹 Nitpick comments (5)
GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.cs (1)

213-219: ApplyPauseAwareProcessingMode() 被重复调用

OnShow 在第 216 行调用 ApplyPauseAwareProcessingMode(),随后第 218 行调用 OnResume(),而 OnResume 内部(第 189 行)也会调用 ApplyPauseAwareProcessingMode()。这导致同一方法在 OnShow 流程中被执行两次。

虽然 ProcessMode 赋值是幂等的,不会导致功能问题,但冗余调用可以移除以保持代码简洁。

♻️ 建议移除 OnShow 中的冗余调用
     public virtual void OnShow()
     {
         _page?.OnShow();
-        ApplyPauseAwareProcessingMode();
         Owner.Show();
         OnResume();
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.cs` around lines 213 - 219,
在 OnShow 方法中移除对 ApplyPauseAwareProcessingMode() 的冗余调用;保留 _page?.OnShow(),
Owner.Show() 和随后调用的 OnResume()(OnResume 已会调用
ApplyPauseAwareProcessingMode()),因此只需删除 OnShow 中直接调用
ApplyPauseAwareProcessingMode() 即可,确保行为不变且避免重复执行。
GFramework.Game.Abstractions/UI/UiInteractionProfile.cs (2)

71-89: ModalTopmost 配置完全相同,存在代码重复。

可合并 switch 分支以减少重复:

♻️ 消除重复的建议
 public static UiInteractionProfile CreateDefault(UiLayer layer)
 {
     return layer switch
     {
-        UiLayer.Modal => new UiInteractionProfile
-        {
-            CapturedActions = UiInputActionMask.Cancel,
-            BlocksWorldPointerInput = true,
-            BlocksWorldActionInput = true
-        },
-        UiLayer.Topmost => new UiInteractionProfile
+        UiLayer.Modal or UiLayer.Topmost => new UiInteractionProfile
         {
             CapturedActions = UiInputActionMask.Cancel,
             BlocksWorldPointerInput = true,
             BlocksWorldActionInput = true
         },
         _ => Default
     };
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game.Abstractions/UI/UiInteractionProfile.cs` around lines 71 -
89, The CreateDefault method in UiInteractionProfile duplicates the same
initialization for UiLayer.Modal and UiLayer.Topmost; consolidate these branches
into a single case to remove repetition by matching both UiLayer.Modal and
UiLayer.Topmost to the same new UiInteractionProfile (preserving
CapturedActions, BlocksWorldPointerInput and BlocksWorldActionInput) and keep
the default fallback to Default; update the switch in CreateDefault accordingly.

9-90: Abstractions 项目包含运行时实现类。

根据编码规范,Abstractions 项目应仅包含接口和契约定义,不应包含运行时实现逻辑。UiInteractionProfile 是一个包含工厂方法 (CreateDefault) 和业务逻辑 (Captures) 的 sealed class,违反了此规范。

建议将此类移动到 GFramework.Game 项目中,或将其重构为纯数据传输对象,将工厂逻辑移至专门的工厂类。

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

In `@GFramework.Game.Abstractions/UI/UiInteractionProfile.cs` around lines 9 - 90,
UiInteractionProfile contains runtime logic (sealed class, CreateDefault factory
and Captures method) which violates the Abstractions-only rule; fix by removing
runtime behavior from the Abstractions assembly: either move the entire
UiInteractionProfile type (including CreateDefault and Captures) into the
GFramework.Game runtime project, or convert UiInteractionProfile into a pure
data DTO (e.g., immutable record with properties Default, CapturedActions,
BlocksWorldPointerInput, BlocksWorldActionInput, PauseMode, PauseGroup,
ContinueProcessingWhenPaused, PauseReason) and relocate the CreateDefault
factory and Captures logic into a runtime factory/class (e.g.,
UiInteractionProfileFactory or UiInteractionProfileExtensions) in
GFramework.Game; ensure all call sites referencing
UiInteractionProfile.CreateDefault or UiInteractionProfile.Captures are updated
to use the new factory/extension or the moved type.
GFramework.Game/UI/UiRouterBase.cs (2)

478-489: 方法名与返回值语义存在歧义。

TryHandleUiAction 的命名暗示返回值表示"是否成功处理了动作",但实际返回值表示"是否有页面捕获了该动作"(即使 owner.TryHandleUiAction 返回 false,方法仍返回 true)。

这可能导致调用方误解:认为返回 true 意味着动作已被成功处理。建议:

  1. 重命名为 TryDispatchUiActionTryCaptureUiAction 更准确反映语义;
  2. 或返回更丰富的结果类型区分"已捕获但未处理"与"已捕获且已处理"。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game/UI/UiRouterBase.cs` around lines 478 - 489, The method
TryHandleUiAction currently returns true whenever a page captures the action
even if owner.TryHandleUiAction(action) returned false, causing semantic
confusion; update this by either renaming TryHandleUiAction to
TryDispatchUiAction or TryCaptureUiAction to reflect "capture" semantics, or
change the signature to return a richer result (e.g. an enum or a tuple) that
distinguishes CapturedAndHandled vs CapturedButNotHandled vs NotCaptured; adjust
callers of TryHandleUiAction accordingly and update references to
GetUiActionOwner, owner.TryHandleUiAction, and the Log.Debug call to reflect the
new name or return type so callers can correctly interpret whether the action
was actually handled.

838-850: 实例 ID 排序依赖固定宽度格式,存在潜在风险。

当前使用 StringComparer.Ordinal 对实例 ID 进行排序来实现"最近显示优先"。这依赖于 GenerateInstanceId() 生成的固定宽度格式 ui_{id:D6}

_instanceCounter 超过 999999,生成的 ID 如 "ui_1000000""ui_0999999" 比较时,字符串排序结果将不正确("0" > "1" 在字符比较中不成立,但 "ui_1000000" 字典序实际小于 "ui_0999999" 的第5字符比较)。

考虑使用数值比较替代字符串比较,或在生成 ID 时使用更大的填充宽度:

♻️ 可选:提取数值进行比较
 private IEnumerable<IUiPageBehavior> EnumerateVisibleLayerPages(UiLayer layer)
 {
     if (!_layers.TryGetValue(layer, out var layerDict))
         yield break;

     foreach (var page in layerDict
-                 .OrderByDescending(static pair => pair.Key, StringComparer.Ordinal)
+                 .OrderByDescending(static pair => ExtractInstanceNumber(pair.Key))
                  .Select(static pair => pair.Value)
                  .Where(static page => page.IsAlive && page.IsVisible))
     {
         yield return page;
     }
 }
+
+private static int ExtractInstanceNumber(string instanceId)
+{
+    // instanceId format: "ui_000001"
+    return int.TryParse(instanceId.AsSpan(3), out var num) ? num : 0;
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game/UI/UiRouterBase.cs` around lines 838 - 850, The sorting for
recent-visible pages in EnumerateVisibleLayerPages relies on string ordering of
instance IDs (generated by GenerateInstanceId as "ui_{id:D6}") which breaks once
_instanceCounter exceeds the fixed width; change the sort to numeric ordering by
extracting the numeric suffix from the key (or, better, store the numeric
counter alongside the page and sort by that). Concretely: in
EnumerateVisibleLayerPages replace OrderByDescending(static pair => pair.Key,
StringComparer.Ordinal) with OrderByDescending using the parsed integer suffix
(e.g., parse after "ui_" or use int.TryParse on the appropriate substring) or
sort by the page's stored instanceCounter field if available, ensuring stable
and correct recent-first ordering even when _instanceCounter grows beyond
999999.
🤖 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/UI/UiRouterBase.cs`:
- Around line 758-768: TryBindPauseStackManager currently swallows
InvalidOperationException when resolving IPauseStackManager leaving
_pauseStackManager null without any trace; update the catch in
TryBindPauseStackManager to log a debug/error message via your logging facility
(or Debug.Log) including context like "IPauseStackManager not found" and the
exception message/stack so callers can see why PauseMode won't work, keeping the
existing behavior of setting _pauseStackManager = null.

---

Nitpick comments:
In `@GFramework.Game.Abstractions/UI/UiInteractionProfile.cs`:
- Around line 71-89: The CreateDefault method in UiInteractionProfile duplicates
the same initialization for UiLayer.Modal and UiLayer.Topmost; consolidate these
branches into a single case to remove repetition by matching both UiLayer.Modal
and UiLayer.Topmost to the same new UiInteractionProfile (preserving
CapturedActions, BlocksWorldPointerInput and BlocksWorldActionInput) and keep
the default fallback to Default; update the switch in CreateDefault accordingly.
- Around line 9-90: UiInteractionProfile contains runtime logic (sealed class,
CreateDefault factory and Captures method) which violates the Abstractions-only
rule; fix by removing runtime behavior from the Abstractions assembly: either
move the entire UiInteractionProfile type (including CreateDefault and Captures)
into the GFramework.Game runtime project, or convert UiInteractionProfile into a
pure data DTO (e.g., immutable record with properties Default, CapturedActions,
BlocksWorldPointerInput, BlocksWorldActionInput, PauseMode, PauseGroup,
ContinueProcessingWhenPaused, PauseReason) and relocate the CreateDefault
factory and Captures logic into a runtime factory/class (e.g.,
UiInteractionProfileFactory or UiInteractionProfileExtensions) in
GFramework.Game; ensure all call sites referencing
UiInteractionProfile.CreateDefault or UiInteractionProfile.Captures are updated
to use the new factory/extension or the moved type.

In `@GFramework.Game/UI/UiRouterBase.cs`:
- Around line 478-489: The method TryHandleUiAction currently returns true
whenever a page captures the action even if owner.TryHandleUiAction(action)
returned false, causing semantic confusion; update this by either renaming
TryHandleUiAction to TryDispatchUiAction or TryCaptureUiAction to reflect
"capture" semantics, or change the signature to return a richer result (e.g. an
enum or a tuple) that distinguishes CapturedAndHandled vs CapturedButNotHandled
vs NotCaptured; adjust callers of TryHandleUiAction accordingly and update
references to GetUiActionOwner, owner.TryHandleUiAction, and the Log.Debug call
to reflect the new name or return type so callers can correctly interpret
whether the action was actually handled.
- Around line 838-850: The sorting for recent-visible pages in
EnumerateVisibleLayerPages relies on string ordering of instance IDs (generated
by GenerateInstanceId as "ui_{id:D6}") which breaks once _instanceCounter
exceeds the fixed width; change the sort to numeric ordering by extracting the
numeric suffix from the key (or, better, store the numeric counter alongside the
page and sort by that). Concretely: in EnumerateVisibleLayerPages replace
OrderByDescending(static pair => pair.Key, StringComparer.Ordinal) with
OrderByDescending using the parsed integer suffix (e.g., parse after "ui_" or
use int.TryParse on the appropriate substring) or sort by the page's stored
instanceCounter field if available, ensuring stable and correct recent-first
ordering even when _instanceCounter grows beyond 999999.

In `@GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.cs`:
- Around line 213-219: 在 OnShow 方法中移除对 ApplyPauseAwareProcessingMode() 的冗余调用;保留
_page?.OnShow(), Owner.Show() 和随后调用的 OnResume()(OnResume 已会调用
ApplyPauseAwareProcessingMode()),因此只需删除 OnShow 中直接调用
ApplyPauseAwareProcessingMode() 即可,确保行为不变且避免重复执行。
🪄 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: ff305847-ec3b-46dd-8777-df2bb742e8e3

📥 Commits

Reviewing files that changed from the base of the PR and between 665b3e8 and 2e7fd1f.

📒 Files selected for processing (10)
  • GFramework.Game.Abstractions/UI/IUiActionHandler.cs
  • GFramework.Game.Abstractions/UI/IUiInteractionProfileProvider.cs
  • GFramework.Game.Abstractions/UI/IUiPageBehavior.cs
  • GFramework.Game.Abstractions/UI/IUiRouter.cs
  • GFramework.Game.Abstractions/UI/UiInputAction.cs
  • GFramework.Game.Abstractions/UI/UiInputActionMask.cs
  • GFramework.Game.Abstractions/UI/UiInteractionProfile.cs
  • GFramework.Game.Abstractions/UI/UiPauseMode.cs
  • GFramework.Game/UI/UiRouterBase.cs
  • GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.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 (AGENTS.md)

**/*.cs: All public, protected, and internal types and members in C# MUST include XML documentation comments (///) using <summary>, <param>, <returns>, <exception>, and <remarks> tags where applicable
XML documentation comments must explain intent, contract, and usage constraints instead of restating syntax
If a member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly in XML documentation
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds and edge cases, and registration order or lifecycle sequencing
Avoid obvious comments such as // increment i
Core framework components such as Architecture, Module, System, Context, Registry, Service Module, and Lifecycle types MUST include high-level explanations of responsibilities, lifecycle, interaction with other components, why the abstraction exists, and when to use it instead of alternatives
Generated logic and generator pipelines MUST explain what is generated, why it is generated, the semantic assumptions the generator relies on, and any diagnostics or fallback behavior
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling, if any
Comments MUST NOT be trivial, redundant, or misleading. Prefer explaining why and when, not just what
Code should remain understandable without requiring external context. Prefer slightly more explanation over too little for framework code
Missing required documentation is a coding standards violation. Code that does not meet the documentation rules is considered incomplete
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, and con...

Files:

  • GFramework.Game.Abstractions/UI/IUiInteractionProfileProvider.cs
  • GFramework.Game.Abstractions/UI/UiPauseMode.cs
  • GFramework.Game.Abstractions/UI/UiInputAction.cs
  • GFramework.Game.Abstractions/UI/IUiActionHandler.cs
  • GFramework.Game.Abstractions/UI/IUiPageBehavior.cs
  • GFramework.Game.Abstractions/UI/UiInputActionMask.cs
  • GFramework.Game.Abstractions/UI/IUiRouter.cs
  • GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.cs
  • GFramework.Game/UI/UiRouterBase.cs
  • GFramework.Game.Abstractions/UI/UiInteractionProfile.cs
**/*Abstractions/**/*.cs

📄 CodeRabbit inference engine (CLAUDE.md)

Abstractions projects should only contain interfaces and contract definitions without any runtime implementation logic

Files:

  • GFramework.Game.Abstractions/UI/IUiInteractionProfileProvider.cs
  • GFramework.Game.Abstractions/UI/UiPauseMode.cs
  • GFramework.Game.Abstractions/UI/UiInputAction.cs
  • GFramework.Game.Abstractions/UI/IUiActionHandler.cs
  • GFramework.Game.Abstractions/UI/IUiPageBehavior.cs
  • GFramework.Game.Abstractions/UI/UiInputActionMask.cs
  • GFramework.Game.Abstractions/UI/IUiRouter.cs
  • GFramework.Game.Abstractions/UI/UiInteractionProfile.cs
🧠 Learnings (2)
📚 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.Abstractions/UI/IUiInteractionProfileProvider.cs
  • GFramework.Game.Abstractions/UI/UiPauseMode.cs
  • GFramework.Game.Abstractions/UI/UiInputAction.cs
  • GFramework.Game.Abstractions/UI/IUiActionHandler.cs
  • GFramework.Game.Abstractions/UI/IUiPageBehavior.cs
  • GFramework.Game.Abstractions/UI/UiInputActionMask.cs
  • GFramework.Game.Abstractions/UI/IUiRouter.cs
  • GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.cs
  • GFramework.Game/UI/UiRouterBase.cs
  • GFramework.Game.Abstractions/UI/UiInteractionProfile.cs
📚 Learning: 2026-04-16T13:25:59.124Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 235
File: GFramework.SourceGenerators.Tests/Rule/ContextAwareGeneratorSnapshotTests.cs:1-3
Timestamp: 2026-04-16T13:25:59.124Z
Learning: In the GeWuYou/GFramework repository, the test project `GFramework.SourceGenerators.Tests` provides `global using NUnit.Framework;` via its `GlobalUsings.cs`. Do not flag missing `using NUnit.Framework;` directives in test files within this project, as the global using already covers it. The project-wide GlobalUsings.cs pattern is used broadly to supply common namespaces (including NUnit, System.IO, etc.) without per-file imports.

Applied to files:

  • GFramework.Game/UI/UiRouterBase.cs
🔇 Additional comments (17)
GFramework.Game.Abstractions/UI/UiPauseMode.cs (1)

1-17: LGTM!

枚举定义清晰,XML 文档完整说明了页面显示与暂停系统的协作模式。命名空间和代码风格符合规范。

GFramework.Game.Abstractions/UI/IUiInteractionProfileProvider.cs (1)

1-16: LGTM!

接口定义简洁,XML 文档清晰说明了运行时动态提供交互配置的用途。<param><returns> 标签完整。

GFramework.Game.Abstractions/UI/UiInputActionMask.cs (1)

1-23: LGTM!

位标记枚举定义正确,使用 [Flags] 属性并采用位移语法声明值。XML 文档完整。

GFramework.Game.Abstractions/UI/UiInputAction.cs (1)

1-23: LGTM!

枚举值与 UiInputActionMask 的位掩码值对齐(Cancel=1, Confirm=2),便于在路由仲裁中进行位运算检查。XML 文档清晰解释了语义动作与具体按键/设备事件解耦的设计意图。

GFramework.Game.Abstractions/UI/IUiActionHandler.cs (1)

1-17: LGTM!

接口定义清晰,返回值语义文档化良好:true 表示完成处理,false 时路由器仍将捕获声明视为已消费。这是重要的行为契约。

GFramework.Game.Abstractions/UI/IUiPageBehavior.cs (2)

69-72: LGTM!

新增的 InteractionProfile 属性为页面暴露输入、阻断与暂停配置提供了统一接口,与路由器可见性逻辑集成良好。


104-111: LGTM!

TryHandleUiAction 方法与 IUiActionHandler 契约一致,返回值语义文档清晰。

GFramework.Game.Abstractions/UI/IUiRouter.cs (1)

189-214: LGTM!

新增的四个方法完善了 UI 语义动作仲裁和 World 输入阻断查询能力:

  • GetUiActionOwner 查询动作捕获优先级
  • TryHandleUiAction 分发动作给页面
  • BlocksWorldPointerInput / BlocksWorldActionInput 判断输入阻断

XML 文档完整,契约清晰。

GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.cs (3)

38-65: LGTM!

通过类型转换从 owner 获取可选的 IUiInteractionProfileProviderIUiActionHandler 实现,设计灵活且符合组合优于继承的原则。字段和构造函数的 XML 文档完整。


129-134: LGTM!

InteractionProfile 属性使用空条件运算符优雅地处理了 _profileProvider 为 null 的情况,回退到 UiInteractionProfile.CreateDefault(Layer) 是合理的默认行为。


221-239: LGTM!

TryHandleUiActionApplyPauseAwareProcessingMode 方法实现正确:

  • TryHandleUiAction 使用空条件运算符安全委托,无处理器时返回 false
  • ApplyPauseAwareProcessingMode 根据 ContinueProcessingWhenPaused 配置切换 Godot 节点处理模式
GFramework.Game/UI/UiRouterBase.cs (5)

541-554: LGTM!

销毁时正确清理所有暂停令牌。使用 ToArray() 创建防御性副本避免迭代时修改集合,实现正确。


775-794: LGTM!

暂停请求同步逻辑正确处理了幂等性(不会重复推送令牌)和边界条件(管理器不可用、PauseMode 为 None)。


815-831: LGTM!

页面优先级枚举顺序合理:TopmostModalOverlay → 页面栈 → Toast。此顺序确保高优先级浮层(如模态框)优先捕获输入动作。


24-47: LGTM!

新增的暂停管理字段有完整的 XML 文档,可空标注正确。


462-509: LGTM!

UI 动作所有权查询和 World 输入阻断检查实现正确,复用了 EnumerateVisiblePagesByPriority 保持一致性。

GFramework.Game.Abstractions/UI/UiInteractionProfile.cs (1)

56-64: Captures 方法的当前实现是适当的,建议的位运算重构不适用于此枚举设计。

分析表明 UiInputAction 使用顺序值(0, 1, 2),而 UiInputActionMask 使用位标志值(0, 1, 2)。建议的位运算公式 1 << (int)action 会产生错误的映射:

  • 对于 Cancel (1):1 << 1 得到 2,对应 Confirm 而非 Cancel
  • 对于 Confirm (2):1 << 2 得到 4,在枚举中无对应值

枚举值的直接对应关系(UiInputAction 值可直接转换为 UiInputActionMask 值)使当前的 switch 表达式成为最清晰和最易维护的实现。当需要扩展新动作时,只需在两个枚举中添加新成员并在 switch 中增加对应分支。

			> Likely an incorrect or invalid review comment.

Comment thread GFramework.Game/UI/UiRouterBase.cs
- 实现UiRouterBase基类,提供页面栈管理和层级UI管理功能
- 定义IUiRouter接口,规范UI界面导航和切换操作
- 添加UI过渡管道机制,支持UI切换处理器注册和执行
- 实现页面栈操作方法,包括Push、Pop、Replace、Clear等
- 添加层级UI管理功能,支持Overlay、Modal、Toast等浮层显示
- 集成暂停管理功能,实现页面可见性驱动的暂停令牌管理
- 提供UI动作分发机制,支持语义动作捕获和分发
- 实现UI交互配置文件UiInteractionProfile,定义页面交互契约

@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.

Caution

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

⚠️ Outside diff range comments (1)
GFramework.Game/UI/UiRouterBase.cs (1)

1-6: ⚠️ Potential issue | 🔴 Critical

缺少 using GFramework.Core.Abstractions.Pause; 导致构建失败。

静态分析显示 PauseToken(第 25 行)和 IPauseStackManager(第 45 行)类型未找到。需要添加暂停模块的命名空间引用。

🐛 建议修复
 using GFramework.Core.Extensions;
 using GFramework.Game.Abstractions.Enums;
 using GFramework.Game.Abstractions.UI;
 using GFramework.Game.Routing;
+using GFramework.Core.Abstractions.Pause;
 
 namespace GFramework.Game.UI;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game/UI/UiRouterBase.cs` around lines 1 - 6, The build fails
because PauseToken and IPauseStackManager are unresolved; add the missing
namespace import by adding "using GFramework.Core.Abstractions.Pause;" alongside
the other using statements at the top of UiRouterBase.cs so references to
PauseToken (used around line ~25) and IPauseStackManager (used around line ~45)
resolve.
🧹 Nitpick comments (1)
GFramework.Game.Tests/UI/UiRouterInteractionTests.cs (1)

163-168: 测试路由器未调用 OnInit() 初始化生命周期。

CreateRouter() 仅绑定根节点,未调用 OnInit(),导致 _factory_pauseStackManager 未初始化。当前测试因避开了使用 _factory 的代码路径而能通过,但若未来测试扩展或路由器实现变更,可能产生脆弱性。

建议考虑在 TestUiRouter 中显式暴露初始化方法,或模拟必要依赖。

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

In `@GFramework.Game.Tests/UI/UiRouterInteractionTests.cs` around lines 163 - 168,
CreateRouter currently constructs TestUiRouter and only calls BindRoot, leaving
lifecycle initialization undone so OnInit isn't invoked and fields like _factory
and _pauseStackManager remain uninitialized; fix by invoking the router
initialization after binding the root (call the router's initialization method
or OnInit) or modify TestUiRouter to expose an explicit Init/Initialize method
that sets up _factory and _pauseStackManager (or provide mocks for those
dependencies) before returning from CreateRouter so all lifecycle state is ready
for tests that exercise those paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@GFramework.Game/UI/UiRouterBase.cs`:
- Around line 1-6: The build fails because PauseToken and IPauseStackManager are
unresolved; add the missing namespace import by adding "using
GFramework.Core.Abstractions.Pause;" alongside the other using statements at the
top of UiRouterBase.cs so references to PauseToken (used around line ~25) and
IPauseStackManager (used around line ~45) resolve.

---

Nitpick comments:
In `@GFramework.Game.Tests/UI/UiRouterInteractionTests.cs`:
- Around line 163-168: CreateRouter currently constructs TestUiRouter and only
calls BindRoot, leaving lifecycle initialization undone so OnInit isn't invoked
and fields like _factory and _pauseStackManager remain uninitialized; fix by
invoking the router initialization after binding the root (call the router's
initialization method or OnInit) or modify TestUiRouter to expose an explicit
Init/Initialize method that sets up _factory and _pauseStackManager (or provide
mocks for those dependencies) before returning from CreateRouter so all
lifecycle state is ready for tests that exercise those paths.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 14818a2e-46be-4cba-ad29-d3beda76489a

📥 Commits

Reviewing files that changed from the base of the PR and between 2e7fd1f and 053fd4a.

📒 Files selected for processing (6)
  • GFramework.Game.Abstractions/UI/IUiRouter.cs
  • GFramework.Game.Abstractions/UI/UiInteractionProfile.cs
  • GFramework.Game.Tests/UI/UiRouterInteractionTests.cs
  • GFramework.Game/UI/UiInteractionProfiles.cs
  • GFramework.Game/UI/UiRouterBase.cs
  • GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.cs
🚧 Files skipped from review as they are similar to previous changes (2)
  • GFramework.Game.Abstractions/UI/IUiRouter.cs
  • GFramework.Game.Abstractions/UI/UiInteractionProfile.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 (1)
**/*.cs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.cs: All public, protected, and internal types and members in C# MUST include XML documentation comments (///) using <summary>, <param>, <returns>, <exception>, and <remarks> tags where applicable
XML documentation comments must explain intent, contract, and usage constraints instead of restating syntax
If a member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly in XML documentation
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds and edge cases, and registration order or lifecycle sequencing
Avoid obvious comments such as // increment i
Core framework components such as Architecture, Module, System, Context, Registry, Service Module, and Lifecycle types MUST include high-level explanations of responsibilities, lifecycle, interaction with other components, why the abstraction exists, and when to use it instead of alternatives
Generated logic and generator pipelines MUST explain what is generated, why it is generated, the semantic assumptions the generator relies on, and any diagnostics or fallback behavior
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling, if any
Comments MUST NOT be trivial, redundant, or misleading. Prefer explaining why and when, not just what
Code should remain understandable without requiring external context. Prefer slightly more explanation over too little for framework code
Missing required documentation is a coding standards violation. Code that does not meet the documentation rules is considered incomplete
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, and con...

Files:

  • GFramework.Game/UI/UiInteractionProfiles.cs
  • GFramework.Game.Tests/UI/UiRouterInteractionTests.cs
  • GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.cs
  • GFramework.Game/UI/UiRouterBase.cs
🧠 Learnings (6)
📚 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/UI/UiInteractionProfiles.cs
  • GFramework.Game.Tests/UI/UiRouterInteractionTests.cs
  • GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.cs
  • GFramework.Game/UI/UiRouterBase.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.csproj : Public API changes must be covered by unit or integration tests

Applied to files:

  • GFramework.Game.Tests/UI/UiRouterInteractionTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
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/UI/UiRouterInteractionTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.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/UI/UiRouterInteractionTests.cs
📚 Learning: 2026-04-16T13:25:59.124Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 235
File: GFramework.SourceGenerators.Tests/Rule/ContextAwareGeneratorSnapshotTests.cs:1-3
Timestamp: 2026-04-16T13:25:59.124Z
Learning: In the GeWuYou/GFramework repository, the test project `GFramework.SourceGenerators.Tests` provides `global using NUnit.Framework;` via its `GlobalUsings.cs`. Do not flag missing `using NUnit.Framework;` directives in test files within this project, as the global using already covers it. The project-wide GlobalUsings.cs pattern is used broadly to supply common namespaces (including NUnit, System.IO, etc.) without per-file imports.

Applied to files:

  • GFramework.Game/UI/UiRouterBase.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.cs : Use the namespace pattern `GFramework.{Module}.{Feature}` with PascalCase segments

Applied to files:

  • GFramework.Game/UI/UiRouterBase.cs
🪛 GitHub Check: Build and Test
GFramework.Game/UI/UiRouterBase.cs

[failure] 25-25:
The type or namespace name 'PauseToken' could not be found (are you missing a using directive or an assembly reference?)


[failure] 25-25:
The type or namespace name 'PauseToken' could not be found (are you missing a using directive or an assembly reference?)


[failure] 25-25:
The type or namespace name 'PauseToken' could not be found (are you missing a using directive or an assembly reference?)


[failure] 45-45:
The type or namespace name 'IPauseStackManager' could not be found (are you missing a using directive or an assembly reference?)


[failure] 45-45:
The type or namespace name 'IPauseStackManager' could not be found (are you missing a using directive or an assembly reference?)


[failure] 45-45:
The type or namespace name 'IPauseStackManager' could not be found (are you missing a using directive or an assembly reference?)

🔇 Additional comments (10)
GFramework.Game/UI/UiInteractionProfiles.cs (1)

1-59: LGTM!设计合理,文档齐全。

该 helper 类将默认策略和语义判定保留在运行时层,避免反向依赖 Abstractions,符合架构分层原则。Captures 方法使用显式 switch 处理当前已定义的 CancelConfirm 动作,简洁高效。

GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.cs (3)

38-65: 接口检测与可选依赖注入设计良好。

通过 as 转换在构造时捕获可选接口引用(IUiInteractionProfileProviderIUiActionHandler),既避免了运行时重复类型检查,又保持了向后兼容——视图无需实现这些接口即可正常工作。


129-134: 交互配置回退逻辑正确。

当页面未实现 IUiInteractionProfileProvider 时,回退到 UiInteractionProfiles.CreateDefault(Layer),确保 Modal/Topmost 层默认获得阻塞型配置,与路由器语义保持一致。


220-238: 动作处理与暂停模式切换实现清晰。

TryHandleUiAction 的空值安全委托和 ApplyPauseAwareProcessingMode 根据 ContinueProcessingWhenPaused 设置节点处理模式的逻辑都很简洁。

GFramework.Game/UI/UiRouterBase.cs (3)

763-777: 暂停管理器绑定失败时已添加日志。

TryBindPauseStackManager 中的异常处理现在包含 Debug 日志,便于排查 PauseMode 配置不生效的问题。


459-518: 语义动作分发与阻断查询实现良好。

  • GetUiActionOwner 按优先级遍历可见页面并返回首个捕获该动作的页面
  • TryDispatchUiAction 区分"已捕获"与"已处理",并在未显式处理时记录日志
  • 旧方法 TryHandleUiAction 通过 [Obsolete] 提供清晰的迁移指引
  • BlocksWorldPointerInput/BlocksWorldActionInput 使用 Any() 实现短路求值

820-873: 可见页面优先级枚举逻辑正确。

  • 优先级顺序符合预期:Topmost → Modal → Overlay → Page 栈 → Toast
  • ExtractInstanceSequence 使用数值解析而非字符串比较,正确处理 _instanceCounter 超过 6 位宽度后的排序
  • 格式异常时返回 int.MinValue 将异常条目排到最后,是合理的降级策略
GFramework.Game.Tests/UI/UiRouterInteractionTests.cs (3)

28-43: 交互配置默认值测试覆盖全面。

验证了 Modal 和 Topmost 共享同一 BlockingCancel 实例、Cancel 动作被捕获、以及 World 输入阻断标志均为 true


78-108: 实例 ID 宽度溢出排序回归测试设计巧妙。

通过反射将 _instanceCounter 设置为 999998,随后显示两个页面分别获得 ui_999999ui_1000000 标识符,验证了数值排序而非字符串排序的正确性。


225-338: TestUiPage 测试替身设计良好。

  • ResumeFromShow 属性模拟 CanvasItemUiPageBehaviorBaseOnShow() 中触发 OnResume() 的行为
  • 计数器属性 (OnResumeCallCount, TryHandleUiActionCallCount) 便于验证调用次数
  • InteractionProfile 可配置,支持不同测试场景

coderabbitai[bot]
coderabbitai Bot previously approved these changes Apr 17, 2026
GeWuYou added 2 commits April 17, 2026 22:51
- 在GFramework.Game项目中添加暂停抽象接口的全局引用
- 在GFramework.Game项目中添加日志抽象接口的全局引用
- 从测试项目的UI路由交互测试文件中移除不必要的using声明
- 将测试项目的全局using声明统一到GlobalUsings.cs文件中
- 在GFramework.Godot项目中添加扩展方法的全局引用
- 优化全局命名空间引用的组织结构
- 验证模态层和顶层共享同一套阻塞型默认交互配置
- 验证页面捕获动作后路由分发返回成功的语义
- 验证层级页面排序使用实例自增序号而非字符串顺序
- 验证恢复挂起页面时避免重复调用 OnResume 的问题
- 验证弹出栈顶页面后恢复下层页面的重复触发问题
- 创建测试用路由器、根节点和可配置页面类
- 实现实例计数器设置功能以覆盖边界条件测试

@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 (2)
GFramework.Game.Tests/UI/UiRouterInteractionTests.cs (2)

33-39: 断言过度绑定到“同一实例”实现细节

这里用 Is.SameAs 会把测试绑定到对象复用策略;若后续改为返回“等价但不同实例”的配置对象,会出现误报。建议断言语义一致性而不是引用相等。

建议修改
         Assert.Multiple(() =>
         {
             Assert.That(modal.CapturedActions, Is.EqualTo(UiInputActionMask.Cancel));
             Assert.That(modal.BlocksWorldPointerInput, Is.True);
             Assert.That(modal.BlocksWorldActionInput, Is.True);
-            Assert.That(topmost, Is.SameAs(modal));
+            Assert.That(topmost.CapturedActions, Is.EqualTo(UiInputActionMask.Cancel));
+            Assert.That(topmost.BlocksWorldPointerInput, Is.True);
+            Assert.That(topmost.BlocksWorldActionInput, Is.True);
         });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game.Tests/UI/UiRouterInteractionTests.cs` around lines 33 - 39,
The test currently asserts reference equality with Assert.That(topmost,
Is.SameAs(modal)) which ties the test to instance reuse; instead assert semantic
equivalence of the topmost and modal configuration (for example compare a stable
identity or key property or use structural equality) inside the same
Assert.Multiple block. Locate the Assert.Multiple in UiRouterInteractionTests
(the modal and topmost locals) and replace the Is.SameAs assertion with an
assertion that checks a semantic property (e.g., Assert.That(topmost.Id,
Is.EqualTo(modal.Id)) or Assert.That(topmost, Is.EqualTo(modal)) assuming Equals
is implemented) so the test verifies behavior rather than object identity.

173-179: 建议降低对私有字段名反射的脆弱耦合

"_instanceCounter" 的硬编码反射在内部重命名或类型调整时会导致与业务语义无关的测试失败。若短期仍保留反射,至少补充字段类型断言和失败信息,减少排障成本。

最小增强(当前文件内可做)
     private static void SetInstanceCounter(UiRouterBase router, int value)
     {
         var field = typeof(UiRouterBase).GetField("_instanceCounter", BindingFlags.Instance | BindingFlags.NonPublic);
-        Assert.That(field, Is.Not.Null);
+        Assert.That(field, Is.Not.Null, "UiRouterBase._instanceCounter 字段未找到,可能发生了内部重构。");
+        Assert.That(field!.FieldType, Is.EqualTo(typeof(int)), "_instanceCounter 字段类型已变化,请同步调整测试。");
 
-        field!.SetValue(router, value);
+        field.SetValue(router, value);
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game.Tests/UI/UiRouterInteractionTests.cs` around lines 173 - 179,
SetInstanceCounter uses hard-coded reflection on UiRouterBase's private field
"_instanceCounter", which is brittle; update the method to include explicit
assertions and messages and a field-type check before setting: locate the field
via typeof(UiRouterBase).GetField("_instanceCounter", BindingFlags.Instance |
BindingFlags.NonPublic), Assert.That(field, Is.Not.Null, "Expected private field
'_instanceCounter' on UiRouterBase not found"), Assert.That(field.FieldType,
Is.EqualTo(typeof(int)), "Expected '_instanceCounter' to be of type int"), then
call field.SetValue(router, value); this preserves the reflection approach but
gives clearer failure diagnostics and guards against type changes in
SetInstanceCounter/UiRouterBase.
🤖 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/UI/UiRouterInteractionTests.cs`:
- Around line 33-39: The test currently asserts reference equality with
Assert.That(topmost, Is.SameAs(modal)) which ties the test to instance reuse;
instead assert semantic equivalence of the topmost and modal configuration (for
example compare a stable identity or key property or use structural equality)
inside the same Assert.Multiple block. Locate the Assert.Multiple in
UiRouterInteractionTests (the modal and topmost locals) and replace the
Is.SameAs assertion with an assertion that checks a semantic property (e.g.,
Assert.That(topmost.Id, Is.EqualTo(modal.Id)) or Assert.That(topmost,
Is.EqualTo(modal)) assuming Equals is implemented) so the test verifies behavior
rather than object identity.
- Around line 173-179: SetInstanceCounter uses hard-coded reflection on
UiRouterBase's private field "_instanceCounter", which is brittle; update the
method to include explicit assertions and messages and a field-type check before
setting: locate the field via typeof(UiRouterBase).GetField("_instanceCounter",
BindingFlags.Instance | BindingFlags.NonPublic), Assert.That(field, Is.Not.Null,
"Expected private field '_instanceCounter' on UiRouterBase not found"),
Assert.That(field.FieldType, Is.EqualTo(typeof(int)), "Expected
'_instanceCounter' to be of type int"), then call field.SetValue(router, value);
this preserves the reflection approach but gives clearer failure diagnostics and
guards against type changes in SetInstanceCounter/UiRouterBase.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c309936d-0367-42b9-a7b3-514688ff33c3

📥 Commits

Reviewing files that changed from the base of the PR and between 053fd4a and eec8dc8.

📒 Files selected for processing (5)
  • GFramework.Game.Tests/GlobalUsings.cs
  • GFramework.Game.Tests/UI/UiRouterInteractionTests.cs
  • GFramework.Game/GlobalUsings.cs
  • GFramework.Game/UI/UiRouterBase.cs
  • GFramework.Godot/GlobalUsings.cs
✅ Files skipped from review due to trivial changes (3)
  • GFramework.Game/GlobalUsings.cs
  • GFramework.Godot/GlobalUsings.cs
  • GFramework.Game.Tests/GlobalUsings.cs
🚧 Files skipped from review as they are similar to previous changes (1)
  • GFramework.Game/UI/UiRouterBase.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: Code Quality & Security
  • GitHub Check: Analyze (C#)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.cs: All public, protected, and internal types and members in C# MUST include XML documentation comments (///) using <summary>, <param>, <returns>, <exception>, and <remarks> tags where applicable
XML documentation comments must explain intent, contract, and usage constraints instead of restating syntax
If a member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly in XML documentation
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds and edge cases, and registration order or lifecycle sequencing
Avoid obvious comments such as // increment i
Core framework components such as Architecture, Module, System, Context, Registry, Service Module, and Lifecycle types MUST include high-level explanations of responsibilities, lifecycle, interaction with other components, why the abstraction exists, and when to use it instead of alternatives
Generated logic and generator pipelines MUST explain what is generated, why it is generated, the semantic assumptions the generator relies on, and any diagnostics or fallback behavior
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling, if any
Comments MUST NOT be trivial, redundant, or misleading. Prefer explaining why and when, not just what
Code should remain understandable without requiring external context. Prefer slightly more explanation over too little for framework code
Missing required documentation is a coding standards violation. Code that does not meet the documentation rules is considered incomplete
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, and con...

Files:

  • GFramework.Game.Tests/UI/UiRouterInteractionTests.cs
🧠 Learnings (3)
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
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/UI/UiRouterInteractionTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.csproj : Public API changes must be covered by unit or integration tests

Applied to files:

  • GFramework.Game.Tests/UI/UiRouterInteractionTests.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/UI/UiRouterInteractionTests.cs
🔇 Additional comments (2)
GFramework.Game.Tests/UI/UiRouterInteractionTests.cs (2)

46-105: 交互捕获与所有权排序回归覆盖到位

TryDispatchUiAction 与实例序号溢出场景的 GetUiActionOwner 回归点都覆盖到了关键语义,断言也聚焦行为结果,质量不错。

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


111-153: 恢复去重回归场景设计合理

ResumePopAsync 两条路径都验证了 OnShow 触发恢复时不重复 OnResume,很好地锁定了此前易回归的生命周期语义。

Based on learnings: Public API changes must be covered by unit or integration tests.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Apr 17, 2026
- 修改modal验证逻辑为验证topmost元素的输入动作掩码、指针输入阻塞和操作输入阻塞属性
- 为_instanceCounter字段查找添加更详细的错误消息以提高调试可读性
- 验证_instanceCounter字段类型是否仍为int类型以确保测试稳定性
- 移除对field变量的null合并操作符调用以简化代码逻辑

@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: 1

🧹 Nitpick comments (1)
GFramework.Game.Tests/UI/UiRouterInteractionTests.cs (1)

345-346: 建议使用 Allman 风格大括号包裹 if 分支

这里是非平凡行为(OnShow 内触发恢复),建议显式大括号以减少后续维护时误改风险,并与仓库统一风格保持一致。

♻️ 建议修改
-            if (ResumeFromShow)
-                OnResume();
+            if (ResumeFromShow)
+            {
+                OnResume();
+            }
As per coding guidelines: `Use Allman braces in C# code`.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game.Tests/UI/UiRouterInteractionTests.cs` around lines 345 - 346,
Wrap the single-line conditional in Allman-style braces to make the non-trivial
behavior explicit: replace the current if (ResumeFromShow) OnResume(); with an
if block that opens and closes on their own lines and calls OnResume() inside
(preserving the ResumeFromShow and OnResume identifiers and their current
semantics), ensuring consistent C# Allman brace style with the rest of the
repository.
🤖 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/UI/UiRouterInteractionTests.cs`:
- Around line 133-138: Add a <returns> XML doc to the public async test method
PopAsync_WhenPageResumesDuringShow_DoesNotCallResumeTwice in the
UiRouterInteractionTests class: document that the method returns a Task
representing the asynchronous test operation (e.g., "A Task that represents the
asynchronous test."). Place the <returns> tag inside the existing XML comment
block above the method to satisfy the repository's C# documentation guidelines.

---

Nitpick comments:
In `@GFramework.Game.Tests/UI/UiRouterInteractionTests.cs`:
- Around line 345-346: Wrap the single-line conditional in Allman-style braces
to make the non-trivial behavior explicit: replace the current if
(ResumeFromShow) OnResume(); with an if block that opens and closes on their own
lines and calls OnResume() inside (preserving the ResumeFromShow and OnResume
identifiers and their current semantics), ensuring consistent C# Allman brace
style with the rest of the repository.
🪄 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: 30061ff0-883f-41f9-aaac-3991c7372589

📥 Commits

Reviewing files that changed from the base of the PR and between eec8dc8 and ef8530a.

📒 Files selected for processing (1)
  • GFramework.Game.Tests/UI/UiRouterInteractionTests.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: Code Quality & Security
  • GitHub Check: Analyze (C#)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.cs: All public, protected, and internal types and members in C# MUST include XML documentation comments (///) using <summary>, <param>, <returns>, <exception>, and <remarks> tags where applicable
XML documentation comments must explain intent, contract, and usage constraints instead of restating syntax
If a member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly in XML documentation
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds and edge cases, and registration order or lifecycle sequencing
Avoid obvious comments such as // increment i
Core framework components such as Architecture, Module, System, Context, Registry, Service Module, and Lifecycle types MUST include high-level explanations of responsibilities, lifecycle, interaction with other components, why the abstraction exists, and when to use it instead of alternatives
Generated logic and generator pipelines MUST explain what is generated, why it is generated, the semantic assumptions the generator relies on, and any diagnostics or fallback behavior
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling, if any
Comments MUST NOT be trivial, redundant, or misleading. Prefer explaining why and when, not just what
Code should remain understandable without requiring external context. Prefer slightly more explanation over too little for framework code
Missing required documentation is a coding standards violation. Code that does not meet the documentation rules is considered incomplete
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, and con...

Files:

  • GFramework.Game.Tests/UI/UiRouterInteractionTests.cs
🧠 Learnings (2)
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.csproj : Public API changes must be covered by unit or integration tests

Applied to files:

  • GFramework.Game.Tests/UI/UiRouterInteractionTests.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/UI/UiRouterInteractionTests.cs
🔇 Additional comments (3)
GFramework.Game.Tests/UI/UiRouterInteractionTests.cs (3)

48-72: 语义动作“已捕获但未消费”分发回归覆盖到位

这个用例把返回值和调用次数一起校验,能稳定锁定 TryDispatchUiAction 的语义契约,回归价值很高。


74-107: 实例序号宽度溢出场景的排序回归测试设计合理

通过强制 _instanceCounter 到临界值来验证数值排序优先级,能够直接覆盖这次修复的关键风险路径。


147-156: Pop 流程下恢复幂等性的断言清晰且必要

先记录 resumeCountBeforePop 再断言 +1,能有效防止“OnShow 触发恢复 + 路由额外恢复”导致的重复调用回归。

Based on learnings: Public API changes must be covered by unit or integration tests.

Comment thread GFramework.Game.Tests/UI/UiRouterInteractionTests.cs
- 为PopAsync测试方法添加了缺失的返回值XML文档注释
- 在Godot页面行为恢复逻辑中添加了正确的代码块括号
- 确保当ResumeFromShow为true时OnResume方法调用的条件判断正确
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