feat(coroutine): 添加多种协程等待指令及对应单元测试#13
Merged
Merged
Conversation
- 实现 WaitForConditionChange 指令，支持等待条件状态变化 - 实现 WaitForEndOfFrame 指令，支持等待当前帧渲染完成 - 实现 WaitForFixedUpdate 指令，支持等待物理固定更新周期 - 实现 WaitForMultipleEvents 指令，支持等待多个事件中的任意一个触发 - 实现 WaitForNextFrame 指令，支持等待下一帧开始 - 实现 WaitForPredicate 指令，支持通用谓词等待功能 - 实现 WaitForSecondsRealtime 指令，支持基于真实时间的等待 - 实现 WaitForSecondsScaled 指令，支持受时间缩放影响的等待 - 实现 WaitUntilOrTimeout 指令，支持带超时的条件等待 - 为所有新指令添加完整的单元测试覆盖
Reviewer's GuideAdds a set of coroutine yield instructions for various waiting scenarios (condition changes, predicates, next frame/fixed update/end-of-frame, timed waits, multi-event waits) along with comprehensive NUnit unit tests for each instruction. Sequence diagram for WaitForMultipleEvents responding to event bussequenceDiagram
participant CoroutineScheduler
participant WaitForMultipleEvents
participant EventBus as IEventBus
participant Publisher
CoroutineScheduler->>WaitForMultipleEvents: new WaitForMultipleEvents(eventBus)
WaitForMultipleEvents->>EventBus: Register~TEvent1~(OnFirstEvent)
WaitForMultipleEvents->>EventBus: Register~TEvent2~(OnSecondEvent)
Publisher-->>EventBus: Publish TEvent1(eventData)
EventBus-->>WaitForMultipleEvents: OnFirstEvent(eventData)
activate WaitForMultipleEvents
WaitForMultipleEvents-->WaitForMultipleEvents: store FirstEventData
WaitForMultipleEvents-->WaitForMultipleEvents: set TriggeredBy = 1
WaitForMultipleEvents-->WaitForMultipleEvents: set _done = true
deactivate WaitForMultipleEvents
loop game_loop
CoroutineScheduler->>WaitForMultipleEvents: Update(deltaTime)
alt IsDone and handlers_not_unregistered
WaitForMultipleEvents->>EventBus: _unRegister1.UnRegister()
WaitForMultipleEvents->>EventBus: _unRegister2.UnRegister()
end
end
Class diagram for new coroutine time-based wait instructionsclassDiagram
class IYieldInstruction {
<<interface>>
+bool IsDone
+void Update(double deltaTime)
}
class WaitForSecondsRealtime {
-double _remaining
+WaitForSecondsRealtime(double seconds)
+void Update(double deltaTime)
+bool IsDone
}
class WaitForSecondsScaled {
-double _remaining
+WaitForSecondsScaled(double seconds)
+void Update(double deltaTime)
+bool IsDone
}
class WaitForEndOfFrame {
-bool _completed
+WaitForEndOfFrame()
+void Update(double deltaTime)
+bool IsDone
}
class WaitForFixedUpdate {
-bool _completed
+WaitForFixedUpdate()
+void Update(double deltaTime)
+bool IsDone
}
class WaitForNextFrame {
-bool _completed
+WaitForNextFrame()
+void Update(double deltaTime)
+bool IsDone
}
IYieldInstruction <|.. WaitForSecondsRealtime
IYieldInstruction <|.. WaitForSecondsScaled
IYieldInstruction <|.. WaitForEndOfFrame
IYieldInstruction <|.. WaitForFixedUpdate
IYieldInstruction <|.. WaitForNextFrame
Class diagram for new coroutine predicate and condition wait instructionsclassDiagram
class IYieldInstruction {
<<interface>>
+bool IsDone
+void Update(double deltaTime)
}
class WaitForPredicate {
-Func~bool~ _predicate
+WaitForPredicate(Func~bool~ predicate, bool waitForTrue)
+void Update(double deltaTime)
+bool IsDone
}
class WaitForConditionChange {
-Func~bool~ _conditionGetter
-bool _isCompleted
-bool _initialState
+WaitForConditionChange(Func~bool~ conditionGetter, bool waitForTransitionTo)
+void Update(double deltaTime)
+bool IsDone
}
class WaitUntilOrTimeout {
-Func~bool~ _predicate
-double _timeout
-double _elapsedTime
+WaitUntilOrTimeout(Func~bool~ predicate, double timeoutSeconds)
+bool ConditionMet
+bool IsTimedOut
+void Update(double deltaTime)
+bool IsDone
}
IYieldInstruction <|.. WaitForPredicate
IYieldInstruction <|.. WaitForConditionChange
IYieldInstruction <|.. WaitUntilOrTimeout
Class diagram for WaitForMultipleEvents coroutine instructionclassDiagram
class IYieldInstruction {
<<interface>>
+bool IsDone
+void Update(double deltaTime)
}
class IDisposable {
<<interface>>
+void Dispose()
}
class IUnRegister {
<<interface>>
+void UnRegister()
}
class IEventBus {
<<interface>>
+IUnRegister Register~TEvent~(Action~TEvent~ handler)
}
class WaitForMultipleEvents~TEvent1,TEvent2~ {
-bool _disposed
-bool _done
-IUnRegister _unRegister1
-IUnRegister _unRegister2
+WaitForMultipleEvents(IEventBus eventBus)
+TEvent1 FirstEventData
+TEvent2 SecondEventData
+int TriggeredBy
+bool IsDone
+void Update(double deltaTime)
+void Dispose()
-void OnFirstEvent(TEvent1 eventData)
-void OnSecondEvent(TEvent2 eventData)
}
IYieldInstruction <|.. WaitForMultipleEvents
IDisposable <|.. WaitForMultipleEvents
IEventBus --> IUnRegister : uses
WaitForMultipleEvents --> IEventBus : registers_handlers
WaitForMultipleEvents --> IUnRegister : holds
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In WaitForMultipleEvents, consider short‑circuiting the event handlers when
_doneor_disposedis already true so that a second event firing cannot overwriteTriggeredByand the captured event data intended to represent “the first event that arrived.” - For WaitForMultipleEvents, you might want to unregister from the event bus immediately inside the first event handler (or via a shared completion method) instead of deferring unregistration to the next
Updatecall, so that handlers are released even if the coroutine is never updated again after completion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In WaitForMultipleEvents, consider short‑circuiting the event handlers when `_done` or `_disposed` is already true so that a second event firing cannot overwrite `TriggeredBy` and the captured event data intended to represent “the first event that arrived.”
- For WaitForMultipleEvents, you might want to unregister from the event bus immediately inside the first event handler (or via a shared completion method) instead of deferring unregistration to the next `Update` call, so that handlers are released even if the coroutine is never updated again after completion.
## Individual Comments
### Comment 1
<location> `GFramework.Core/coroutine/instructions/WaitForMultipleEvents.cs:83-87` </location>
<code_context>
+ /// <summary>
+ /// 第一个事件触发处理
+ /// </summary>
+ private void OnFirstEvent(TEvent1 eventData)
+ {
+ FirstEventData = eventData;
+ TriggeredBy = 1;
+ _done = true;
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against multiple event invocations so that the first event wins consistently.
`OnFirstEvent` (and the corresponding second handler) always overwrite `TriggeredBy` and the event data, so if both events can fire, the later one wins non‑deterministically. Use `_done` as a guard in each handler (e.g., `if (_done) return;` at the top) so that once one event sets the state, subsequent invocations do nothing and the first event reliably wins.
</issue_to_address>
### Comment 2
<location> `GFramework.Core/coroutine/instructions/WaitForMultipleEvents.cs:70-57` </location>
<code_context>
+ /// 更新方法，检测条件变化
+ /// </summary>
+ /// <param name="deltaTime">时间增量</param>
+ public void Update(double deltaTime)
+ {
+ if (_isCompleted)
</code_context>
<issue_to_address>
**issue (bug_risk):** Current auto-unregister logic can leak subscriptions if the scheduler stops calling Update once IsDone is true.
The callbacks are only unregistered if `_isCompleted` is true *and* `Update` is called again:
```csharp
if (!_done || (_unRegister1 == null && _unRegister2 == null)) return;
```
If the scheduler stops invoking `Update` once `IsDone`/`_isCompleted` is true (as many coroutine schedulers do), those event subscriptions are never released unless `Dispose` is called manually.
Consider unregistering directly in `OnFirstEvent`/`OnSecondEvent` when `_done` is set, or factoring the unregister logic into a helper that’s called from both `Dispose` and the event handlers so cleanup doesn’t depend on a final `Update` call.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- 添加了完成状态检查，避免在已完成或释放后继续处理事件 - 立即注销事件监听器以防止内存泄漏 - 在事件触发后清理注册器引用 - 添加了完整的单元测试覆盖各种事件场景
- 为 Delay 指令添加完整的单元测试覆盖各种时间情况 - 为 WaitForCoroutine 指令添加单元测试验证协程等待功能 - 为 WaitForFrames 指令添加单元测试覆盖帧计数逻辑 - 为 WaitForTask<T> 指令添加单元测试包括异常处理场景 - 为 WaitOneFrame 指令添加单元测试验证单帧等待 - 为 WaitUntil 和 WaitWhile 指令添加单元测试覆盖谓词逻辑 - 将 WaitForMultipleEventsTests 中的异步方法标记为 async Task 类型 - 修改测试事件类的 Data 属性为可变的 set 访问器而不是只读 init - 优化 WaitForMultipleEventsTests 中的断言注释描述
This was referenced Feb 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add a set of coroutine waiting instructions covering frame, time, predicate, condition-change, timeout, and multi-event scenarios, along with corresponding unit test coverage.
New Features:
Tests: