fix: units resume attack-moving after killing target instead of going…#1379
Conversation
… idle When an attackmoving unit arrived at its destination and then found/killed a target, it would go idle with assault mode - but with targetX/targetY cleared. It could only scan for enemies within sight range from its current position. If the battle moved away, the unit was stranded. Now when a target dies and the unit has assaultDestination (from the original attack-move command), it resumes attackmoving toward that destination. This is the SC2 behavior: after killing a target, units continue toward the a-move point, engaging enemies along the way. This ensures units keep pushing into the fight rather than standing idle after each kill, which was a major cause of units appearing to "not fight". https://claude.ai/code/session_012Sc49F3UHGXxttLJpBPggB
There was a problem hiding this comment.
💡 Codex Review
voidstrike/src/engine/systems/CombatSystem.ts
Lines 806 to 810 in 8a44ca2
This branch still sends assault-mode units to idle when their target is dead but the entity remains in the world. In this codebase, units are marked state = 'dead' without being destroyed, so the usual post-kill flow hits the targetHealth.isDead() path rather than the !targetEntity path where the new assault-destination resume logic was added. That means the fix won’t apply for most kills: assault-move units will still idle after killing a target whose entity hasn’t been destroyed, which contradicts the intended SC2-style behavior.
ℹ️ 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".
The previous commit (8a44ca2) added logic to resume attack-moving toward assaultDestination after killing a target, but placed it only in the !targetEntity || targetEntity.isDestroyed() branch. In this codebase, killed units are marked state='dead' WITHOUT being destroyed, so most kills flow through the targetHealth.isDead() path instead. This meant the fix never applied for most kills, leaving units stranded idle. Apply the same assaultDestination resume logic to: - The targetHealth.isDead() path (primary kill flow) - The !canAttackThisTarget path (target type mismatch) Also adds regression tests for target-death state transition logic. https://claude.ai/code/session_012Sc49F3UHGXxttLJpBPggB
… idle
When an attackmoving unit arrived at its destination and then found/killed a target, it would go idle with assault mode - but with targetX/targetY cleared. It could only scan for enemies within sight range from its current position. If the battle moved away, the unit was stranded.
Now when a target dies and the unit has assaultDestination (from the original attack-move command), it resumes attackmoving toward that destination. This is the SC2 behavior: after killing a target, units continue toward the a-move point, engaging enemies along the way.
This ensures units keep pushing into the fight rather than standing idle after each kill, which was a major cause of units appearing to "not fight".
https://claude.ai/code/session_012Sc49F3UHGXxttLJpBPggB