fix(auto_coffee): 修复了自动做咖啡无法选到正确关卡的问题#241
Merged
Merged
Conversation
Contributor
Reviewer's Guide调整自动咖啡动作，在开始前可靠地滚动到最顶端并选择正确的关卡（1-1 / Coffee Master），并移除未使用的时间工具模块。 更新后的自动咖啡等级选择顺序图sequenceDiagram
participant TaskRunner
participant Context
participant Controller
TaskRunner->>Context: run
Context->>Controller: get_image
Context->>Context: run_recognition MakeCoffeeStart
alt MakeCoffeeStart hit
loop until MakeCoffeeTargetCoffeeMaster hit
Context->>Context: run_action MakeCoffeeScrollToTop
Context->>Controller: get_image
Context->>Context: run_recognition MakeCoffeeTargetCoffeeMaster
end
Context->>Controller: click_rect target_result.box
Context->>Controller: get_image
Context->>Context: run_recognition MakeCoffeeStart
alt MakeCoffeeStart hit
Context->>Controller: click_rect start_roi
end
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideAdjusts the auto coffee action to reliably scroll to the top and select the correct level (1-1 / Coffee Master) before starting, and removes an unused time utility module. Sequence diagram for updated auto coffee level selectionsequenceDiagram
participant TaskRunner
participant Context
participant Controller
TaskRunner->>Context: run
Context->>Controller: get_image
Context->>Context: run_recognition MakeCoffeeStart
alt MakeCoffeeStart hit
loop until MakeCoffeeTargetCoffeeMaster hit
Context->>Context: run_action MakeCoffeeScrollToTop
Context->>Controller: get_image
Context->>Context: run_recognition MakeCoffeeTargetCoffeeMaster
end
Context->>Controller: click_rect target_result.box
Context->>Controller: get_image
Context->>Context: run_recognition MakeCoffeeStart
alt MakeCoffeeStart hit
Context->>Controller: click_rect start_roi
end
end
File-Level Changes
Possibly linked issues
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}
Contributor
There was a problem hiding this comment.
Hey - 我发现了两个问题，并给出了一些整体建议：
- 反复调用
scroll_to_top_action的while True循环没有最大迭代次数或超时保护；建议添加重试次数限制或基于时间的退出条件，以避免在识别一直未命中时出现潜在的无限循环。 - 将
click_rect中的点击延迟从0.001增加到0.05会减慢所有使用该辅助方法的调用；如果更长的延迟只在这一特定流程中需要，建议将该延迟做成可配置参数，或使用单独的点击辅助方法，以免影响其他调用方。 - 新增的
scroll_to_top_action = "MakeCoffeeScrollToTop"假设在 pipeline/tasks JSON 中存在对应的 action；为避免默默无效的情况，更安全的做法是处理context.run_action(scroll_to_top_action)失败或缺失的场景。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `while True` loop that repeatedly calls `scroll_to_top_action` has no max-iteration or timeout guard; consider adding a retry limit or time-based escape to avoid potential infinite loops if recognition never hits.
- Increasing the click delay from `0.001` to `0.05` in `click_rect` will slow down every use of this helper; if the longer delay is only needed for this specific flow, consider making the delay configurable or using a separate click helper so other callers are not penalized.
- The newly introduced `scroll_to_top_action = "MakeCoffeeScrollToTop"` assumes a corresponding action exists in the pipeline/tasks JSON; it would be safer to handle the case where `context.run_action(scroll_to_top_action)` fails or is missing to prevent silent no-ops.
## Individual Comments
### Comment 1
<location path="agent/custom/action/auto_make_coffee.py" line_range="76-85" />
<code_context>
img = get_image(controller)
start_result = context.run_recognition("MakeCoffeeStart", img)
if start_result and start_result.hit:
+ while True:
+ if context.tasker.stopping:
+ return CustomAction.RunResult(success=False)
+ context.run_action(scroll_to_top_action)
+ time.sleep(1)
+ img = get_image(controller)
+ target_result = context.run_recognition(
+ "MakeCoffeeTargetCoffeeMaster", img
+ )
+ if target_result and target_result.hit:
+ break
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Add a timeout or max-iteration guard to avoid a potentially infinite search loop.
This `while True` loop will keep calling `scroll_to_top_action` and recognition until either the target is detected or `tasker.stopping` is set. If `MakeCoffeeTargetCoffeeMaster` is never matched (e.g., asset change or unexpected UI state), this loop can run indefinitely and block the pipeline. Please add a max-attempt or time-based limit and return a failure or safe fallback once that limit is reached.
</issue_to_address>
### Comment 2
<location path="agent/custom/action/auto_make_coffee.py" line_range="80" />
<code_context>
+ if context.tasker.stopping:
+ return CustomAction.RunResult(success=False)
+ context.run_action(scroll_to_top_action)
+ time.sleep(1)
+ img = get_image(controller)
+ target_result = context.run_recognition(
</code_context>
<issue_to_address>
**suggestion:** Consider using an existing cadence/constant instead of a hard-coded 1s sleep in the polling loop.
This fixed 1s sleep sets a separate cadence from `check_freq` and other timing settings in this action. To keep behavior consistent and easier to tune, either reuse `check_freq` (or another shared timing constant) or promote this to a named constant so it’s configurable without digging into the loop body.
Suggested implementation:
```python
time.sleep(check_freq)
```
This change assumes `check_freq` is already defined and in scope for this block (e.g., as a variable, attribute, or imported constant used by the rest of the action's timing logic). If it is not, you should:
1. Introduce a shared constant near the top of the file or in the existing configuration module, for example:
- `AUTO_MAKE_COFFEE_CHECK_FREQ = 1.0`
2. Use that constant consistently:
- Replace or define `check_freq = AUTO_MAKE_COFFEE_CHECK_FREQ` in the action setup.
- Ensure any other sleeps or polling logic in this action also use `check_freq` (or the shared constant) instead of hard-coded literals.
This keeps all the action’s timing behavior controlled from one configuration value instead of scattering literal delays through the loop body.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The
while Trueloop that repeatedly callsscroll_to_top_actionhas no max-iteration or timeout guard; consider adding a retry limit or time-based escape to avoid potential infinite loops if recognition never hits. - Increasing the click delay from
0.001to0.05inclick_rectwill slow down every use of this helper; if the longer delay is only needed for this specific flow, consider making the delay configurable or using a separate click helper so other callers are not penalized. - The newly introduced
scroll_to_top_action = "MakeCoffeeScrollToTop"assumes a corresponding action exists in the pipeline/tasks JSON; it would be safer to handle the case wherecontext.run_action(scroll_to_top_action)fails or is missing to prevent silent no-ops.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `while True` loop that repeatedly calls `scroll_to_top_action` has no max-iteration or timeout guard; consider adding a retry limit or time-based escape to avoid potential infinite loops if recognition never hits.
- Increasing the click delay from `0.001` to `0.05` in `click_rect` will slow down every use of this helper; if the longer delay is only needed for this specific flow, consider making the delay configurable or using a separate click helper so other callers are not penalized.
- The newly introduced `scroll_to_top_action = "MakeCoffeeScrollToTop"` assumes a corresponding action exists in the pipeline/tasks JSON; it would be safer to handle the case where `context.run_action(scroll_to_top_action)` fails or is missing to prevent silent no-ops.
## Individual Comments
### Comment 1
<location path="agent/custom/action/auto_make_coffee.py" line_range="76-85" />
<code_context>
img = get_image(controller)
start_result = context.run_recognition("MakeCoffeeStart", img)
if start_result and start_result.hit:
+ while True:
+ if context.tasker.stopping:
+ return CustomAction.RunResult(success=False)
+ context.run_action(scroll_to_top_action)
+ time.sleep(1)
+ img = get_image(controller)
+ target_result = context.run_recognition(
+ "MakeCoffeeTargetCoffeeMaster", img
+ )
+ if target_result and target_result.hit:
+ break
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Add a timeout or max-iteration guard to avoid a potentially infinite search loop.
This `while True` loop will keep calling `scroll_to_top_action` and recognition until either the target is detected or `tasker.stopping` is set. If `MakeCoffeeTargetCoffeeMaster` is never matched (e.g., asset change or unexpected UI state), this loop can run indefinitely and block the pipeline. Please add a max-attempt or time-based limit and return a failure or safe fallback once that limit is reached.
</issue_to_address>
### Comment 2
<location path="agent/custom/action/auto_make_coffee.py" line_range="80" />
<code_context>
+ if context.tasker.stopping:
+ return CustomAction.RunResult(success=False)
+ context.run_action(scroll_to_top_action)
+ time.sleep(1)
+ img = get_image(controller)
+ target_result = context.run_recognition(
</code_context>
<issue_to_address>
**suggestion:** Consider using an existing cadence/constant instead of a hard-coded 1s sleep in the polling loop.
This fixed 1s sleep sets a separate cadence from `check_freq` and other timing settings in this action. To keep behavior consistent and easier to tune, either reuse `check_freq` (or another shared timing constant) or promote this to a named constant so it’s configurable without digging into the loop body.
Suggested implementation:
```python
time.sleep(check_freq)
```
This change assumes `check_freq` is already defined and in scope for this block (e.g., as a variable, attribute, or imported constant used by the rest of the action's timing logic). If it is not, you should:
1. Introduce a shared constant near the top of the file or in the existing configuration module, for example:
- `AUTO_MAKE_COFFEE_CHECK_FREQ = 1.0`
2. Use that constant consistently:
- Replace or define `check_freq = AUTO_MAKE_COFFEE_CHECK_FREQ` in the action setup.
- Ensure any other sleeps or polling logic in this action also use `check_freq` (or the shared constant) instead of hard-coded literals.
This keeps all the action’s timing behavior controlled from one configuration value instead of scattering literal delays through the loop body.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+76
to
+85
| while True: | ||
| if context.tasker.stopping: | ||
| return CustomAction.RunResult(success=False) | ||
| context.run_action(scroll_to_top_action) | ||
| time.sleep(1) | ||
| img = get_image(controller) | ||
| target_result = context.run_recognition( | ||
| "MakeCoffeeTargetCoffeeMaster", img | ||
| ) | ||
| if target_result and target_result.hit: |
Contributor
There was a problem hiding this comment.
issue (bug_risk): 添加超时或最大迭代次数保护，以避免潜在的无限搜索循环。
这个 while True 循环会持续调用 scroll_to_top_action 并进行识别，直到检测到目标或设置了 tasker.stopping。如果始终无法匹配到 MakeCoffeeTargetCoffeeMaster（例如资源变更或出现了意外的 UI 状态），该循环可能会无限运行并阻塞整个 pipeline。请添加最大尝试次数或基于时间的限制，并在达到该限制时返回失败结果或安全的降级处理。
Original comment in English
issue (bug_risk): Add a timeout or max-iteration guard to avoid a potentially infinite search loop.
This while True loop will keep calling scroll_to_top_action and recognition until either the target is detected or tasker.stopping is set. If MakeCoffeeTargetCoffeeMaster is never matched (e.g., asset change or unexpected UI state), this loop can run indefinitely and block the pipeline. Please add a max-attempt or time-based limit and return a failure or safe fallback once that limit is reached.
| if context.tasker.stopping: | ||
| return CustomAction.RunResult(success=False) | ||
| context.run_action(scroll_to_top_action) | ||
| time.sleep(1) |
Contributor
There was a problem hiding this comment.
suggestion: 建议在轮询循环中使用现有的频率/常量，而不是硬编码 1 秒的 sleep。
这个固定的 1 秒 sleep 引入了与本 action 中 check_freq 以及其他计时设置不同的一套节奏。为保持行为一致且便于调优，建议要么复用 check_freq（或其他共享的时间常量），要么将该值提升为具名常量，这样无需深入循环体就可以进行配置。
建议的实现方式：
time.sleep(check_freq)此修改假设 check_freq 已经定义并在该代码块的作用域中可用（例如作为变量、属性或被其余计时逻辑使用的导入常量）。如果尚未定义，你可以：
- 在文件顶部或现有配置模块中引入一个共享常量，例如：
AUTO_MAKE_COFFEE_CHECK_FREQ = 1.0
- 在整个逻辑中统一使用该常量：
- 在 action 初始化阶段替换或定义
check_freq = AUTO_MAKE_COFFEE_CHECK_FREQ。 - 确保该 action 中其他
sleep或轮询逻辑也使用check_freq（或该共享常量），而不是分散的硬编码字面量。
- 在 action 初始化阶段替换或定义
这样可以通过单一配置值统一控制整个 action 的计时行为，而不是在循环体中分散多个延迟常量。
Original comment in English
suggestion: Consider using an existing cadence/constant instead of a hard-coded 1s sleep in the polling loop.
This fixed 1s sleep sets a separate cadence from check_freq and other timing settings in this action. To keep behavior consistent and easier to tune, either reuse check_freq (or another shared timing constant) or promote this to a named constant so it’s configurable without digging into the loop body.
Suggested implementation:
time.sleep(check_freq)This change assumes check_freq is already defined and in scope for this block (e.g., as a variable, attribute, or imported constant used by the rest of the action's timing logic). If it is not, you should:
- Introduce a shared constant near the top of the file or in the existing configuration module, for example:
AUTO_MAKE_COFFEE_CHECK_FREQ = 1.0
- Use that constant consistently:
- Replace or define
check_freq = AUTO_MAKE_COFFEE_CHECK_FREQin the action setup. - Ensure any other sleeps or polling logic in this action also use
check_freq(or the shared constant) instead of hard-coded literals.
- Replace or define
This keeps all the action’s timing behavior controlled from one configuration value instead of scattering literal delays through the loop body.
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.
现在会拖到最上面选择1-1关卡了
Summary by Sourcery
确保自动咖啡动作在开始运行前，能够可靠地滚动到顶部并选择正确的目标等级。
Bug Fixes:
Original summary in English
Summary by Sourcery
Ensure the auto coffee action reliably scrolls to the top and selects the correct target level before starting the run.
Bug Fixes: