feat(config): safe Mission Maker userSetup API (FEAT-USERCONFIG-API)#45
Merged
Conversation
…ation Implements FEAT-USERCONFIG-API: ctld.userSetup callbacks with add/remove/patch helpers, injectAACrates moved into ctld.initialize(), userConfig template rewrite, Section 1 default fixes. Docs (EN+FR) + CHANGELOG updated.
Reviewer's GuideIntroduces a guarded Mission Maker configuration callback API Sequence diagram for config materialisation with ctld.userSetupsequenceDiagram
participant ctld
participant CTLDConfig
participant CTLDCrateAssemblyManager
participant CTLDCrateManager
participant userSetupCallback
ctld->>CTLDConfig: get()
CTLDConfig-->>ctld: configInstance
ctld->>CTLDConfig: load()
ctld->>CTLDConfig: gs(spawnableCrates)
CTLDConfig-->>ctld: spawnableCrates
ctld->>CTLDCrateAssemblyManager: injectAACrates(spawnableCrates)
ctld->>ctld: runUserSetup()
loop for each ctld.userSetup callback
ctld->>userSetupCallback: pcall(callback, configInstance)
userSetupCallback->>CTLDConfig: mutate settings via helpers
end
CTLDCrateManager->>CTLDConfig: gs(spawnableCrates)
CTLDConfig-->>CTLDCrateManager: spawnableCrates (defaults + AA + user patches)
CTLDCrateManager->>CTLDCrateManager: _processSpawnableCrates()
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 1 issue, and left some high level feedback:
- In
ctld.addCrate, consider explicitly validating thatentryandentry.weightare non-nil and of the expected type, and logging a clear warning when they are not, to avoid silently inserting crates that can never be referenced or removed later. - For
ctld.addTo, you might want to assert or guard that the target setting is a numerically indexed array (e.g. via#andipairs) beforetable.insert, so that accidental use with a dictionary-like table is caught early rather than mutating arbitrary config structures.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ctld.addCrate`, consider explicitly validating that `entry` and `entry.weight` are non-nil and of the expected type, and logging a clear warning when they are not, to avoid silently inserting crates that can never be referenced or removed later.
- For `ctld.addTo`, you might want to assert or guard that the target setting is a numerically indexed array (e.g. via `#` and `ipairs`) before `table.insert`, so that accidental use with a dictionary-like table is caught early rather than mutating arbitrary config structures.
## Individual Comments
### Comment 1
<location path="src/CTLD_userSetup.lua" line_range="43-64" />
<code_context>
+---@param section string
+---@param entry table
+---@return boolean added
+function ctld.addCrate(section, entry)
+ local crates = _settings()["spawnableCrates"]
+ if type(crates) ~= "table" then
+ ctld.logWarning("ctld.addCrate: spawnableCrates unavailable — entry rejected")
+ return false
+ end
+ if entry and entry.weight and _findCrate(entry.weight) then
+ local msg = string.format(
+ "CTLD userConfig: duplicate crate weight %s — entry rejected", tostring(entry.weight))
+ ctld.logWarning("ctld.addCrate: %s", msg)
+ if trigger and trigger.action and trigger.action.outText then
+ trigger.action.outText(msg, 30)
+ end
+ return false
+ end
+ crates[section] = crates[section] or {}
+ table.insert(crates[section], entry)
+ return true
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Validate `entry.weight` presence/type before inserting crates to avoid hard-to-debug catalogue issues.
As written, crates with `weight = nil` or a non-number still get added to `spawnableCrates`, but later logic (e.g. `_findCrate` and the crate manager) assumes `weight` is a valid numeric key. That mismatch will manifest as odd runtime behaviour instead of a clear configuration error. Consider enforcing `type(entry.weight) == "number"` before insertion and logging/rejecting any invalid entries.
```suggestion
---@param section string
---@param entry table
---@return boolean added
function ctld.addCrate(section, entry)
local crates = _settings()["spawnableCrates"]
if type(crates) ~= "table" then
ctld.logWarning("ctld.addCrate: spawnableCrates unavailable — entry rejected")
return false
end
if type(entry) ~= "table" then
ctld.logWarning("ctld.addCrate: invalid entry (expected table, got %s) — entry rejected", type(entry))
return false
end
local weight = entry.weight
if type(weight) ~= "number" then
local msg = string.format(
"CTLD userConfig: crate weight must be a number (got %s) — entry rejected",
tostring(weight)
)
ctld.logWarning("ctld.addCrate: %s", msg)
if trigger and trigger.action and trigger.action.outText then
trigger.action.outText(msg, 30)
end
return false
end
if _findCrate(weight) then
local msg = string.format(
"CTLD userConfig: duplicate crate weight %s — entry rejected",
tostring(weight)
)
ctld.logWarning("ctld.addCrate: %s", msg)
if trigger and trigger.action and trigger.action.outText then
trigger.action.outText(msg, 30)
end
return false
end
crates[section] = crates[section] or {}
table.insert(crates[section], entry)
return true
end
```
</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
+43
to
+64
| ---@param section string | ||
| ---@param entry table | ||
| ---@return boolean added | ||
| function ctld.addCrate(section, entry) | ||
| local crates = _settings()["spawnableCrates"] | ||
| if type(crates) ~= "table" then | ||
| ctld.logWarning("ctld.addCrate: spawnableCrates unavailable — entry rejected") | ||
| return false | ||
| end | ||
| if entry and entry.weight and _findCrate(entry.weight) then | ||
| local msg = string.format( | ||
| "CTLD userConfig: duplicate crate weight %s — entry rejected", tostring(entry.weight)) | ||
| ctld.logWarning("ctld.addCrate: %s", msg) | ||
| if trigger and trigger.action and trigger.action.outText then | ||
| trigger.action.outText(msg, 30) | ||
| end | ||
| return false | ||
| end | ||
| crates[section] = crates[section] or {} | ||
| table.insert(crates[section], entry) | ||
| return true | ||
| end |
There was a problem hiding this comment.
suggestion (bug_risk): Validate entry.weight presence/type before inserting crates to avoid hard-to-debug catalogue issues.
As written, crates with weight = nil or a non-number still get added to spawnableCrates, but later logic (e.g. _findCrate and the crate manager) assumes weight is a valid numeric key. That mismatch will manifest as odd runtime behaviour instead of a clear configuration error. Consider enforcing type(entry.weight) == "number" before insertion and logging/rejecting any invalid entries.
Suggested change
| ---@param section string | |
| ---@param entry table | |
| ---@return boolean added | |
| function ctld.addCrate(section, entry) | |
| local crates = _settings()["spawnableCrates"] | |
| if type(crates) ~= "table" then | |
| ctld.logWarning("ctld.addCrate: spawnableCrates unavailable — entry rejected") | |
| return false | |
| end | |
| if entry and entry.weight and _findCrate(entry.weight) then | |
| local msg = string.format( | |
| "CTLD userConfig: duplicate crate weight %s — entry rejected", tostring(entry.weight)) | |
| ctld.logWarning("ctld.addCrate: %s", msg) | |
| if trigger and trigger.action and trigger.action.outText then | |
| trigger.action.outText(msg, 30) | |
| end | |
| return false | |
| end | |
| crates[section] = crates[section] or {} | |
| table.insert(crates[section], entry) | |
| return true | |
| end | |
| ---@param section string | |
| ---@param entry table | |
| ---@return boolean added | |
| function ctld.addCrate(section, entry) | |
| local crates = _settings()["spawnableCrates"] | |
| if type(crates) ~= "table" then | |
| ctld.logWarning("ctld.addCrate: spawnableCrates unavailable — entry rejected") | |
| return false | |
| end | |
| if type(entry) ~= "table" then | |
| ctld.logWarning("ctld.addCrate: invalid entry (expected table, got %s) — entry rejected", type(entry)) | |
| return false | |
| end | |
| local weight = entry.weight | |
| if type(weight) ~= "number" then | |
| local msg = string.format( | |
| "CTLD userConfig: crate weight must be a number (got %s) — entry rejected", | |
| tostring(weight) | |
| ) | |
| ctld.logWarning("ctld.addCrate: %s", msg) | |
| if trigger and trigger.action and trigger.action.outText then | |
| trigger.action.outText(msg, 30) | |
| end | |
| return false | |
| end | |
| if _findCrate(weight) then | |
| local msg = string.format( | |
| "CTLD userConfig: duplicate crate weight %s — entry rejected", | |
| tostring(weight) | |
| ) | |
| ctld.logWarning("ctld.addCrate: %s", msg) | |
| if trigger and trigger.action and trigger.action.outText then | |
| trigger.action.outText(msg, 30) | |
| end | |
| return false | |
| end | |
| crates[section] = crates[section] or {} | |
| table.insert(crates[section], entry) | |
| return true | |
| end |
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.
Implements the FEAT-USERCONFIG-API lot (see
.backlog/FEAT-USERCONFIG-API/PRD.md).What
ctld.userSetupAPI — Mission Makers customise the complex config tables from setupcallbacks instead of the silently-broken Section 2 of
CTLD_userConfig.lua(which calledCTLDConfig.get()before CTLD defined it). Helpers:addCrate,removeCrate,patchCrate(deep-merge one level),
addTroopGroup,removeTroopGroup,addTo,logDefaults. Eachcallback runs guarded — a failing one warns without aborting the others or the mission.
injectAACratesrelocated fromCTLDCrateManager:_processSpawnableCrates()toctld.initialize()(before the userSetup callbacks).initialize()is now the single placethat materialises the full config: defaults → AA injection → userSetup → managers.
CTLD_userConfig.luatemplate rewritten — broken Section 2 replaced by documentedctld.userSetupexamples + per-table field schemas; test-only debug block removed; Section 1defaults corrected (
parachuteMinAltitude*= 152,JTAC_droneAltitude= 4000).mission-maker/configuration.md(EN+FR) updated to the callback-based flow.Tests
usersetup_spec.lua(7 helpers); extendedconfig_spec.lua(dispatch: order, nil-safe,mutations, failure isolation) and
aasystem_spec.lua(injection relocation +_weightIndexnon-regression).
lua5.1(busted not installable locally → CI);CTLD.luarebuilt,parses clean, no BOM.
Notes
CTLD_config.luadefaults were already correct.Summary by Sourcery
Introduce a safe, callback-based Mission Maker configuration API via ctld.userSetup, centralize config materialization in initialization, update the user configuration template and documentation, and expand tests around the new API and AA crate injection ordering.
New Features:
Enhancements:
Documentation:
Tests: