Skip to content

Comments

Fix silos with delay before firing being able to fire twice#7010

Merged
BlackYps merged 15 commits intoFAForever:developfrom
lL1l1:Fix/nuke-subs
Feb 13, 2026
Merged

Fix silos with delay before firing being able to fire twice#7010
BlackYps merged 15 commits intoFAForever:developfrom
lL1l1:Fix/nuke-subs

Conversation

@lL1l1
Copy link
Contributor

@lL1l1 lL1l1 commented Feb 10, 2026

Description of the proposed changes

Fixes discord report of uef nuke sub and uef nuke launcher firing double missiles.

  1. After a lot of digging around in the engine and learning RE, I've learned how manual fire silo weapons and overcharge weapons work. I attempt to fix the root cause of the bug with the busy state not being set correctly in the FireReadyState, and I documented what I've learned.

  2. Add a failsafe ammo check before creating a projectile.

Testing done on the proposed changes

To make the UEF nuke sub fire double missiles, give a nuke order when you have ammo, cancel it by giving a move order, then give another nuke order within the 1 second charge delay of the firing sequence. This works because it will stack 2 nuke tasks on top of each other with the first being in the "finishing firing" state and the second in the "finished building ammo" state. When the unit busy state toggles, the finishing firing task expires and the finished building ammo task moves on to firing.

Additional Context

I annotated the lua files but will submit a separate pr.

Checklist

Summary by CodeRabbit

  • Bug Fixes

    • Fixed issue Fix silos with delay before firing being able to fire twice #7010: nuke and tactical missiles no longer consume a single ammo unit multiple times or allow ammo to go negative; firing is blocked when ammo is insufficient.
    • Improved firing readiness so units correctly respect manual/overcharge constraints before marking as busy.
  • Documentation

    • Clarified weapon/ammo behavior and firing notes in system documentation.

lL1l1 added 11 commits February 6, 2026 15:42
UEF sub had 1 second when it was not busy in which you could queue a new launch order that would take effect once it re-entered the idle state due to how the engine understands busy state in the manual weapon/counted projectile firing cycle.
This reverts commit b666da8.
This reverts commit 84a99b6.
The engine only allows these types of weapons to be used in launch orders.
It also doesn't allow manual fire weapons to ever fire outside of OC or lua functions.
@lL1l1 lL1l1 added this to the Development Iteration I of 2026 milestone Feb 10, 2026
@lL1l1 lL1l1 requested review from 4z0t and BlackYps February 10, 2026 10:54
@lL1l1 lL1l1 added type: bug area: sim Area that is affected by the Simulation of the Game labels Feb 10, 2026
@lL1l1 lL1l1 marked this pull request as ready for review February 10, 2026 10:58
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 10, 2026

📝 Walkthrough

Walkthrough

Prevents creating multiple nuke/tactical projectiles from a single ammo unit by gating projectile creation on silo/ammo availability and adjusts busy-state gating to require ManualFire and not OverchargeWeapon. Adds changelog and expanded inline documentation clarifying firing prerequisites and ammo checks.

Changes

Cohort / File(s) Summary
Changelog
changelog/snippets/fix.7010.md
Adds changelog entry describing fix for double-firing and negative ammo when using nuke/tactical ammo.
Blueprints / Docs
engine/Core/Blueprints/WeaponBlueprint.lua
Expanded inline comments: clarifies that projectiles must be built/stored before firing and that engine must verify silo ammo; clarifies nuke field semantics.
Unit-level docs
engine/Sim/UnitWeapon.lua
Added descriptive comment block describing tactical/nuke firing state machine and usage restriction to ManualFire && not OverchargeWeapon.
Firing logic
lua/sim/weapons/DefaultProjectileWeapon.lua
Conditionalized projectile creation to require counted-projectile absence or available nuke/tactical ammo; adjusted Busy flag gating from CountedProjectile/WeaponUnpacks to ManualFire && not OverchargeWeapon to prevent double-firing and negative ammo.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Unit
participant WeaponLogic as Weapon Logic
participant AmmoSilo as Ammo/Silo
participant ProjectileSystem as Projectile
Unit->>WeaponLogic: Trigger fire (ManualFire)
WeaponLogic->>AmmoSilo: Check ammo availability (nuke/tactical)
alt ammo available
WeaponLogic->>ProjectileSystem: Create projectile at muzzle
ProjectileSystem-->>WeaponLogic: Confirm creation (countedProjectile set)
WeaponLogic->>AmmoSilo: Consume ammo unit
WeaponLogic-->>Unit: Fire acknowledged
else no ammo
WeaponLogic-->>Unit: Prevent fire (no projectile created)
end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • BlackYps
  • 4z0t

Poem

🐰 I dug a logic burrow deep,

No double-boom to wake the sheep,
One ammo hop, one faithful blast,
Now missiles mind their stores at last. 🥕✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (17 files):

⚔️ changelog/snippets/balance.6939.md (content)
⚔️ changelog/snippets/balance.6966.md (content)
⚔️ changelog/snippets/balance.7000.md (content)
⚔️ changelog/snippets/balance.7016.md (content)
⚔️ changelog/snippets/balance.7019.md (content)
⚔️ engine/Core/Blueprints/WeaponBlueprint.lua (content)
⚔️ engine/Sim/UnitWeapon.lua (content)
⚔️ lua/sim/Profiler.lua (content)
⚔️ lua/sim/weapons/DefaultProjectileWeapon.lua (content)
⚔️ lua/ui/override/ArmiesTable.lua (content)
⚔️ lua/utilities.lua (content)
⚔️ units/UEL0001/UEL0001_unit.bp (content)
⚔️ units/UEL0401/UEL0401_lod0.scm (content)
⚔️ units/XSL0001/XSL0001_unit.bp (content)
⚔️ units/XSL0111/XSL0111_unit.bp (content)
⚔️ units/XSS0201/XSS0201_unit.bp (content)
⚔️ units/XSS0203/XSS0203_unit.bp (content)

These conflicts must be resolved before merging into develop.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main fix: preventing silos from firing twice when there is a delay before firing.
Description check ✅ Passed The description includes the proposed changes, testing methodology, and context. However, the changelog snippet requirement is marked as unchecked despite a changelog file being present in the changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch Fix/nuke-subs
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@lua/sim/weapons/DefaultProjectileWeapon.lua`:
- Around line 1060-1066: The conditional in DefaultProjectileWeapon.lua around
CreateProjectileAtMuzzle suffers from Lua operator-precedence: the tactical ammo
check is not gated by the weapon being non-nuke; fix by grouping the ammo checks
so nuke ammo is required for bp.NukeWeapon and tactical ammo only applies when
not a nuke, e.g. change the condition for calling CreateProjectileAtMuzzle to
explicitly combine (bp.NukeWeapon and self.unit:GetNukeSiloAmmoCount() > 0) or
(not bp.NukeWeapon and self.unit:GetTacticalSiloAmmoCount() > 0) while
preserving the existing not countedProjectile branch and still calling
CreateProjectileAtMuzzle(muzzle) when that combined condition is true.

@lL1l1 lL1l1 mentioned this pull request Feb 10, 2026
3 tasks
lL1l1 and others added 2 commits February 11, 2026 14:13
…le at 0 nuke ammo

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@BlackYps BlackYps merged commit 7ca1107 into FAForever:develop Feb 13, 2026
6 checks passed
relent0r pushed a commit to relent0r/fa that referenced this pull request Feb 23, 2026
…r#7010)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: sim Area that is affected by the Simulation of the Game type: bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants