fix: apply FullGas review answers (POST-FULLGAS-FIXES)#22
Merged
Conversation
Commit 0a15814 (DCS-cargo fix) dropped the loadableList computation in refreshRequestEquipmentSection and hardcoded spawnAsVehicle=false, silently disabling whole-vehicle spawn from the Request Equipment menu. Restored the Feature Q logic verbatim from the b1ddfe4 reference: build loadableList from caps.loadableVehiclesRED/BLUE when canTransportWholeVehicle, and set spawnAsVehicle per item when its type is in that list. Regression confirmed by FullGas. Validated live against Test_CTLDNEXT_01: scenario_fq_vehicle_whole_transport PASS 9/9 (was 1/9). CTLD.lua rebuilt. Opens lot POST-FULLGAS-FIXES (applies FullGas's review answers). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…et 06) The coord_farp-1 static is absent from the VEAF test mission. Instead of editing the .miz binary, anchor both FARP spawn scenarios to David's provided map coords (x=-356482, z=616908, ~6.4 m) with default BLUE/USA. Validated live: METAL-FARP-SPAWN PASS, CS-FARP-SPAWN PASS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- WarnAndSkip (ticket 03): rewritten against the current design — asserts Farp_FG_Petit_Helipad (probeSkip=true) is excluded, the 3 stock heliports appear as probeType=HELIPORT, and an INFO "skipped" is logged. PASS 8/8. - ProbeLifeCheck (ticket 02 part): run-once guard. PASS 4/4. ProbeOffMap (ticket 02) NOT included: its C3 count-diff still fails because CTLD init pre-creates ghost airbases (7 present), so the test's name collides — FullGas's "run 1 before=0" assumption doesn't hold here. Needs a deeper fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…er (ticket 04) - Apply FullGas's corrections: F-117 gates on reconF10Menu (not reconEnabled) and asserts the "reconF10Menu" keyword; F-118 inverts U-04/U-05 to the current design — scan() has no early-return, so _activeScans stays SET with zero targets after an all-layers-OFF scan (old marks cleaned). The agent had kept the stale == nil assertions; corrected to ~= nil + zero-targets. - Make both self-sufficient: scan() takes a unit (not a live client), so spawn a throwaway ground observer resolvable via Unit.getByName instead of requiring a BLUE player. Stays tier auto. Observer destroyed + config restored on teardown. Validated live: F-117 PASS 3/3, F-118 PASS 5/5. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… pre-creates ghosts) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… ticket 02 done C3 no longer counts before/after ghost airbases (which collided with the ghosts CTLD init pre-creates at load, failing on the first run). It now captures the idx the valid SINGLE_HELIPAD probe used and asserts that specific ghost (CTLD_MVP_H<idx>) exists by name — robust to pre-existing ghosts. Run-once guard kept (protects C1 across re-runs). Validated live: PASS 4/4. Ticket 02 complete; ProbeOffMap un-flagged from FullGas (resolved here). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ket 05)
- _loadAIZonesFromConfig: pickMaxStock = entry.isPickup and 0 or nil, so dropoff-only
zones keep nil (hasPickup() stays false) while pickup zones use 0 (unlimited guard;
real per-template stock lives in _aiTroopStock).
- G3 validation: warn when troopStock is nil OR not a {[name]=N} table OR empty (catches
legacy scalars 0/-1/10). New message "troopStock nil/invalid — use a {[templateName]=N}
table" (i18n EN/FR/ES/KO updated). Decision (David): -1 legacy scalar → WARN.
- scenario_fr_ai_zones: rewrite the 7 affected checks to the new model (troopStock as a
named-template table; pickMaxStock=0 for pickup; new G3 message). PASS 150/150 live
(was 126/145). CTLD.lua rebuilt.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rc changes Document that a src/ change is picked up by reloading the mission (Shift+R), NOT by injecting the 1.2 MB CTLD.lua via exec_lua (times out, HTTP 504) nor by dofile-ing it over a live mission (re-runs init → freezes the DCS Lua thread). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reviewer's GuideApplies FullGas’s post-review corrections: fixes a Feature Q whole-vehicle spawn regression, aligns U-108 heliport probe behavior and tests with current design (including run-once guards and new ghost verification), updates recon F-117/F-118 tests to the latest recon API semantics, retools AI zone troop stock handling and warnings, hardcodes FARP spawn anchors, documents safe integration testing workflow, and adds PRD/ticket backlog docs for this fix lot. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…dation) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
refreshRequestEquipmentSection, consider replacing the hard-codedplayerObj.coalition == 1check withcoalition.side.RED(or the appropriate constant) to avoid relying on magic numeric coalition IDs. - The hardcoded observer/FARP coordinates (
x = -356482, y = 6.4, z = 616908) now appear in multiple scenarios; consider centralizing them in a shared helper or constant to keep these tests aligned if the location ever needs to change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `refreshRequestEquipmentSection`, consider replacing the hard-coded `playerObj.coalition == 1` check with `coalition.side.RED` (or the appropriate constant) to avoid relying on magic numeric coalition IDs.
- The hardcoded observer/FARP coordinates (`x = -356482, y = 6.4, z = 616908`) now appear in multiple scenarios; consider centralizing them in a shared helper or constant to keep these tests aligned if the location ever needs to change.
## Individual Comments
### Comment 1
<location path="src/CTLD_crate.lua" line_range="2762-2764" />
<code_context>
if sideOk and (not _crateIsJTAC(sc) or jtacOk) then
- -- Request Equipment always spawns crates, never a whole vehicle.
- -- Feature Q whole-vehicle spawn is handled by the dedicated "Load Vehicle" menu.
+ -- Feature Q: detect if this item should spawn a whole vehicle WAITING
local spawnAsVehicle = false
+ if loadableList then
</code_context>
<issue_to_address>
**nitpick (typo):** The word 'WAITING' in this comment looks like a leftover artifact and could be removed for clarity.
The trailing 'WAITING' reads like a copy/paste artifact and makes the comment harder to parse. Consider removing it or replacing it with a more explicit phrase if there’s a pending TODO here.
```suggestion
if sideOk and (not _crateIsJTAC(sc) or jtacOk) then
-- Feature Q: detect if this item should spawn a whole vehicle
local spawnAsVehicle = false
```
</issue_to_address>
### Comment 2
<location path=".backlog/POST-FULLGAS-FIXES/tickets/04-recon-f117-f118.md" line_range="6" />
<code_context>
+Status: ✅ done
+Type: AFK
+
+F-117/F-118 recon reconF10Menu + scan-stays-active rewrite
+
+## Blocked by
</code_context>
<issue_to_address>
**issue (typo):** Potential duplicated word "recon" in ticket description
"F-117/F-118 recon reconF10Menu + scan-stays-active rewrite" appears to include an unintended duplicate "recon". Please adjust the wording (for example, by removing one "recon" or separating it with punctuation) to make the description clearer.
```suggestion
F-117/F-118 recon F10Menu + scan-stays-active rewrite
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…ion.side.RED - Remove the leftover "WAITING" word from the Feature Q comment. - Replace magic `playerObj.coalition == 1` with `coalition.side.RED` (iso-behaviour, RED == 1) in refreshRequestEquipmentSection. - Fix duplicated "recon" in ticket 04 description. CTLD.lua rebuilt. No behaviour change (comment + constant only). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Context
Applies FullGas's answers to our
dev/fullgas-report.mdfindings. One real productregression, several stale tests rewritten against the current design, a mission-content
gap, and a doc note. Lot
POST-FULLGAS-FIXES— see.backlog/POST-FULLGAS-FIXES/PRD.md.Tickets
refreshRequestEquipmentSection(dropped by fork commit 0a15814, restored from b1ddfe4).Live:
fq_vehicle_whole_transport9/9 (was 1/9).probe's own ghost by name, robust to init-created ghosts). Both 4/4 live.
self-sufficient (spawn a ground observer instead of needing a BLUE player). 3/3, 5/5 live.
pickMaxStockconditional + G3 WARN on invalidtroopStockformat(i18n EN/FR/ES/KO); 7 checks rewritten to the
{[templateName]=N}table model. 150/150 live(was 126/145).
the mission). Both PASS live.
Also: documented mission reload (
Shift+R) as the way to loadsrc/changes in theintegration-testingskill (injecting the 1.2 MBCTLD.luatimes out; dofile-over-live freezes DCS).🤖 Generated with Claude Code
Summary by Sourcery
Apply FullGas review follow-ups across CTLD core, tests, and integration tooling.
Bug Fixes:
Enhancements:
Tests: