Smart FDC 战术层增强:任务优先级、移动目标提前量、炮击顺序规划、装填恢复、唯一任务序号 - #31
Conversation
Seven adversarial auditors compared v1 against the legacy diff; all findings folded in. Three declared deviations from legacy behavior: UTF-8 BOM discipline (no mojibake), cancellations now recorded in RecentTasks, and the reflection-contract note on CoroutineLock.Acquire overload ambiguity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sequencing/log-order/threshold precision from the second adversarial pass, the ResolveElevation liveness exception, the FcsModule TaskContext carve-out, and the DisplayName()/ProgressText HUD contracts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Seven Opus implementers rebuilt the enhancements from REQUIREMENTS.md alone (no access to the legacy diff); UTF-8 BOM discipline applied repo-wide; build clean at 0 errors / 0 warnings in Logic. Adversarial verification (7 comparators + reflection-contract check against the legacy behavior) found 4 low regressions - all fixed - and 12 accepted deviations, archived in REQUIREMENTS.md Appendix D. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![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: 469c11a997
ℹ️ 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".
| // Take the gun slot first and leave the queue only after that succeeded: an external reader must | ||
| // never observe a frame in which the task is neither pending nor on a gun. | ||
| if (!RemovePendingTask(task)) | ||
| MelonLogger.Warning($"[FCS Dispatch] admitted T{task.targetId} was no longer present in pending queue"); | ||
| MelonLogger.Warning($"[FCS Dispatch] admitted #{task.serial} was no longer present in pending queue"); |
There was a problem hiding this comment.
When CancelPendingBySerial or SweepExpiredTasks removes a task while this planning coroutine is suspended in MaterializeCandidate, the stale assignment still reaches AddPlan; this failed RemovePendingTask is only logged after the gun slot has already been assigned. Consequently, a task that was reported as cancelled/expired and recorded in RecentTasks can become active and eventually fire. Confirm the task is still pending and not failed before creating/admitting its plan, and discard or restart stale materialization otherwise.
Useful? React with 👍 / 👎.
| if (!ReferenceEquals(_current, plan) || !IsActive(plan) || plan.Failed) | ||
| yield break; | ||
| if (_fcs.Turret.LastRotationSucceeded) | ||
| appliedAzimuth = plan.Task.angel; |
There was a problem hiding this comment.
When a same-bearing follower has already been armed during a manual fire wait and the current moving target then triggers this re-lay, the shared turret rotates but the follower remains armed. Pulling the shared trigger can therefore fire the follower at the current task's new bearing rather than its own target. After a successful re-lay, re-evaluate the follower against the effective bearing and clear its ready/armed state if it is now outside SameAzimuthToleranceDegrees.
Useful? React with 👍 / 👎.
| result => { | ||
| LastCardRequestResult = $"{request.CardId}: {result} @{FcsRuntimeClock.Now:F0}"; | ||
| MelonLogger.Msg($"[FCS] console card request {request.CardId} -> {result}"); |
There was a problem hiding this comment.
If two identical card requests fail on an immediate path such as card not found or card has no DraggableItem, both callbacks can run within the same rounded second and produce exactly the same LastCardRequestResult. The external poller detects completions only by string inequality, so the later request then has no observable result and its caller can wait indefinitely. Include a monotonically increasing request/completion identifier rather than relying only on F0 time.
Useful? React with 👍 / 👎.
| var requiredCharge = BallisticCalculator.MinimumCharge(urgent.distance); | ||
| FirePlan? victim = null; | ||
|
|
||
| // Left before right, replacing only on a strictly lower priority, so equal priorities pick the left gun. | ||
| foreach (var plan in new[] { _leftPlan, _rightPlan }) | ||
| { | ||
| if (plan == null | ||
| || plan.Task.priority >= urgent.priority | ||
| || ReferenceEquals(_current, plan) | ||
| || ReferenceEquals(_fireWaitOwner, plan) | ||
| || plan.ShotObserved | ||
| || plan.Shell != urgent.bulletType | ||
| || plan.Charge < requiredCharge) |
There was a problem hiding this comment.
EnqueueTask attempts urgent preemption before the planning round calls RefreshPendingSolutions, so urgent.distance can still describe the unled target or an aim computed from an old firing origin. If motion lead or origin re-survey moves the target across a 5 km charge boundary, this check can preempt a plan whose committed charge becomes insufficient as soon as planning refreshes the urgent task; the victim is disrupted while the urgent task remains pending instead of taking the freed gun. Evaluate the urgent task's late-bound solution before calculating requiredCharge, or otherwise use a charge requirement that accounts for that pending refresh.
Useful? React with 👍 / 👎.
…ult uniqueness, stale preemption solution - Discard materialized plans whose task was cancelled/expired mid-round (and skip them in the pending-reset sweep - a second resurrection hole found while fixing the first). - Disarm a same-bearing armed follower when a manual-wait re-lay moves the shared turret outside SameAzimuthToleranceDegrees; new TriggerConsole.Disarm(side) primitive, follower task unaffected. - LastCardRequestResult gains a monotonic completion counter so identical same-second results stay distinguishable to pollers. - TryPreemptForUrgent refreshes the urgent task's late-bound solution before computing the transferable-charge requirement. Spec sections (16, 8.5, 15.2/17.13, 5.5, appendix C) updated in step. Verified via an equivalent out-of-repo compile (0 errors) - the local game install was removed after archival, taking the Il2Cpp reference assemblies with it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Codex 的四条意见全部采纳,已在 e0f1fd2 修复:
REQUIREMENTS.md 相应章节已同步。 🤖 Generated with Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0f1fd2f03
ℹ️ 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".
| @@ -540,6 +888,79 @@ private bool CanFollowerArm(FirePlan current, FirePlan follower, out float azimu | |||
| return azimuthDelta <= SameAzimuthToleranceDegrees; | |||
There was a problem hiding this comment.
When either task's bearing changes during the new execution-stage tracking refresh, these immutable FirePlan.Azimuth values remain at their planning-time bearings. In particular, the current plan can complete its pre-fire azimuth correction before opening the wait, after which this check still approves a follower whose original bearing matched even though the turret now points elsewhere; pulling the shared trigger then fires that follower off-target. Use the current turret/applied bearing and the follower's refreshed task bearing for this authorization.
Useful? React with 👍 / 👎.
| task.motionVelLocalPerSec = task.hasMotion | ||
| ? Vector3.Lerp(task.motionVelLocalPerSec, sampled, VelocityLowPassFactor) | ||
| : sampled; | ||
| _entitySamples[task.trackEntityId] = (local, now); |
There was a problem hiding this comment.
When multiple pending tasks track the same entity ID, the first task processed updates this shared sample timestamp, so every following task in the same refresh sees dt < 0.5s and leaves its own motionVelLocalPerSec unchanged. With the stable queue iteration order, a later task can remain at zero velocity through every refresh and be planned or armed without lead while the first task tracks correctly. Cache the fitted velocity with the entity sample or sample each entity once and propagate the model to all matching tasks.
Useful? React with 👍 / 👎.
|
|
||
| if (preFireSolve.Ok) | ||
| { | ||
| appliedDistance = plan.Task.distance; |
There was a problem hiding this comment.
If the pre-fire elevation command times out or otherwise leaves LastElevationSucceeded false, this assignment still records the refreshed range as applied. For an unchanged tracked range or a static agent re-aim, the manual-wait condition subsequently sees no range delta and never retries, while the trigger path proceeds with the old elevation (including immediate AutoFire). Move the baseline update after successful SetElevation; the manual-relay path has the same premature update pattern.
Useful? React with 👍 / 👎.
| if (plan == null | ||
| || plan.Task.priority >= urgent.priority | ||
| || ReferenceEquals(_current, plan) | ||
| || ReferenceEquals(_fireWaitOwner, plan) |
There was a problem hiding this comment.
When a same-bearing follower has armed itself during the current manual fire wait, it is neither _current nor _fireWaitOwner, so an arriving higher-priority task can select it as the victim here. ReleaseGunSlot clears only the logical ready state and never disarms the physical safety; the commander's subsequent shared-trigger pull can therefore fire the preempted round after its FirePlan has been discarded, leaving an uncontrolled shot with no task to settle. Treat _armedFollower as non-preemptable, or synchronously disarm it before releasing the slot.
Useful? React with 👍 / 👎.
| _fcs.MapTable.ApplyMotionModel(urgent); | ||
| _fcs.MapTable.RefreshSolution(urgent); | ||
|
|
||
| var requiredCharge = BallisticCalculator.MinimumCharge(urgent.distance); |
There was a problem hiding this comment.
For a refreshed urgent range above the maximum C6 reach of 30 km, MinimumCharge saturates at 6 rather than indicating failure, so this method can evict a lower-priority C6 plan. The next planning round then rejects the urgent task at the explicit task.distance > charge * 5 eligibility check, meaning the victim was disrupted without creating any usable firing opportunity. Check the maximum supported range before selecting or tearing down a victim.
Useful? React with 👍 / 👎.
| continue; | ||
| } | ||
|
|
||
| var plan = _fcs.Planner.CreatePlan(assignment.Planning, item.Candidate, commitAt); |
There was a problem hiding this comment.
A pending task remains adjustable while MaterializeCandidate yields on the physical calculator, but this admission path verifies only cancellation/expiry. If the commander moves the aim across a charge boundary during that window, the candidate still carries the old charge and elevation while CreatePlan reads the task's new bearing and range; the now-insufficient charge can be physically committed, and both execution-stage elevation solves then fail without preventing the trigger path from firing the stale solution. Detect solution changes after materialization and rematch or rematerialize before admission.
Useful? React with 👍 / 👎.
| if (task.serial == 0) | ||
| task.serial = ++_serialCounter; |
There was a problem hiding this comment.
When a supported external caller pre-sets a positive serial, this branch preserves it but does not advance or otherwise reserve _serialCounter. For example, enqueueing external serial = 1 followed by an ordinary zero-serial task assigns #1 to both, after which cancellation and re-aim operations select whichever duplicate they encounter first and recent outcomes cannot distinguish the missions. Preserve the supplied value while also advancing the counter or rejecting an already-used serial.
Useful? React with 👍 / 👎.
|
感谢你为这个 PR 投入这么多时间，也感谢你认真处理了前面的 review。 我重新对照了 PR、AgentBridge 的实际使用方式，以及 Smart 现在的架构。先说结论： 这个 PR 我目前不会按现在的整体形态合并。 原因不只是当前实现里还有一些 race / lifecycle 问题，更主要的是几项核心能力放置的层级和 Smart 现在的职责边界不一致。也就是说，即使剩下的 review issue 全部修完，也不代表整个 PR 就会进入主线。 Smart 目前仍然坚持： “自动化操作，不自动化战术。” 在默认的人类操作模型里，玩家负责目标选择、任务提交顺序、弹种以及是否使用 Auto Fire / Max Charge；Smart 负责后续的机械执行、双炮协调、装填、瞄准和物理状态收敛。 1. Agent 动态调度的需求成立，但边界应该停在 Pending进一步看过 AgentBridge 后，我理解 priority、engagement-order optimization、moving prediction 等设计想解决的核心问题：Agent 和玩家的 workload 不一样。Agent 一次决策里可以生成一批任务，这一批任务更像一个待调度任务包；允许动态重排尚未执行的任务，确实可以提高双炮利用率，也能减少 Agent 在执行期间反复等待和重新提交任务的次数。 这个需求本身我认为是成立的。 但真正的分歧在于：这种动态性应该停在哪一层。 我更倾向于把边界明确放在 Pending 和 Execution 之间：
也就是：
这样 Agent 仍然可以一次规划大量任务，也可以在新情报出现时把紧急任务插到队首、删除失效任务、重排剩余任务；如果整个计划失效，也仍然可以用 F9 做粗粒度的全局重规划。 相应地，像 priority、任务包排序 / Held-Karp、移动目标运动模型和提前量预测这类主要服务于 Commander / Agent 的算法，本身没有必要进入 Smart 的执行核心。它们完全可以保留在 Bridge 一侧，通过 Smart 暴露的状态快照和 Pending Queue 接口完成计算，再把最终任务顺序或最终瞄点写回 Pending。 Smart 更适合提供的是：可靠的物理事实、稳定的 Task ID、受控的 Pending Queue API，以及必要的状态/事件，而不是承载某一种 Commander 的具体调度算法。 我更希望最终形成这样的边界：
如果 Agent 修改某个 Pending 任务时，它刚好已经被 admission，接口可以明确返回 移动目标也可以先沿用这个原则：由上层维护运动模型和预测，在任务仍处于 Pending 时持续更新，并在 Admission 前物化成最终射击任务。只有以后实测证明执行期持续跟踪确实不可替代，才值得单独讨论，而不是默认让执行中的 FirePlan 持续可变。 因此 priority / Smart 内部自动任务排序 / tactical preemption / 执行期持续改写 FirePlan 这一整套当前实现，我目前仍然不准备按现在的方式放进主线。 2. 射击原点模型这一点是比较明确的架构冲突。 Smart 现在把实际的 地图上的 这个 PR 的实现重新优先使用 3. 自动清膛属于新的真实射击行为装填失败恢复里“重新读取实际物理装药，并以物理状态为准”这个原则，我是认可的，它和 Smart 当前 Host / Logic 的关系一致。 但我不倾向于让 Executor 为了恢复一个任务，自动产生一发额外的缩程清膛射击。 如果当前物理装填状态已经无法完成原任务，我更希望 Smart 明确报告状态、让任务失败或者等待新的明确指令，而不是为了继续完成原任务，自行决定再打一发。 如果以后确实要支持清膛，我更倾向于把它定义成一个明确操作，由玩家或上层 Commander 主动决定，而不是作为 4. 可以吸收的方向这个 PR 不是没有可取的东西。相反，它把未来外部集成真正需要的基础设施暴露得比较清楚。 我目前认为比较值得单独吸收或重新实现的方向包括：
这些如果以后要做，我更希望每个问题单独做成很小的 reliability / infrastructure / integration 改动，不同时引入新的战术语义。 至于 priority、Smart 内部自动炮击排序、urgent preemption、以 所以你不用继续为了这个大 PR 把剩下的 review 问题全部修完。 还是感谢你做了这一整套实现。这个 PR 对我来说最大的价值之一，是把 Smart 的架构边界以及未来外部集成真正应该开放的位置暴露得很清楚：把未来做强，把现在做稳。 |
2 participants
为 Smart FDC 补齐一组"战术层"增强,全部在
IronNestFCS.Logic内、不改既有公开 API。这套功能已随一个 LLM 火控指挥官项目实战通关完整战役(演示:YouTube / Bilibili),随后按规格做了一次 clean-room 重写,即本 PR。功能
ArtilleryTask.priority(0–100,默认 50)。高优先级任务赢下配位、同批开火顺序、跨批次执行顺序;≥90 视为紧急——跳过凑对窗口,且可抢占一门"装载已匹配"的忙炮(被抢占任务无损退回队列)。仰角 = 距离km × 12 / 装药,60° 封顶,52 组台解验证残差 ±0.01°);重解仰角走解析式,零耗时不占弹道台,物理台只做兜底。#N:取代可回收的地图标记号,取消/改瞄/日志/HUD 全部以 serial 寻址;左右炮位在 HUD 以固定槽位 T9/T10 标示。validForSeconds>0 的任务超窗自动撤销(仅限仍在等待队列的)。AdjustTaskAim非阻塞改瞄、CancelPendingTask、RequestConsoleCard打孔卡代购队列(方位/距离/起始网格拨盘自动化)。质量与兼容
FcsLocalization双语通道。REQUIREMENTS.md,约 1800 行,经两轮共 135 条对抗性审计),再由未接触旧代码的实现者按规格重写,最后逐子系统对照实战版本做行为回归验证。构建 0 error / 0 warning(nullable 全开)。如希望拆分成更小的 PR(比如只要优先级/移动目标/恢复中的某几项),我可以按子系统拆——各功能在提交上是可分的。
🤖 Generated with Claude Code