feat(farp): register a troop pickup zone when a FARP scene completes - #137
Merged
Conversation
A built FARP has no troop-pickup capability at all, unlike a FOB (fixed by FIX-FOB-TROOP-PICKUP, PR #136). Grilled with the user 2026-08-26: reuses registerFOBAsTroopZone/unregisterTroopZone as-is, detects FARP destruction via Airbase.getByName + CTLDStaticWatcher (both already proven for this object class by the recon FARP detection code) instead of a new bespoke manager or the deferred generic zone-link system. Also marks the originating roadmap entry ("TRZ_ automatique...") as formalized for both its FOB and FARP halves.
Single AFK ticket: troopPickupAtFARP + farpTroopPickupRadius settings, final registration step on all 3 FARP scenes (reusing registerFOBAsTroopZone/unregisterTroopZone and CTLDStaticWatcher as-is), tests, CHANGELOG, dev+MM docs. Granularity confirmed with the user - one cohesive slice, same reasoning as FIX-FOB-TROOP-PICKUP.
Follows FIX-FOB-TROOP-PICKUP (PR #136): a built FARP had no troop pickup capability at all, unlike a FOB. New troopPickupAtFARP (default true) and farpTroopPickupRadius (150) settings, plus a final step on all three FARP scenes (farpScene, FARP Alpha, Countryside FARP) that resolves the built Airbase and registers a troop zone via the existing registerFOBAsTroopZone/unregisterTroopZone (reused as-is, including their collision guard and F10 displayName). Destruction detection needs no bespoke manager: a FARP is a real DCS Airbase, so its death is binary and native (Airbase:isExist()). Reuses CTLDStaticWatcher, already proven for this exact object class by CTLDReconManager's FARP-detection code, instead of building a CTLDFARPManager or the deferred generic zone-link system. New shared CTLDZoneManager:registerFARPTroopPickupFromScene(ctx) avoids tripling the same ~15 lines across the three scene files. FEAT-FARP-TROOP-PICKUP, grilled with the user 2026-08-26.
Reviewer's GuideBuilt FARP scenes now automatically expose configurable troop pickup zones by resolving their spawned DCS Airbase, registering through the existing CTLD troop-zone machinery, and watching native Airbase existence for cleanup; the change includes shared lifecycle wiring, configuration and documentation updates, generated defaults, and comprehensive tests for all three scenes. Sequence diagram for FARP troop pickup registration and cleanupsequenceDiagram
participant Scene as FARP Scene
participant Manager as CTLDZoneManager
participant DCS as DCS Airbase
participant Watcher as CTLDStaticWatcher
participant Zone as Troop Zone
Scene->>Manager: registerFARPTroopPickupFromScene(ctx)
Manager->>DCS: Airbase.getByName(name)
DCS-->>Manager: Airbase
alt troopPickupAtFARP enabled
Manager->>Zone: registerFOBAsTroopZone(name, point, radius, coalitionId)
Zone-->>Manager: registration succeeded
Manager->>Watcher: watch(trz_farp_name, isExist, unregisterTroopZone)
Watcher->>DCS: isExist()
DCS-->>Watcher: false
Watcher->>Manager: unregisterTroopZone(name)
Manager->>Zone: remove troop zone
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/CTLD_config_schema.yaml" line_range="438-442" />
<code_context>
en: Radius (m) within which troops may board at a FOB
fr: Rayon (m) dans lequel les troops peuvent embarquer à un FOB
+farpTroopPickupRadius:
+ group: fob
+ standard: true
+ label: {en: FARP troop boarding radius, fr: Rayon d'embarquement à une FARP}
+ unit: m
+ description:
+ en: Radius (m) within which troops may board at a FARP
+ fr: Rayon (m) dans lequel les troops peuvent embarquer à une FARP
</code_context>
<issue_to_address>
**nitpick:** `farpTroopPickupRadius` is assigned to the `fob` schema group even though the feature defines it as a FARP troop setting alongside `troopPickupAtFARP` in the `troops` group, so schema-driven configuration UIs and generated documentation place the setting under the wrong section.
**Suggested fix:** Change the schema entry's group from `fob` to `troops` and regenerate any derived configuration artifacts.
</issue_to_address>
### Comment 2
<location path="tests/ci/unit/scenes_minefields_spec.lua" line_range="276-287" />
<code_context>
+ return model.steps[#model.steps].func
+ end
+
+ it("registers a pickup-capable troop zone when troopPickupAtFARP is true", function()
+ local pad = fakeHelipad(sceneName .. "-1", { x = 10, y = 0, z = 20 })
+ local ab = fakeAirbase(pad)
+ Airbase.getByName = function(n) return (n == pad:getName()) and ab or nil end
+
+ lastStepFunc()(ctxFor(pad))
+
+ local zone = CTLDZoneManager.getInstance():getTroopZoneAtPoint(
+ { x = 10, y = 0, z = 20 }, coalition.side.BLUE)
+ assert.is_not_nil(zone)
+ assert.is_true(zone:hasPickup())
+ end)
+
+ it("registers no troop zone when troopPickupAtFARP is false", function()
</code_context>
<issue_to_address>
**nitpick (testing):** The successful-registration test leaves its `CTLDStaticWatcher` entry and scheduled watcher timer installed after the test; the suite resets `_troopZones` but not the shared watcher registry, so later tests run with stale closures and accumulated watcher state.
**Triggers:** When the test suite continues after any of the three successful-registration cases.
**Suggested fix:** Unwatch the generated FARP watcher or reset the shared `CTLDStaticWatcher` state in `after_each`.
</issue_to_address>
### Comment 3
<location path="CHANGELOG.md" line_range="21-22" />
<code_context>
+ scene completes, discovered through the same `CTLDZoneManager` machinery as a Mission-Editor
+ `TRZ_…` zone or a built FOB. Follows directly from `FIX-FOB-TROOP-PICKUP`: reuses
+ `registerFOBAsTroopZone`/`unregisterTroopZone` as-is.
+- Unlike a FOB, a FARP registers as a real DCS airbase, so its destruction is detected natively
+ (`Airbase:isExist()`) via the existing `CTLDStaticWatcher` — already used for this exact object
+ class by the recon FARP-detection code — rather than a new bespoke manager. The zone disappears
+ the moment DCS considers the FARP destroyed.
+
### Fixed — troops can now actually be picked up at a built FOB (FIX-FOB-TROOP-PICKUP)
</code_context>
<issue_to_address>
**nitpick:** The documentation says the troop zone disappears immediately when DCS considers the FARP destroyed, but the implementation only polls `Airbase:isExist()` on the installer's one-second `CTLDStaticWatcher` timer, so the zone remains available until the next poll.
**Triggers:** When a FARP is destroyed between watcher polls.
**Suggested fix:** Document the polling delay, or make the destruction path event-driven if immediate removal is a required contract.
```suggestion
class by the recon FARP-detection code — rather than a new bespoke manager. The zone is removed
on the next `CTLDStaticWatcher` poll, within one second of DCS considering the FARP destroyed.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+438
to
+442
| group: fob | ||
| standard: true | ||
| label: {en: FARP troop boarding radius, fr: Rayon d'embarquement à une FARP} | ||
| unit: m | ||
| description: |
There was a problem hiding this comment.
nitpick: farpTroopPickupRadius is assigned to the fob schema group even though the feature defines it as a FARP troop setting alongside troopPickupAtFARP in the troops group, so schema-driven configuration UIs and generated documentation place the setting under the wrong section.
Suggested fix: Change the schema entry's group from fob to troops and regenerate any derived configuration artifacts.
Comment on lines
+276
to
+287
| it("registers a pickup-capable troop zone when troopPickupAtFARP is true", function() | ||
| local pad = fakeHelipad(sceneName .. "-1", { x = 10, y = 0, z = 20 }) | ||
| local ab = fakeAirbase(pad) | ||
| Airbase.getByName = function(n) return (n == pad:getName()) and ab or nil end | ||
|
|
||
| lastStepFunc()(ctxFor(pad)) | ||
|
|
||
| local zone = CTLDZoneManager.getInstance():getTroopZoneAtPoint( | ||
| { x = 10, y = 0, z = 20 }, coalition.side.BLUE) | ||
| assert.is_not_nil(zone) | ||
| assert.is_true(zone:hasPickup()) | ||
| end) |
There was a problem hiding this comment.
nitpick (testing): The successful-registration test leaves its CTLDStaticWatcher entry and scheduled watcher timer installed after the test; the suite resets _troopZones but not the shared watcher registry, so later tests run with stale closures and accumulated watcher state.
Triggers: When the test suite continues after any of the three successful-registration cases.
Suggested fix: Unwatch the generated FARP watcher or reset the shared CTLDStaticWatcher state in after_each.
Comment on lines
+21
to
+22
| class by the recon FARP-detection code — rather than a new bespoke manager. The zone disappears | ||
| the moment DCS considers the FARP destroyed. |
There was a problem hiding this comment.
nitpick: The documentation says the troop zone disappears immediately when DCS considers the FARP destroyed, but the implementation only polls Airbase:isExist() on the installer's one-second CTLDStaticWatcher timer, so the zone remains available until the next poll.
Triggers: When a FARP is destroyed between watcher polls.
Suggested fix: Document the polling delay, or make the destruction path event-driven if immediate removal is a required contract.
Suggested change
| class by the recon FARP-detection code — rather than a new bespoke manager. The zone disappears | |
| the moment DCS considers the FARP destroyed. | |
| class by the recon FARP-detection code — rather than a new bespoke manager. The zone is removed | |
| on the next `CTLDStaticWatcher` poll, within one second of DCS considering the FARP destroyed. |
8-angle review of PR #137, findings applied: - CTLDSceneManager:packScene destroyed a FARP's objects with no cleanup of any troop zone/watcher registered for it, relying on an unverified DCS behavior (does destroying a packed static also flip a separately-resolved Airbase handle's isExist()?) that this suite cannot check. Only Countryside FARP supports packing; its onRepack (already called by packScene before destruction) now calls unregisterTroopZone/CTLDStaticWatcher:unwatch explicitly first. - CTLDStaticWatcher:watch now logs a WARN when it silently overwrites a still-live entry for the same id - a pre-existing gap in the shared cross-feature registry, cheap and consistent with the collision guard registerFOBAsTroopZone already has. New static_watcher_spec.lua covers watch/unwatch/tick and the WARN. - registerFARPTroopPickupFromScene: documented why it lives on CTLDZoneManager rather than a dedicated CTLDFARPManager (no such entity exists or is needed); removed a redundant ab:getName() call; added a WARN on its two previously-silent failure branches. - Tests: replaced a hand-rolled ctld.gs monkey-patch with ctldTestSettings.borrow/:restore (the same fix already applied on PR #136, missed again here); shared fakeHelipad/fakeAirbase/ctxFor across describe blocks instead of duplicating; trimmed unused unit/_params fields from the ctx fixture; fixed a miscounted step total in a test description string. FEAT-FARP-TROOP-PICKUP, PRD post-review addendum.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FIX-FOB-TROOP-PICKUP(PR fix(fob): reconnect troopPickupAtFOB to the F10 troop pickup path #136): a built FARP had no troop pickup capability at all, unlike a FOB. NewtroopPickupAtFARP(defaulttrue) andfarpTroopPickupRadius(default150, independent of the FOB's own radius) settings.farpScene,FARP Alpha,Countryside FARP) gain a final step that resolves the builtAirbaseand registers a troop pickup zone via the existingregisterFOBAsTroopZone/unregisterTroopZone(reused as-is — collision guard and F10displayNameinherited for free).Airbase, so its death is binary and native (Airbase:isExist()). ReusesCTLDStaticWatcher, already proven for this exact object class byCTLDReconManager's FARP-detection code, instead of a newCTLDFARPManageror the deferred generic zone-link system (dev/roadmap.md).CTLDZoneManager:registerFARPTroopPickupFromScene(ctx)avoids tripling the same logic across the three scene files..backlog/FEAT-FARP-TROOP-PICKUP/PRD.md).Test plan
tests/ci/unit/scenes_minefields_spec.luafor all three scenes (newdescribeblock forCountryside FARP, previously untested), plus behavior tests driving each scene's finalstep.func(ctx)directly and asserting through the publicCTLDZoneManager:getTroopZoneAtPointpath (zone registered whentroopPickupAtFARP=true, none whenfalse, removed afterCTLDStaticWatcher:_tickonce the stubbedAirbase:isExist()flips false).busted --pattern=_spec --helper=tests/ci/helpers/init.lua tests/ci→ 1347 passed / 0 failed / 1 pending (pre-existing, DCS-live gated).tests/ci/data/config_defaults.jsonregenerated viactld-tools gen(oracle drift guard) after the two new settings.luac -p CTLD.lua— Lua 5.1 syntax OK (rebuilt viamerge_CTLD.ps1).CLAUDE.md, relies on CI).Summary by Sourcery
Add automatic troop pickup support to built FARPs with configurable behavior and lifecycle cleanup.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: