fix(campaign): 低心情撤退后切换后续活动任务 - #767
Open
a2893005741 wants to merge 8 commits into
Open
Conversation
CI 检查报告
检查结果
导入冒烟测试
|
Reviewer's Guide为战役事件实现低心情撤退流程：在计算模式下拦截低心情强制出击弹窗，通过引导撤退流程依次导航准备界面和结果界面，将心情记录重置为 0，将当前事件任务延后到计算出的恢复时间，并可选地切换到下一个已配置的事件任务，同时用单元测试覆盖相关行为。 战役运行中低心情撤退流程的时序图sequenceDiagram
actor User
participant CampaignBase
participant CampaignRun
participant MapOperation as CampaignMapOperation
participant Emotion
participant Config
User->>CampaignBase: handle_combat_low_emotion()
CampaignBase->>CampaignBase: handle_popup_cancel(IGNORE_LOW_EMOTION)
alt Emotion_Mode == calculate and popup cancelled
CampaignBase->>CampaignBase: low_emotion_withdrawn = True
CampaignBase-->>CampaignRun: CampaignEnd(LowEmotionWithdraw)
else other emotion modes
CampaignBase->>CampaignBase: super.handle_combat_low_emotion()
end
CampaignRun->>CampaignRun: run()
CampaignRun->>CampaignBase: campaign.run()
CampaignBase-->>CampaignRun: CampaignEnd(LowEmotionWithdraw)
CampaignRun->>CampaignRun: catch CampaignEnd
CampaignRun->>CampaignRun: handle_low_emotion_withdrawal()
alt low_emotion_withdrawn
CampaignRun->>MapOperation: withdraw(skip_first_screenshot=False)
loop withdraw loop
MapOperation->>MapOperation: handle_withdraw_battle_preparation()
MapOperation->>MapOperation: handle_enter_map_preparation_cancel()
MapOperation->>MapOperation: handle_withdraw_result()
MapOperation->>MapOperation: handle_popup_confirm(WITHDRAW/COMBAT_STATUS)
MapOperation->>MapOperation: handle_in_stage()
end
CampaignRun->>Emotion: emotion.update()
CampaignRun->>Emotion: emotion.record()
CampaignRun->>Emotion: get_recovered_for_battle(campaign._map_battle)
Emotion-->>CampaignRun: recovered datetime
CampaignRun->>Config: task_delay(target=recovered)
CampaignRun->>CampaignRun: get_low_emotion_next_event_task(task)
alt next_event exists
CampaignRun->>Config: task_call(next_event, force_call=False)
Config-->>CampaignRun: success
CampaignRun->>Config: update()
end
CampaignRun->>Config: task_stop('[低心情] 已撤退并延后当前任务')
CampaignRun->>CampaignBase: low_emotion_withdrawn = False
else no low_emotion_withdrawn
CampaignRun-->>User: propagate CampaignEnd
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideImplements a low-emotion withdrawal flow for campaign events: intercepting the low-emotion forced sortie prompt in calculate mode, performing a guided withdrawal that navigates preparation and result UIs, resetting emotion records to 0, delaying the current event task to the computed recovery time, and optionally switching to the next configured event task, with unit tests covering the behavior. Sequence diagram for low emotion withdrawal flow in campaign runsequenceDiagram
actor User
participant CampaignBase
participant CampaignRun
participant MapOperation as CampaignMapOperation
participant Emotion
participant Config
User->>CampaignBase: handle_combat_low_emotion()
CampaignBase->>CampaignBase: handle_popup_cancel(IGNORE_LOW_EMOTION)
alt Emotion_Mode == calculate and popup cancelled
CampaignBase->>CampaignBase: low_emotion_withdrawn = True
CampaignBase-->>CampaignRun: CampaignEnd(LowEmotionWithdraw)
else other emotion modes
CampaignBase->>CampaignBase: super.handle_combat_low_emotion()
end
CampaignRun->>CampaignRun: run()
CampaignRun->>CampaignBase: campaign.run()
CampaignBase-->>CampaignRun: CampaignEnd(LowEmotionWithdraw)
CampaignRun->>CampaignRun: catch CampaignEnd
CampaignRun->>CampaignRun: handle_low_emotion_withdrawal()
alt low_emotion_withdrawn
CampaignRun->>MapOperation: withdraw(skip_first_screenshot=False)
loop withdraw loop
MapOperation->>MapOperation: handle_withdraw_battle_preparation()
MapOperation->>MapOperation: handle_enter_map_preparation_cancel()
MapOperation->>MapOperation: handle_withdraw_result()
MapOperation->>MapOperation: handle_popup_confirm(WITHDRAW/COMBAT_STATUS)
MapOperation->>MapOperation: handle_in_stage()
end
CampaignRun->>Emotion: emotion.update()
CampaignRun->>Emotion: emotion.record()
CampaignRun->>Emotion: get_recovered_for_battle(campaign._map_battle)
Emotion-->>CampaignRun: recovered datetime
CampaignRun->>Config: task_delay(target=recovered)
CampaignRun->>CampaignRun: get_low_emotion_next_event_task(task)
alt next_event exists
CampaignRun->>Config: task_call(next_event, force_call=False)
Config-->>CampaignRun: success
CampaignRun->>Config: update()
end
CampaignRun->>Config: task_stop('[低心情] 已撤退并延后当前任务')
CampaignRun->>CampaignBase: low_emotion_withdrawn = False
else no low_emotion_withdrawn
CampaignRun-->>User: propagate CampaignEnd
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
a2893005741
marked this pull request as ready for review
August 14, 2026 10:37
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我在这里给出了一些总体性的反馈：
- 在
CampaignBase.handle_combat_low_emotion中，建议在低情绪弹窗已处理并触发撤退时返回True，这样调用方就可以在进一步处理前进行短路，与原方法的语义保持一致。 - 在
handle_low_emotion_withdrawal中，将硬编码的任务名称映射{'Event': 'Event2', 'Event2': 'Event3'}改为配置驱动或集中管理，可以避免事件流逻辑到处散落，也更方便未来调整事件序列。 - 在
MapOperation.withdraw中，为战斗结果界面新增的连续hasattr检查，如果重构为一个单独的辅助函数，按顺序迭代已注册的处理器，而不是复制同样的模式四次，可能会更清晰、也更易维护。
面向 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- In `CampaignBase.handle_combat_low_emotion`, consider returning `True` when the low-emotion popup is handled and withdrawal is triggered so callers can short-circuit further handling consistently with the original method’s semantics.
- The hardcoded task name mapping `{'Event': 'Event2', 'Event2': 'Event3'}` in `handle_low_emotion_withdrawal` could be made configuration-driven or centralized to avoid scattering event flow logic and make future event sequence changes easier.
- The new chained `hasattr` checks in `MapOperation.withdraw` for battle result screens might be clearer and more maintainable if they are refactored into a single helper that iterates registered handlers in order instead of duplicating the same pattern four times.帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- In
CampaignBase.handle_combat_low_emotion, consider returningTruewhen the low-emotion popup is handled and withdrawal is triggered so callers can short-circuit further handling consistently with the original method’s semantics. - The hardcoded task name mapping
{'Event': 'Event2', 'Event2': 'Event3'}inhandle_low_emotion_withdrawalcould be made configuration-driven or centralized to avoid scattering event flow logic and make future event sequence changes easier. - The new chained
hasattrchecks inMapOperation.withdrawfor battle result screens might be clearer and more maintainable if they are refactored into a single helper that iterates registered handlers in order instead of duplicating the same pattern four times.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `CampaignBase.handle_combat_low_emotion`, consider returning `True` when the low-emotion popup is handled and withdrawal is triggered so callers can short-circuit further handling consistently with the original method’s semantics.
- The hardcoded task name mapping `{'Event': 'Event2', 'Event2': 'Event3'}` in `handle_low_emotion_withdrawal` could be made configuration-driven or centralized to avoid scattering event flow logic and make future event sequence changes easier.
- The new chained `hasattr` checks in `MapOperation.withdraw` for battle result screens might be clearer and more maintainable if they are refactored into a single helper that iterates registered handlers in order instead of duplicating the same pattern four times.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f7cff4938
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
a2893005741
marked this pull request as draft
August 14, 2026 10:46
a2893005741
marked this pull request as ready for review
August 14, 2026 10:55
There was a problem hiding this comment.
Hey - 我发现了两个问题，并留下了一些总体反馈：
- 建议在
CampaignBase中初始化low_emotion_withdrawn（例如在__init__中），这样它的生命周期会更清晰，也避免依赖临时创建属性。 CampaignRun中硬编码的映射LOW_EMOTION_NEXT_EVENT = {'Event': 'Event2', 'Event2': 'Event3'}会把逻辑和具体任务名强耦合；你可能希望改为由配置或任务元数据驱动，这样在事件名称变更时仍能保持一致性。- 在
handle_low_emotion_withdrawal中，非公开 emotion 路径假设emotion.fleets非空且可索引；为了避免在边缘配置下出现运行时错误，最好增加对空或异常 fleet 列表的防护。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider initializing `low_emotion_withdrawn` in `CampaignBase` (e.g., in `__init__`) so its lifecycle is clearer and you don't rely on ad-hoc attribute creation.
- The hard-coded `LOW_EMOTION_NEXT_EVENT = {'Event': 'Event2', 'Event2': 'Event3'}` mapping in `CampaignRun` couples the logic to specific task names; you might want to make this driven by configuration or task metadata so it stays consistent when event names change.
- In `handle_low_emotion_withdrawal`, the non-public emotion path assumes `emotion.fleets` is non-empty and indexed; it may be safer to guard against empty or malformed fleet lists to avoid runtime errors in edge configurations.
## Individual Comments
### Comment 1
<location path="tests/test_low_emotion_withdraw.py" line_range="46-55" />
<code_context>
+ campaign.handle_popup_cancel.assert_not_called()
+ campaign.handle_popup_confirm.assert_called_once_with('IGNORE_LOW_EMOTION')
+
+ def test_withdrawal_delays_current_task_to_emotion_recovery(self):
+ recovered = datetime(2026, 8, 14, 12, 0, 0)
+ fleet = Mock()
+ fleet.fleet = 1
+ fleet.current = 75
+ fleet.get_recovered.return_value = recovered
+ emotion = Mock(using_public=False)
+ emotion.fleets = [fleet, Mock()]
+ campaign = SimpleNamespace(
+ low_emotion_withdrawn=True,
+ emotion=emotion,
+ withdraw=Mock(side_effect=CampaignEnd('Withdraw')),
+ )
+ runner = CampaignRun.__new__(CampaignRun)
+ runner.__dict__['campaign'] = campaign
+ runner.__dict__['config'] = Mock()
+ runner.config.task.command = 'Event'
+ runner.config.FLEET_2 = 0
+
+ self.assertTrue(runner.handle_low_emotion_withdrawal())
</code_context>
<issue_to_address>
**suggestion (testing):** 为 `handle_low_emotion_withdrawal` 中的 `using_public`/`public_fleet` 分支增加测试覆盖。
当前测试只覆盖 `using_public=False`（单舰队和双舰队）。请再添加一个针对 `using_public=True` 分支的测试，其中设置 `emotion.public_fleet`。该测试应验证：只有 `public_fleet.current` 被设置为 0，并且 `public_fleet.get_recovered()` 的返回值被用作 `task_delay`。
</issue_to_address>
### Comment 2
<location path="tests/test_low_emotion_withdraw.py" line_range="146-155" />
<code_context>
+ def test_withdraw_processes_battle_result_before_waiting_for_stage(self):
</code_context>
<issue_to_address>
**suggestion (testing):** 扩展 `MapOperation` 的测试，以覆盖 `handle_withdraw_result` 中 `COMBAT_STATUS` 弹窗分支。
当所有特定处理函数都未消耗该界面时，`handle_withdraw_result` 会回退到 `handle_popup_confirm('COMBAT_STATUS')`。请添加一个测试：其中 `handle_battle_status`、`handle_exp_info`、`handle_get_ship` 和 `handle_get_items` 都返回 `False`，`handle_popup_confirm('COMBAT_STATUS')` 返回 `True`，并断言 `handle_withdraw_result()` 返回 `True`，以验证这个回退路径已被覆盖。
建议实现：
```python
def test_withdraw_processes_battle_result_before_waiting_for_stage(self):
operation = MapOperation.__new__(MapOperation)
operation.device = SimpleNamespace(screenshot=Mock())
operation.handle_battle_status = Mock(side_effect=[True, False])
operation.handle_exp_info = Mock(return_value=False)
operation.handle_get_ship = Mock(return_value=False)
operation.handle_get_items = Mock(return_value=False)
operation.handle_popup_confirm = Mock(return_value=False)
operation.appear_then_click = Mock(return_value=False)
operation.handle_auto_search_exit = Mock(return_value=False)
operation.appear = Mock(return_value=False)
def test_handle_withdraw_result_falls_back_to_combat_status_popup(self):
operation = MapOperation.__new__(MapOperation)
operation.device = SimpleNamespace(screenshot=Mock())
operation.handle_battle_status = Mock(return_value=False)
operation.handle_exp_info = Mock(return_value=False)
operation.handle_get_ship = Mock(return_value=False)
operation.handle_get_items = Mock(return_value=False)
operation.handle_popup_confirm = Mock(return_value=True)
result = operation.handle_withdraw_result()
self.assertTrue(result)
operation.handle_popup_confirm.assert_called_once_with('COMBAT_STATUS')
```
此更改假定 `MapOperation` 已在 `tests/test_low_emotion_withdraw.py` 中导入，并且 `handle_withdraw_result` 是 `MapOperation` 的实例方法。如果尚未导入 `MapOperation`，请在文件顶部添加 `from your_module_path import MapOperation`，并遵循现有的导入约定。
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- Consider initializing
low_emotion_withdrawninCampaignBase(e.g., in__init__) so its lifecycle is clearer and you don't rely on ad-hoc attribute creation. - The hard-coded
LOW_EMOTION_NEXT_EVENT = {'Event': 'Event2', 'Event2': 'Event3'}mapping inCampaignRuncouples the logic to specific task names; you might want to make this driven by configuration or task metadata so it stays consistent when event names change. - In
handle_low_emotion_withdrawal, the non-public emotion path assumesemotion.fleetsis non-empty and indexed; it may be safer to guard against empty or malformed fleet lists to avoid runtime errors in edge configurations.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider initializing `low_emotion_withdrawn` in `CampaignBase` (e.g., in `__init__`) so its lifecycle is clearer and you don't rely on ad-hoc attribute creation.
- The hard-coded `LOW_EMOTION_NEXT_EVENT = {'Event': 'Event2', 'Event2': 'Event3'}` mapping in `CampaignRun` couples the logic to specific task names; you might want to make this driven by configuration or task metadata so it stays consistent when event names change.
- In `handle_low_emotion_withdrawal`, the non-public emotion path assumes `emotion.fleets` is non-empty and indexed; it may be safer to guard against empty or malformed fleet lists to avoid runtime errors in edge configurations.
## Individual Comments
### Comment 1
<location path="tests/test_low_emotion_withdraw.py" line_range="46-55" />
<code_context>
+ campaign.handle_popup_cancel.assert_not_called()
+ campaign.handle_popup_confirm.assert_called_once_with('IGNORE_LOW_EMOTION')
+
+ def test_withdrawal_delays_current_task_to_emotion_recovery(self):
+ recovered = datetime(2026, 8, 14, 12, 0, 0)
+ fleet = Mock()
+ fleet.fleet = 1
+ fleet.current = 75
+ fleet.get_recovered.return_value = recovered
+ emotion = Mock(using_public=False)
+ emotion.fleets = [fleet, Mock()]
+ campaign = SimpleNamespace(
+ low_emotion_withdrawn=True,
+ emotion=emotion,
+ withdraw=Mock(side_effect=CampaignEnd('Withdraw')),
+ )
+ runner = CampaignRun.__new__(CampaignRun)
+ runner.__dict__['campaign'] = campaign
+ runner.__dict__['config'] = Mock()
+ runner.config.task.command = 'Event'
+ runner.config.FLEET_2 = 0
+
+ self.assertTrue(runner.handle_low_emotion_withdrawal())
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for `using_public`/`public_fleet` branch in `handle_low_emotion_withdrawal`.
Current tests only cover `using_public=False` (single and dual fleet). Please also add a test for the `using_public=True` branch where `emotion.public_fleet` is set. That test should ensure only `public_fleet.current` is set to 0 and that its `get_recovered()` result is used as the `task_delay` value.
</issue_to_address>
### Comment 2
<location path="tests/test_low_emotion_withdraw.py" line_range="146-155" />
<code_context>
+ def test_withdraw_processes_battle_result_before_waiting_for_stage(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Extend `MapOperation` tests to cover the `COMBAT_STATUS` popup branch in `handle_withdraw_result`.
`handle_withdraw_result` falls back to `handle_popup_confirm('COMBAT_STATUS')` if none of the specific handlers consume the screen. Please add a test where `handle_battle_status`, `handle_exp_info`, `handle_get_ship`, and `handle_get_items` all return `False`, `handle_popup_confirm('COMBAT_STATUS')` returns `True`, and assert that `handle_withdraw_result()` returns `True`, to verify this fallback path is covered.
Suggested implementation:
```python
def test_withdraw_processes_battle_result_before_waiting_for_stage(self):
operation = MapOperation.__new__(MapOperation)
operation.device = SimpleNamespace(screenshot=Mock())
operation.handle_battle_status = Mock(side_effect=[True, False])
operation.handle_exp_info = Mock(return_value=False)
operation.handle_get_ship = Mock(return_value=False)
operation.handle_get_items = Mock(return_value=False)
operation.handle_popup_confirm = Mock(return_value=False)
operation.appear_then_click = Mock(return_value=False)
operation.handle_auto_search_exit = Mock(return_value=False)
operation.appear = Mock(return_value=False)
def test_handle_withdraw_result_falls_back_to_combat_status_popup(self):
operation = MapOperation.__new__(MapOperation)
operation.device = SimpleNamespace(screenshot=Mock())
operation.handle_battle_status = Mock(return_value=False)
operation.handle_exp_info = Mock(return_value=False)
operation.handle_get_ship = Mock(return_value=False)
operation.handle_get_items = Mock(return_value=False)
operation.handle_popup_confirm = Mock(return_value=True)
result = operation.handle_withdraw_result()
self.assertTrue(result)
operation.handle_popup_confirm.assert_called_once_with('COMBAT_STATUS')
```
This change assumes `MapOperation` is already imported in `tests/test_low_emotion_withdraw.py` and that `handle_withdraw_result` is an instance method on `MapOperation`. If `MapOperation` is not imported, add `from your_module_path import MapOperation` at the top of the file, matching existing import conventions.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9c7900bd2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
a2893005741
marked this pull request as draft
August 14, 2026 10:59
a2893005741
marked this pull request as ready for review
August 14, 2026 11:18
There was a problem hiding this comment.
嗨，我在这里给出了一些总体反馈：
LOW_EMOTION_NEXT_EVENT映射中对特定任务名称（"Event"、"Event2"、"Event3"）进行了硬编码；建议考虑将这些配置移动到单独的配置文件或使用更通用的机制，这样未来的事件序列就不需要在这里修改代码。handle_withdraw_result通过getattr根据方法名进行分发；如果这些处理函数以后被重构或重命名，现有逻辑会在不报错的情况下悄悄失效，所以更安全的做法可能是直接引用已绑定的方法，或者在一个集中位置维护统一的结果处理函数列表。
面向 AI 代理的提示词
请根据本次代码评审中的评论进行修改：
## 总体评论
- `LOW_EMOTION_NEXT_EVENT` 映射中对特定任务名称（`"Event"`、`"Event2"`、`"Event3"`）进行了硬编码；建议考虑将这些配置移动到单独的配置文件或使用更通用的机制，这样未来的事件序列就不需要在这里修改代码。
- `handle_withdraw_result` 通过 `getattr` 根据方法名进行分发；如果这些处理函数以后被重构或重命名，现有逻辑会在不报错的情况下悄悄失效，所以更安全的做法可能是直接引用已绑定的方法，或者在一个集中位置维护统一的结果处理函数列表。帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've left some high level feedback:
- The LOW_EMOTION_NEXT_EVENT mapping hardcodes specific task names ('Event', 'Event2', 'Event3'); consider moving this to a configuration or a more generic mechanism so future event sequences don’t require code changes here.
- handle_withdraw_result dispatches on method names via getattr; if these handlers are refactored or renamed later this will silently stop working, so it may be safer to reference the bound methods directly or centralize the list of result handlers in one place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The LOW_EMOTION_NEXT_EVENT mapping hardcodes specific task names ('Event', 'Event2', 'Event3'); consider moving this to a configuration or a more generic mechanism so future event sequences don’t require code changes here.
- handle_withdraw_result dispatches on method names via getattr; if these handlers are refactored or renamed later this will silently stop working, so it may be safer to reference the bound methods directly or centralize the list of result handlers in one place.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd23d6cc5d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for fleet in fleets: | ||
| fleet.current = 0 | ||
| emotion.record() | ||
| recovered = max(fleet.get_recovered() for fleet in fleets) |
There was a problem hiding this comment.
当舰队从 0 心情恢复到控制阈值时，这里未向 get_recovered() 传入下一次出击的预期消耗，因此任务会在刚达到阈值时被唤醒；随后 CampaignBase.run() 中的 emotion.check_reduce(self._map_battle) 又会发现心情不足并再次延迟任务。所有使用心情控制的关卡都会因此产生一次无效的 UI 导航和调度循环，应按下次关卡的预计消耗计算本次延迟目标。
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e786aa5b6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
a2893005741
marked this pull request as draft
August 14, 2026 11:32
a2893005741
marked this pull request as ready for review
August 14, 2026 11:38
There was a problem hiding this comment.
嗨，我发现了 1 个问题，并留下了一些总体反馈：
- 在
handle_low_emotion_withdrawal中，你先调用了emotion.update()，然后再调用emotion.get_recovered_for_battle()，而该方法内部又会再次调用update()/record()/show()；建议只依赖一条路径（例如只通过辅助方法）来完成这类操作，以避免重复的状态更新和额外的 IO。 get_low_emotion_next_event_task每次调用都会重新解析并过滤调度器的优先级列表；如果这个方法被频繁调用，考虑在每个配置实例上缓存解析后的事件链，以减少重复解析并提升性能。
给 AI Agent 的 Prompt
请根据以下代码评审意见进行修改：
## 总体意见
- 在 `handle_low_emotion_withdrawal` 中，你先调用了 `emotion.update()`，然后再调用 `emotion.get_recovered_for_battle()`，而该方法内部又会再次调用 `update()`/`record()`/`show()`；建议只依赖一条路径（例如只通过辅助方法）来完成这类操作，以避免重复的状态更新和额外的 IO。
- `get_low_emotion_next_event_task` 每次调用都会重新解析并过滤调度器的优先级列表；如果这个方法被频繁调用，考虑在每个配置实例上缓存解析后的事件链，以减少重复解析并提升性能。
## 具体意见
### 评论 1
<location path="module/combat/emotion.py" line_range="339-346" />
<code_context>
- def _check_reduce(self, battle):
- """检查战斗带来的情绪减少。
+ def get_recovered_for_battle(self, battle):
+ """计算完成下一次战役所需的情绪恢复时间。
</code_context>
<issue_to_address>
**suggestion (performance):** 在情绪处理流程中重复调用 update/record/show 可能是多余的，并会影响性能。
`get_recovered_for_battle` 的两个分支都会调用 `self.update()`, `self.record()`, 和 `self.show()`，而 `handle_low_emotion_withdrawal` 在调用 `get_recovered_for_battle` 之前还会修改 `fleet.current` 并调用一次 `emotion.record()`。这会导致一次低情绪事件触发多轮记录/展示流程。建议将这些副作用调用集中到单一位置（要么只在 `get_recovered_for_battle` 内部，要么只在它的调用方），以减少冗余的文件系统/UI 操作，并让恢复逻辑更易理解和维护。
建议的实现：
```python
def get_recovered_for_battle(self, battle):
"""计算完成下一次战役所需的情绪恢复时间。
本方法负责在计算完成后统一进行情绪状态的更新与记录，
调用方无需再重复调用 update()/record()/show() 相关方法。
Returns:
datetime: 扣除下一次战役的预计情绪后达到控制阈值的时间。
"""
# 计算完成下一次战役的预计情绪值以及恢复到阈值所需时间。
# 注意：这里集中进行情绪状态更新与记录，避免重复的文件和 UI 操作。
recovered_at = self._calculate_recovered_for_battle(battle)
# 统一集中处理副作用：更新情绪、记录日志、刷新展示
self.update()
self.record()
self.show()
return recovered_at
```
```python
def reduce_shipwreck(self):
return 10
def _calculate_recovered_for_battle(self, battle):
"""内部纯计算方法，仅负责返回恢复时间，不产生副作用。
Args:
battle: 用于计算下一次战役带来的情绪消耗及恢复时间的战役对象。
Returns:
datetime: 扣除下一次战役的预计情绪后达到控制阈值的时间。
"""
# 这里应包含原来 get_recovered_for_battle 中的纯计算逻辑，
# 将其中的 self.update()/self.record()/self.show() 等副作用调用挪到 get_recovered_for_battle 中。
#
# 由于当前上下文缺失，这里给出一个占位实现，开发者需要用原有计算逻辑替换。
#
# 示例：假设有 self.current_emotion, self.threshold, self.recovery_rate 等字段，
# 可以根据 battle 的情绪消耗估算恢复到阈值的时间。
# 下面的代码仅作为结构占位，避免运行错误。
import datetime
# 占位逻辑：立即达到阈值，返回当前时间
return datetime.datetime.now()
```
1. 将原先 `get_recovered_for_battle` 中的纯计算逻辑（不包括 `self.update()`, `self.record()`, `self.show()` 调用）移动到新建的 `_calculate_recovered_for_battle` 方法中，并删除原方法内的这些副作用调用，只保留在 `get_recovered_for_battle` 中集中调用。
2. 在所有调用 `get_recovered_for_battle` 的地方（特别是 `handle_low_emotion_withdrawal`）删除额外的 `emotion.record()`, `emotion.update()`, `emotion.show()` 调用，确保每次低情绪事件只会触发一次记录/展示流程。
3. 如果 `handle_low_emotion_withdrawal` 在调用 `get_recovered_for_battle` 前后有独立的情绪记录（例如专门记录撤退事件），将这类业务性记录与通用情绪状态记录区分开：保留业务特定记录，但去除与状态更新重复的通用记录调用。
4. 检查是否存在其他分支或辅助方法在同一个低情绪处理流程中额外调用 `self.update()`, `self.record()`, `self.show()`；如有，统一改为只通过 `get_recovered_for_battle` 进行一次状态更新与展示，以避免多次文件系统和 UI 操作。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
handle_low_emotion_withdrawalyou callemotion.update()and thenemotion.get_recovered_for_battle(), which itself callsupdate()/record()/show()again; consider relying on one path (e.g. only the helper) to avoid redundant state updates and extra IO. get_low_emotion_next_event_taskrecomputes and filters the parsed scheduler priority list on every call; if this is used frequently, caching the parsed event chain per config instance would reduce repeated parsing and improve performance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `handle_low_emotion_withdrawal` you call `emotion.update()` and then `emotion.get_recovered_for_battle()`, which itself calls `update()`/`record()`/`show()` again; consider relying on one path (e.g. only the helper) to avoid redundant state updates and extra IO.
- `get_low_emotion_next_event_task` recomputes and filters the parsed scheduler priority list on every call; if this is used frequently, caching the parsed event chain per config instance would reduce repeated parsing and improve performance.
## Individual Comments
### Comment 1
<location path="module/combat/emotion.py" line_range="339-346" />
<code_context>
- def _check_reduce(self, battle):
- """检查战斗带来的情绪减少。
+ def get_recovered_for_battle(self, battle):
+ """计算完成下一次战役所需的情绪恢复时间。
</code_context>
<issue_to_address>
**suggestion (performance):** Repeated update/record/show calls in emotion handling may be redundant and impact performance.
Both branches of `get_recovered_for_battle` call `self.update()`, `self.record()`, and `self.show()`, while `handle_low_emotion_withdrawal` also mutates `fleet.current` and calls `emotion.record()` before invoking `get_recovered_for_battle`. This causes multiple record/show cycles for a single low-emotion event. Centralize these side-effect calls in a single place (either only inside `get_recovered_for_battle` or only in its caller) to reduce redundant filesystem/UI work and keep the recovery logic easier to follow.
Suggested implementation:
```python
def get_recovered_for_battle(self, battle):
"""计算完成下一次战役所需的情绪恢复时间。
本方法负责在计算完成后统一进行情绪状态的更新与记录，
调用方无需再重复调用 update()/record()/show() 相关方法。
Returns:
datetime: 扣除下一次战役的预计情绪后达到控制阈值的时间。
"""
# 计算完成下一次战役的预计情绪值以及恢复到阈值所需时间。
# 注意：这里集中进行情绪状态更新与记录，避免重复的文件和 UI 操作。
recovered_at = self._calculate_recovered_for_battle(battle)
# 统一集中处理副作用：更新情绪、记录日志、刷新展示
self.update()
self.record()
self.show()
return recovered_at
```
```python
def reduce_shipwreck(self):
return 10
def _calculate_recovered_for_battle(self, battle):
"""内部纯计算方法，仅负责返回恢复时间，不产生副作用。
Args:
battle: 用于计算下一次战役带来的情绪消耗及恢复时间的战役对象。
Returns:
datetime: 扣除下一次战役的预计情绪后达到控制阈值的时间。
"""
# 这里应包含原来 get_recovered_for_battle 中的纯计算逻辑，
# 将其中的 self.update()/self.record()/self.show() 等副作用调用挪到 get_recovered_for_battle 中。
#
# 由于当前上下文缺失，这里给出一个占位实现，开发者需要用原有计算逻辑替换。
#
# 示例：假设有 self.current_emotion, self.threshold, self.recovery_rate 等字段，
# 可以根据 battle 的情绪消耗估算恢复到阈值的时间。
# 下面的代码仅作为结构占位，避免运行错误。
import datetime
# 占位逻辑：立即达到阈值，返回当前时间
return datetime.datetime.now()
```
1. 将原先 `get_recovered_for_battle` 中的纯计算逻辑（不包括 `self.update()`, `self.record()`, `self.show()` 调用）移动到新建的 `_calculate_recovered_for_battle` 方法中，并删除原方法内的这些副作用调用，只保留在 `get_recovered_for_battle` 中集中调用。
2. 在所有调用 `get_recovered_for_battle` 的地方（特别是 `handle_low_emotion_withdrawal`）删除额外的 `emotion.record()`, `emotion.update()`, `emotion.show()` 调用，确保每次低情绪事件只会触发一次记录/展示流程。
3. 如果 `handle_low_emotion_withdrawal` 在调用 `get_recovered_for_battle` 前后有独立的情绪记录（例如专门记录撤退事件），将这类业务性记录与通用情绪状态记录区分开：保留业务特定记录，但去除与状态更新重复的通用记录调用。
4. 检查是否存在其他分支或辅助方法在同一个低情绪处理流程中额外调用 `self.update()`, `self.record()`, `self.show()`；如有，统一改为只通过 `get_recovered_for_battle` 进行一次状态更新与展示，以避免多次文件系统和 UI 操作。
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
2 participants
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
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
新特性：
错误修复：
功能改进：
测试：
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
新功能：
缺陷修复：
优化与增强：
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
新特性：
错误修复：
功能改进：
测试：
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
新功能：
错误修复：
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
新特性：
错误修复：
功能改进：
测试：
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
新功能：
缺陷修复：
优化与增强：
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
新特性：
错误修复：
功能改进：
测试：
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
通过在活动中推迟当前事件任务、调整情绪恢复的调度，并确保 UI 可靠返回到关卡界面，来处理低情绪撤退情形。
New Features:
Bug Fixes:
Enhancements:
Tests:
Original summary in English
Summary by Sourcery
Handle low-emotion withdrawal in campaigns by postponing the current event task, adjusting emotion recovery scheduling, and ensuring the UI reliably returns to the stage screen.
New Features:
Bug Fixes:
Enhancements:
Tests: