feat(state): 实现异步状态基类的同步方法限制#25
Merged
Merged
Conversation
- 为 AsyncContextAwareStateBase 添加同步方法异常抛出机制 - 禁止在异步状态中使用 OnEnter、OnExit 和 CanTransitionTo 同步方法 - 提供清晰的错误提示引导使用对应的异步方法 - 修复 StateMachine 中的状态转换逻辑确保线程安全 - 更新 IAsyncState 接口继承 IState 接口统一状态管理
Reviewer's GuideEnforces that async states cannot use sync lifecycle/transition methods by throwing clear NotSupportedException messages, aligns IAsyncState with IState for unified state management, and fixes StateMachine async transition logic to use a thread-safe snapshot of the current state during asynchronous checks and callbacks. Sequence diagram for thread-safe async state transition using snapshotsequenceDiagram
actor Caller
participant StateMachine
participant CurrentState as IState_currentSnapshot
participant TargetState as IState_target
Caller->>StateMachine: ChangeToAsync~T~()
activate StateMachine
StateMachine->>StateMachine: lock(_lock)
StateMachine->>StateMachine: resolve TargetState from States
StateMachine->>StateMachine: currentSnapshot = Current
StateMachine->>StateMachine: release lock(_lock)
alt currentSnapshot is not null
StateMachine->>CurrentState: CanTransitionToAsync(TargetState)
activate CurrentState
CurrentState-->>StateMachine: canTransition
deactivate CurrentState
alt canTransition is false
StateMachine->>CurrentState: OnTransitionRejectedAsync(TargetState)
StateMachine-->>Caller: false
else canTransition is true
StateMachine->>StateMachine: perform transition to TargetState
StateMachine-->>Caller: true
end
else currentSnapshot is null
StateMachine->>StateMachine: perform transition to TargetState
StateMachine-->>Caller: true
end
deactivate StateMachine
Class diagram for async state restrictions and interface alignmentclassDiagram
class IState {
<<interface>>
+void OnEnter(IState from)
+void OnExit(IState to)
+bool CanTransitionTo(IState target)
}
class IAsyncState {
<<interface>>
+Task OnEnterAsync(IState from)
+Task OnExitAsync(IState to)
+Task~bool~ CanTransitionToAsync(IState target)
}
class AsyncContextAwareStateBase {
+Task OnEnterAsync(IState from)
+Task OnExitAsync(IState to)
+Task~bool~ CanTransitionToAsync(IState target)
+void OnEnter(IState from)
+void OnExit(IState to)
+bool CanTransitionTo(IState target)
}
IAsyncState --|> IState
AsyncContextAwareStateBase ..|> IAsyncState
class StateMachine {
-Dictionary~Type,IState~ States
-IState Current
-object _lock
+bool ChangeTo~T~()
+Task~bool~ ChangeToAsync~T~()
+Task~bool~ CanTransitionToAsync(IState fromState, IState toState)
+Task OnTransitionRejectedAsync(IState fromState, IState toState)
}
StateMachine --> IState
StateMachine --> IAsyncState
AsyncContextAwareStateBase --> StateMachine
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In the
AsyncContextAwareStateBaseexception messages, consider usingnameof(OnEnterAsync),nameof(OnExitAsync), andnameof(CanTransitionToAsync)instead of hard-coded method names so refactors don’t silently desync the guidance text.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `AsyncContextAwareStateBase` exception messages, consider using `nameof(OnEnterAsync)`, `nameof(OnExitAsync)`, and `nameof(CanTransitionToAsync)` instead of hard-coded method names so refactors don’t silently desync the guidance text.
## Individual Comments
### Comment 1
<location> `GFramework.Core/state/AsyncContextAwareStateBase.cs:69-78` </location>
<code_context>
+ public virtual void OnEnter(IState? from)
</code_context>
<issue_to_address>
**suggestion:** Consider marking the sync members as obsolete or explicit interface implementations to discourage use rather than only throwing at runtime.
Since these methods only exist for interface compatibility and just throw, misuse is only detected at runtime. To surface problems earlier, consider adding `[Obsolete("This is an async state. Use the async methods instead.", error: true)]` or making them explicit `IState` implementations so they’re less discoverable on async states, while still satisfying the interface contract.
Suggested implementation:
```csharp
/// <summary>
/// 同步进入状态(不推荐使用)
/// 异步状态应该使用 OnEnterAsync 方法
/// </summary>
/// <exception cref="NotSupportedException">异步状态不支持同步操作</exception>
[Obsolete("This is an async state. Use the async methods instead.", error: true)]
public virtual void OnEnter(IState? from)
{
throw new NotSupportedException(
$"This is an async state ({GetType().Name}). Use OnEnterAsync instead of OnEnter.");
}
```
1. Apply the same `[Obsolete("This is an async state. Use the async methods instead.", error: true)]` attribute to the synchronous `OnExit` method and any other synchronous `IState` members that only exist to throw.
2. Ensure the file includes `using System;` at the top if it does not already, so the `Obsolete` attribute is available.
3. If you prefer to make these sync methods explicit interface implementations instead (or in addition), change their signatures from `public virtual void OnEnter(IState? from)` to an explicit form like `void IState.OnEnter(IState? from)` (and similarly for other members), and consider whether they still need to be `virtual` or overridden in derived async states.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- 更新类注释说明IAsyncState继承自IState接口 - 添加SetContext和GetContext方法用于架构上下文管理 - 实现Destroy方法用于状态销毁和资源释放 - 显式实现IState同步方法并标记为已弃用 - 使用Obsolete特性标注同步方法并提示使用异步版本 - 恢复IAsyncState异步方法的正常实现 - 添加上下文未设置时的操作异常处理
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
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
Restrict synchronous APIs on async states and improve async state machine safety and consistency.
New Features:
Bug Fixes: