Annotate DefaultProjectileWeapon

[lL1l1]

Description of the proposed changes

• Annotates the states of DefaultProjectileWeapon. They had no annotations at all previously.
• Fixes most warnings. The only one left is param-type-mismatch for GetFocusArmy on L1097.

Testing done on the proposed changes

• Shows no warnings.
• Wrote debug setup code in Fix silos with delay before firing being able to fire twice #7010 in reverted commits, so that touched a lot of places.

Checklist

• Changes are annotated, including comments where useful
• Changes are documented in a changelog snippet according to the guidelines.
• Request 2-3 reviewers from the list of reviewers and their areas of knowledge.

Summary by CodeRabbit

• New Features

  • Weapons now support charge animations with configurable playback speed
  • Weapons now support reload animations with configurable playback speed
  • Expanded weapon targeting capabilities to include props alongside units and entities

• Deprecations

  • Marked weapon unpacking motion configuration as deprecated; use updated alternative

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes enhance the weapon system by introducing animation configuration fields to WeaponBlueprint (charge and reload animations with rates), expanding Unit's GetTargetEntity return type to include Prop, refactoring projectile weapon distance calculations, adding public field declarations and state class annotations to DefaultProjectileWeapon, and exposing the __base reference in the State class.

Changes

Cohort / File(s)	Summary
Changelog changelog/snippets/other.7012.md	Added changelog entry documenting annotation fixes in DefaultProjectileWeapon.lua.
Weapon Blueprint Extensions engine/Core/Blueprints/WeaponBlueprint.lua	Added animation support fields (AnimationCharge, AnimationChargeRate, AnimationReloadRate) and deprecated WeaponUnpackLockMotion in favor of WeaponUnpackLocksMotion.
Unit Type System engine/Sim/Unit.lua	Expanded GetTargetEntity() return type to include Prop alongside Entity and Unit.
Default Projectile Weapon Refactoring lua/sim/weapons/DefaultProjectileWeapon.lua	Replaced VDist2 with GetDistanceBetweenTwoPoints2 for distance calculations, added 8 public fields (WeaponCanFire, UnpackAnimator, RackRecoilReturnSpeed, NumMuzzles, NumRackBones, StateName, WeaponWantEnabled, WeaponAimWantEnabled), added comprehensive state class annotations, updated targeting logic to handle Unit vs Prop differently, and added defensive nil checks around audio blueprint access.
State Class System lua/system/class.lua	Exposed __base table reference as a public field in State class.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~35 minutes

Possibly related PRs

• Fix silos with delay before firing being able to fire twice #7010: Both PRs modify DefaultProjectileWeapon.lua's state handling logic and conditional behavior in RackSalvoFiringState and related states.

Suggested reviewers

• 4z0t
• BlackYps

Poem

🐰 Hoppy hops through weapon states,
Distances leap with better rates,
Props and Units now align,
Animations charge so fine,
The weapon system now takes flight! ✨

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Annotate DefaultProjectileWeapon' is partially related to the changeset; it covers the main annotation work but omits significant changes like VDist2 refactoring, API additions to WeaponBlueprint, and Unit.GetTargetEntity return type expansion.
Description check	✅ Passed	The description covers the annotation work and testing, but the changelog snippet section checkbox is unchecked even though changelog/snippets/other.7012.md exists in the changeset, indicating incomplete tracking of completed items.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into develop

_{✏️ 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

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

lua/sim/weapons/DefaultProjectileWeapon.lua (1)

317-317: ⚠️ Potential issue | 🟠 Major

Pre-existing bug: Z-coordinate uses X values in GetSurfaceHeight call.

Not introduced by this PR, but line 317 passes projPosX + time * projVelX for both the X and Z arguments to GetSurfaceHeight. The second argument should be projPosZ + time * projVelZ.

🐛 Proposed fix

-                    halfHeight = 0.5 * (projPosY - GetSurfaceHeight(projPosX + time * projVelX, projPosX + time * projVelX))
+                    halfHeight = 0.5 * (projPosY - GetSurfaceHeight(projPosX + time * projVelX, projPosZ + time * projVelZ))

🧹 Nitpick comments (2)

lua/sim/weapons/DefaultProjectileWeapon.lua (2)

236-259: Redundant velocity fetch for single-bomb path.

Lines 236–239 already set targetVelX/Y/Z when target.IsUnit is true. Lines 250–253 re-fetch the exact same velocity within the same function call, with no intervening yield or tick. The second fetch is a no-op.

♻️ Remove duplicate velocity fetch

                 if self.Blueprint.MuzzleSalvoSize <= 1 then
                     -- do the calculation but skip any cache or salvo logic
                     if not targetPos then
                         return 4.9
                     end
-                    if target and target.IsUnit then
-                        ---@cast target Unit
-                        targetVelX, targetVelY, targetVelZ = UnitGetVelocity(target)
-                    end
                     local targetPosX, targetPosZ = targetPos[1], targetPos[3]

---

586-586: Inconsistent nil guard: PlayFxWeaponPackSequence lacks the same Audio nil check.

Line 586 accesses self.unit.Blueprint.Audio.Close directly, while PlayFxWeaponUnpackSequence (lines 547–556) now guards against a nil Audio table. If the blueprint can lack an Audio field, this path would also need a guard. Pre-existing, but worth aligning.

🛡️ Proposed guard

     PlayFxWeaponPackSequence = function(self)
         local bp = self.Blueprint
-        local close = self.unit.Blueprint.Audio.Close
-        if close then
-            self:PlaySound(close)
+        local unitBPAudio = self.unit.Blueprint.Audio
+        if unitBPAudio then
+            local close = unitBPAudio.Close
+            if close then
+                self:PlaySound(close)
+            end
         end