fix: fix armor selection logic#46
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
高层总结本PR更新了火力控制配置参数,改进了目标装甲选择的优先级排序算法,将错误处理从 变更内容配置与组件初始化
火力控制算法与错误处理
预估代码审查工作量🎯 3 (中等复杂度) | ⏱️ ~20 分钟 可能相关的PR
建议标签
建议审查者
庆祝诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/module/fire_control/aim_point_chooser.cpp (1)
107-117: 💤 Low value命名建议:
preferred_incoming与值语义相反,易误读。该 lambda 实际上返回的是“是否为离开侧”的惩罚位:
delta与旋向同号(已转过中线、正在离开)时返回 1,否则返回 0;再借助std::tuple的<比较让 0(incoming)胜出。这与下面 line 116 的last_penalty(1 = 有惩罚)写法不一致,建议统一为“penalty”语义,使排序方向更直观。♻️ 命名/语义统一示例
- const auto priority_key = [&](size_t index) { - const auto preferred_incoming = [&] { - const auto delta = candidate_evals[index].delta_yaw; - if (angular_velocity > 0.0) return (delta > 0.0) ? 1 : 0; - if (angular_velocity < 0.0) return (delta < 0.0) ? 1 : 0; - return 0; - }(); + const auto priority_key = [&](size_t index) { + // 1 = 已越过中线/正在离开侧,排序中受惩罚;0 = incoming,优先 + const auto leaving_penalty = [&] { + const auto delta = candidate_evals[index].delta_yaw; + if (angular_velocity > 0.0) return (delta > 0.0) ? 1 : 0; + if (angular_velocity < 0.0) return (delta < 0.0) ? 1 : 0; + return 0; + }(); const auto abs_delta = std::abs(candidate_evals[index].delta_yaw); const auto id = armors[index].id; const auto is_last = last_chosen_armor_id.has_value() && (id == *last_chosen_armor_id); const auto last_penalty = is_last ? 0 : 1; - return std::tuple { preferred_incoming, abs_delta, last_penalty, id, index }; + return std::tuple { leaving_penalty, abs_delta, last_penalty, id, index }; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/module/fire_control/aim_point_chooser.cpp` around lines 107 - 117, The variable preferred_incoming has inverted/ambiguous semantics (it returns 1 when the target is leaving) which conflicts with last_penalty’s meaning and makes tuple-based ordering confusing; change it to an explicit penalty flag (e.g., leaving_penalty or side_penalty) that returns 1 when (candidate_evals[index].delta_yaw and angular_velocity have the same sign) and 0 otherwise, then use that new penalty variable in the returned std::tuple alongside abs_delta, last_penalty, id, index so the numeric meanings (1 = penalized, 0 = preferred) are consistent across preferred/last penalties and comparisons.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/module/fire_control/aim_point_chooser.cpp`:
- Around line 107-117: The variable preferred_incoming has inverted/ambiguous
semantics (it returns 1 when the target is leaving) which conflicts with
last_penalty’s meaning and makes tuple-based ordering confusing; change it to an
explicit penalty flag (e.g., leaving_penalty or side_penalty) that returns 1
when (candidate_evals[index].delta_yaw and angular_velocity have the same sign)
and 0 otherwise, then use that new penalty variable in the returned std::tuple
alongside abs_delta, last_penalty, id, index so the numeric meanings (1 =
penalized, 0 = preferred) are consistent across preferred/last penalties and
comparisons.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: af56cb1b-e312-4306-9465-fa2b04bcff44
📒 Files selected for processing (6)
config/config.yamlsrc/component.cppsrc/module/fire_control/aim_point_chooser.cppsrc/module/fire_control/solver/aim_point_sampling.cppsrc/module/fire_control/solver/aim_point_sampling.hpptest/aim_point_chooser.cpp
装甲选择逻辑修复
核心变更
装甲选择算法优化 (
aim_point_chooser.cpp)重新设计了装甲优先级选择机制,引入了基于预期来弹角度 (
preferred_incoming) 的新优先级键。该键根据偏航角增量和角速度动态计算,实现了对旋转方向的感知选择。选择对比从4元组扩展至5元组,提升了复杂场景下的装甲选择准确性。错误处理现代化 (
aim_point_sampling.cpp/hpp)将错误处理模式从
std::optional迁移至std::expected,更好地支持错误信息传播。sample_aim_point_at()返回类型从std::optional<Eigen::Vector3d>改为std::expected<Eigen::Vector3d, std::string>,当无法选中装甲时返回描述性错误消息。测试覆盖
新增三个单元测试验证装甲选择在不同旋转场景下的正确行为:
参数调优
火控配置参数微调以适应改进的选择逻辑:
代码清理
adapter/sentry.hpp改为adapter/adapter.hpp