Skip to content

fix(aasystem): refresh unpack menu and fix unit overlap after assembly - #101

Merged
FullGas1 merged 2 commits into
developfrom
fix/aasystem-unpack-bugs
Aug 4, 2026
Merged

fix(aasystem): refresh unpack menu and fix unit overlap after assembly#101
FullGas1 merged 2 commits into
developfrom
fix/aasystem-unpack-bugs

Conversation

@FullGas1

@FullGas1 FullGas1 commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Stale F10 unpack menu after AA system assembly: _assemble() called CTLDCrate:destroy() directly, bypassing CTLDCrateManager. OnCrateCleared was never published, so _refreshNearbyPlayers never fired and the unpack submenu stayed visible. Fixed by routing through CTLDCrateManager:destroyCrate().
  • Overlapping units on spawn (S-300 TEL D + Big Bird SR at 240 deg): The arc-step formula used arcRad/partAmount (full circle) instead of arcSegment/partAmount (part's reserved segment). Fixed in _buildSpawnArrays.

Fixes both bugs reported during field testing of the S-300 (SAM long range). Covers all AA systems -- both code paths are generic.

Test plan

  • New busted L1 test: S-300 _buildSpawnArrays -- all positions pairwise-distinct (>=1 m apart)
  • New busted L1 test: S-300 _assemble -- OnCrateCleared published 5x (one per consumed crate)
  • Existing deploy_managers_spec F-021 updated to register crates in CTLDCrateManager (required by new destroy path)
  • 1066 unit + 194 functional tests green
  • CTLD.lua rebuilt

Generated with Claude Code

Summary by Sourcery

Fix AA system unpack behaviour by refreshing the F10 unpack menu after assembly and correcting spawn geometry to prevent overlapping units.

Bug Fixes:

  • Ensure consumed AA system crates are destroyed via CTLDCrateManager so OnCrateCleared is published and the F10 unpack submenu refreshes for nearby pilots.
  • Correct AA system spawn angle computation so multi-unit parts are distributed within their reserved arc segment and no units overlap on spawn.

Enhancements:

  • Register test crates with CTLDCrateManager in assembly tests to cover the new destruction path.

Documentation:

  • Document the AA system unpack bugfixes and their rationale in the changelog and backlog PRD/tickets.

Tests:

  • Add unit tests verifying S-300 spawn positions are pairwise distinct and that _assemble publishes OnCrateCleared once per consumed crate.
  • Update existing deploy manager assembly tests to use CTLDCrateManager for crate registration.

Two bugs in CTLDCrateAssemblyManager._assemble:

1. Menu never refreshed: crates were destroyed via c:destroy() directly,
   bypassing CTLDCrateManager, so OnCrateCleared was never published and
   _refreshNearbyPlayers never fired. Now uses cm:destroyCrate(crateName)
   which publishes the event and triggers the existing refresh contract.

2. S-300 TEL D (NoCrate, amount=2) spawned on top of Big Bird SR: the
   arc-step formula used arcRad/partAmount (full circle) instead of
   arcSegment/partAmount (part's reserved segment). Fixed.

Fixes: FIX-AASYSTEM-UNPACK-BUGS tickets 01 + 02.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@FullGas1
FullGas1 requested a review from davidp57 as a code owner August 4, 2026 12:45
@sourcery-ai

sourcery-ai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Reviewer's Guide

Routes AA system crate destruction through CTLDCrateManager to ensure OnCrateCleared is published (refreshing the F10 unpack menu) and corrects the spawn geometry in _buildSpawnArrays so multi-unit parts are distributed within their arc segment; adds focused unit tests, backlog documentation, and changelog entries for the FIX-AASYSTEM-UNPACK-BUGS work.

Sequence diagram for AA system unpack menu refresh via CTLDCrateManager

sequenceDiagram
    actor Pilot
    participant CTLDCrateAssemblyManager
    participant CTLDCrateManager
    participant F10Menu

    Pilot->>CTLDCrateAssemblyManager:_assemble(heli, crate, allCrates, template, range)
    CTLDCrateAssemblyManager->>CTLDCrateAssemblyManager:_buildSpawnArrays(template, systemParts, origin, heli)
    loop destroy consumed crates
        CTLDCrateAssemblyManager->>CTLDCrateManager:getInstance()
        CTLDCrateAssemblyManager->>CTLDCrateManager:destroyCrate(crateName)
        CTLDCrateManager-->>CTLDCrateManager:OnCrateCleared
        CTLDCrateManager->>CTLDCrateManager:_refreshNearbyPlayers(position)
        CTLDCrateManager->>F10Menu:refreshUnpackSection()
    end
Loading

File-Level Changes

Change Details Files
AA system assembly now destroys crates via CTLDCrateManager so OnCrateCleared is published and the F10 unpack menu refreshes correctly.
  • _assemble now acquires CTLDCrateManager instance and uses destroyCrate(crateName) instead of calling crate:destroy() directly for each consumed part crate.
  • Assembly tests register AA crates inside CTLDCrateManager to match the new destroy path and reset crate manager state in setup/teardown.
  • New unit test for _assemble stubs EventDispatcher:publish and asserts OnCrateCleared is fired once per consumed S-300 crate.
CTLD.lua
src/CTLD_aasystem.lua
tests/ci/unit/deploy_managers_spec.lua
tests/ci/unit/aasystem_spec.lua
AA system spawn geometry is fixed so multi-unit template parts (e.g. S-300 TEL D) no longer overlap other units when spawned.
  • _buildSpawnArrays now computes an arcSegment per part (2π/partCount) and uses step = arcSegment/partAmount, with angles anchored at arcBase, instead of spreading units over the full circle.
  • New S-300-specific geometry test builds systemParts from the S-300 template, calls _buildSpawnArrays, and asserts all spawn positions are pairwise-distinct with a 1 m tolerance.
CTLD.lua
src/CTLD_aasystem.lua
tests/ci/unit/aasystem_spec.lua
Documentation and backlog entries capture the FIX-AASYSTEM-UNPACK-BUGS work and its scope.
  • CHANGELOG adds a Fixed section describing the AA system unpack menu and spawn overlap fixes.
  • Backlog README gains a FIX-AASYSTEM-UNPACK-BUGS entry summarizing the issue.
  • New PRD and ticket markdown files describe problem statement, solution, and acceptance criteria for spawn geometry and menu refresh bugs.
CHANGELOG.md
.backlog/README.md
.backlog/FIX-AASYSTEM-UNPACK-BUGS/PRD.md
.backlog/FIX-AASYSTEM-UNPACK-BUGS/tickets/01-spawn-geometry.md
.backlog/FIX-AASYSTEM-UNPACK-BUGS/tickets/02-menu-refresh.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In the CTLDCrateAssemblyManager _assemble publishes OnCrateCleared per consumed crate spec, you restore EventDispatcher.getInstance().publish using a fresh getInstance() call; consider storing the original EventDispatcher.getInstance() object in before_each and restoring publish on that same instance in after_each to avoid subtle bugs if the singleton instantiation logic ever changes.
  • The S-300 template lookup logic is duplicated between buildS300Parts() and the _assemble publishes OnCrateCleared test; extracting a shared helper for fetching the S-300 template would reduce repetition and keep the tests easier to maintain if template names or layout change.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the `CTLDCrateAssemblyManager _assemble publishes OnCrateCleared per consumed crate` spec, you restore `EventDispatcher.getInstance().publish` using a fresh `getInstance()` call; consider storing the original `EventDispatcher.getInstance()` object in `before_each` and restoring `publish` on that same instance in `after_each` to avoid subtle bugs if the singleton instantiation logic ever changes.
- The S-300 template lookup logic is duplicated between `buildS300Parts()` and the `_assemble publishes OnCrateCleared` test; extracting a shared helper for fetching the S-300 template would reduce repetition and keep the tests easier to maintain if template names or layout change.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

- Extract shared s300TemplateAndParts() helper used by both the geometry
  and menu-refresh describe blocks, eliminating duplicated S-300 template
  lookup logic.
- Store EventDispatcher instance in before_each and restore publish on
  that same object in after_each, avoiding any risk of restoring onto a
  different singleton instance.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@FullGas1
FullGas1 merged commit e21b922 into develop Aug 4, 2026
7 checks passed
FullGas1 added a commit that referenced this pull request Aug 4, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@davidp57 davidp57 mentioned this pull request Aug 14, 2026
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant