Conversation
📝 WalkthroughWalkthroughIntroduces a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ConstructionPanel as Construction Panel
participant TemplatesModule as Templates Module
participant TemplateSource as Template Source
participant Filter as Buildability Filter
participant PreviewUI as Preview UI
User->>ConstructionPanel: Select construction tab
ConstructionPanel->>TemplatesModule: Main() initialization
TemplatesModule->>TemplateSource: Load template definitions
TemplateSource-->>TemplatesModule: Template data
User->>ConstructionPanel: Hover over template grid item
ConstructionPanel->>TemplatesModule: Mouse enter on template
TemplatesModule->>Filter: Filter template against buildable units
Filter-->>TemplatesModule: Buildable?
alt Template buildable
TemplatesModule->>PreviewUI: Create preview control
PreviewUI-->>User: Display template preview
end
User->>ConstructionPanel: Click template
ConstructionPanel->>TemplatesModule: Template selected
TemplatesModule->>ConstructionPanel: Activate build/factory command
User->>ConstructionPanel: Exit preview/tab
TemplatesModule->>PreviewUI: Destroy preview UI
sequenceDiagram
participant BuildTemplate as Build Template<br/>Filtering
participant UnitConversion as Faction Unit<br/>Conversion
participant BuildableCheck as Buildability<br/>Check
participant BuildCommand as Build Command<br/>Execution
BuildTemplate->>UnitConversion: Convert template unit IDs<br/>(apply faction prefix)
UnitConversion-->>BuildTemplate: Faction-specific unit IDs
BuildTemplate->>BuildableCheck: Verify all units buildable
alt All units buildable
BuildableCheck-->>BuildTemplate: Include template
BuildTemplate->>BuildCommand: Enable selection & activation
else Missing units
BuildableCheck-->>BuildTemplate: Exclude template
BuildTemplate->>BuildCommand: Disable/hide template
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes The module introduces substantial new feature logic across multiple interconnected files. Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
mods/ReUI.Construction.Templates/FactoryTemplatePreview.lua (2)
1-4: Unused importsCheckBoxandGroup.Neither is referenced in this file. Consider dropping them to keep the module surface clean.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mods/ReUI.Construction.Templates/FactoryTemplatePreview.lua` around lines 1 - 4, The file imports CheckBox and Group but never uses them; remove the unused imports by deleting or commenting out the lines that bind ReUI.UI.Controls.CheckBox and ReUI.UI.Controls.Group (the local variables named CheckBox and Group) so only actually used controls (Bitmap, Text) remain imported; run a quick grep to confirm no other references to CheckBox or Group in this module before committing.
38-41: Icon lookup lacks fallback; consider reusingUIUtil.UIFilewith existence check.
BuildTemplatePreview.GetUnitImage(sibling file, lines 26-30) probes for the icon on disk and falls back tounidentified.dds. HereIcon:Setunconditionally sets/icons/units/<id>_icon.ddsviaSkinnableFile, so templates referencing units without shipped icons will render as missing texture. Recommend reusing the same helper for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mods/ReUI.Construction.Templates/FactoryTemplatePreview.lua` around lines 38 - 41, The Set function in FactoryTemplatePreview currently calls self:SetTexture(UIUtil.SkinnableFile('/icons/units/' .. data.id .. '_icon.dds')) unconditionally, which shows missing textures for units without shipped icons; change it to use the same lookup/fallback logic as BuildTemplatePreview.GetUnitImage (or call that helper) so you first probe for the icon file and fall back to 'unidentified.dds' when missing, and update the call site (the Set method referencing self:SetTexture and UIUtil.SkinnableFile) to use the returned safe path instead of building the path inline.mods/ReUI.Construction.Templates/Options.lua (1)
5-13: DefaulttemplateNameLengthsits at the slider maximum.Default is
10, which is also the slider's upper bound (line 13). Users tweaking the name length will only ever shorten; the screenshot in the PR description shows8as a more natural default. Consider lowering the default or raising the max if longer template names are expected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mods/ReUI.Construction.Templates/Options.lua` around lines 5 - 13, The default templateNameLength is set to 10 which equals the slider's max in Options.Slider, so users cannot increase it; update the default to a lower sensible value (e.g., 8) or increase the slider max to a larger value to match expected defaults. Locate the templateNameLength assignment and/or the Options.Slider call in Main (symbols: templateNameLength and Options.Slider("Templates name length", 1, 10, 1, options.templateNameLength, 4)) and change either the default value (from 10 to 8) or the slider max (the 3rd numeric arg currently 10) so the default is not at the upper bound. Ensure the default and slider bounds remain consistent after the change.mods/ReUI.Construction.Templates/Templates.lua (2)
305-322: Drop or finalize commented-out blocks.Lines 306-321 (and the related
RemoveByKeystub at 364) are a large dead-code chunk referencing an incompleteUpdateEvent/self.displayTemplatesflow. Either finish wiring it up or remove it so maintainers don't have to reason about stale intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mods/ReUI.Construction.Templates/Templates.lua` around lines 305 - 322, The large commented-out block in OnInit referencing UpdateEvent, self.displayTemplates, and panel:SetAvailableTech should be either removed or fully implemented: either delete the commented lines and also remove the related RemoveByKey stub (to avoid dead code), or restore and wire it up by storing the panel (self.panel = panel), calling panel.UpdateEvent:AddByKey(self.Name, function(panel, context) ... end) to check context.tab and call panel:SetAvailableTech({ ["BUILD_TEMPLATES"] = self.displayTemplates }), and implement a matching cleanup using UpdateEvent:RemoveByKey(self.Name) (where the current RemoveByKey stub exists) so the handler is unregistered on teardown.
47-71:ConvertToBuildable:ibleeds between loops and iteration order ofpairsis undefined.The first
for faction, prefixes in pairs(PREFIXES)loop (lines 52-57) exits early withiset to the position ofprefin some faction's prefix list. Because the prefix lists for all four factions are positionally aligned (T1/T2/T2-sub/experimental), the second loop then reusesias the positional index across factions, which works — but only because all fourPREFIXESarrays share the same ordering convention. This invariant is load-bearing and not documented; a future edit that reorders one faction's prefixes would silently produce wrong cross-faction conversions. Consider adding a brief comment asserting the positional-alignment contract, or indexing by tier name instead of numeric position.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mods/ReUI.Construction.Templates/Templates.lua` around lines 47 - 71, ConvertToBuildable currently relies on a numeric index i bleeding between two pairs() loops which ties correctness to ordering in PREFIXES; instead determine the tier key for the matched prefix and use that key to index other factions. Change the first loop in ConvertToBuildable to find the tierKey (e.g., "T1","T2", etc.) by iterating prefixes and comparing values to pref (or update Contains to return the tierKey), then in the second loop use prefixes[tierKey] to build newId; reference ConvertToBuildable, PREFIXES and avoid using the shared numeric variable i.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mods/ReUI.Construction.Templates/BuildTemplatePreview.lua`:
- Around line 86-143: The code computes offsets only for entries where
__blueprints[id] exists but later always constructs Icons and calls
GetUnitSkirtSizes/GetSkirtCentreOffset unguarded, leading to nil-deref; fix by
applying the same guard to the icon creation and layout loops: in the loop that
fills self.icons and in InitLayout's loop (both iterating template.templateData
from index 3), skip any entry where not __blueprints[id] (i.e., wrap
Icon(self,id) creation and the calls to
GetUnitSkirtSizes/GetSkirtCentreOffset/GetSkirtCentreOffset inside an if
__blueprints[id] then ... end), or alternatively remove the initial
__blueprints[id] check and ensure all helper functions
(GetBackgroundImage/GetUnitImage/GetUnitSkirtSizes/GetSkirtCentreOffset) safely
handle missing blueprints; prefer adding the consistent if __blueprints[id]
guard around Icon construction and layout to match the offset computation.
- Around line 38-44: GetSkirtCentreOffset can nil-arithmetic when
bp.Physics.SkirtOffsetX/Z are missing; change the locals in GetSkirtCentreOffset
to defensive fallbacks (e.g. read bp.Physics then set XSkirtO = bp.Physics and
bp.Physics.SkirtOffsetX or 0, and similarly ZSkirtO = bp.Physics and
bp.Physics.SkirtOffsetZ or 0) so arithmetic uses 0 when those fields are absent;
keep the existing GetUnitSkirtSizes call and size fallbacks as-is.
In `@mods/ReUI.Construction.Templates/FactoryTemplatePreview.lua`:
- Around line 101-127: The DisplayTemplate function can produce NaN when
template.templateData is empty; before computing count/columns/rows (used to set
self.Rows and self.Columns and drive IterateItemsVertically), guard the
empty-case: if template.templateData is nil or has zero length, either return
early (no items to display) or set count = 1 and clamp columns/rows to at least
1 (e.g. columns = math.max(1, math.min(count, MAX_COLUMNS)) and rows =
math.max(1, math.ceil(count / columns))). Update DisplayTemplate to handle a
nil/empty template.templateData and ensure self.Rows and self.Columns are never
set to 0 or NaN.
In `@mods/ReUI.Construction.Templates/Templates.lua`:
- Around line 539-543: The loop mutates handler.actions[1] blindly; instead
locate selection-tab handlers in ReUI.Construction.Panel.PrimaryHandlers and
change only actions that match the original value (e.g., replace "repeatBuild"
with "build_template") or append "build_template" if not present; specifically
update the loop around handler.tab and handler.actions to check handler.actions
entries for the string "repeatBuild" (or test for absence of "build_template")
and perform a targeted replace or append, and avoid hard-indexing [1]; this
ensures UpgradeChain and Selection (and any external handlers) are not
incorrectly clobbered.
- Around line 434-438: Remove the debug print statements that unconditionally
emit "Build template created" on checkbox toggle; specifically delete the print
call in the OnChecked handler that calls Templates.CreateBuildTemplate and the
other identical print at the second checkbox handler (the one around line 506),
or replace them with a proper logger call if needed (e.g., use the project's
logging utility at an appropriate log level). Ensure only the print statements
are removed/changed and leave the Templates.CreateBuildTemplate() calls and
checkBox:SetCheck(false, true) behavior intact.
---
Nitpick comments:
In `@mods/ReUI.Construction.Templates/FactoryTemplatePreview.lua`:
- Around line 1-4: The file imports CheckBox and Group but never uses them;
remove the unused imports by deleting or commenting out the lines that bind
ReUI.UI.Controls.CheckBox and ReUI.UI.Controls.Group (the local variables named
CheckBox and Group) so only actually used controls (Bitmap, Text) remain
imported; run a quick grep to confirm no other references to CheckBox or Group
in this module before committing.
- Around line 38-41: The Set function in FactoryTemplatePreview currently calls
self:SetTexture(UIUtil.SkinnableFile('/icons/units/' .. data.id .. '_icon.dds'))
unconditionally, which shows missing textures for units without shipped icons;
change it to use the same lookup/fallback logic as
BuildTemplatePreview.GetUnitImage (or call that helper) so you first probe for
the icon file and fall back to 'unidentified.dds' when missing, and update the
call site (the Set method referencing self:SetTexture and UIUtil.SkinnableFile)
to use the returned safe path instead of building the path inline.
In `@mods/ReUI.Construction.Templates/Options.lua`:
- Around line 5-13: The default templateNameLength is set to 10 which equals the
slider's max in Options.Slider, so users cannot increase it; update the default
to a lower sensible value (e.g., 8) or increase the slider max to a larger value
to match expected defaults. Locate the templateNameLength assignment and/or the
Options.Slider call in Main (symbols: templateNameLength and
Options.Slider("Templates name length", 1, 10, 1, options.templateNameLength,
4)) and change either the default value (from 10 to 8) or the slider max (the
3rd numeric arg currently 10) so the default is not at the upper bound. Ensure
the default and slider bounds remain consistent after the change.
In `@mods/ReUI.Construction.Templates/Templates.lua`:
- Around line 305-322: The large commented-out block in OnInit referencing
UpdateEvent, self.displayTemplates, and panel:SetAvailableTech should be either
removed or fully implemented: either delete the commented lines and also remove
the related RemoveByKey stub (to avoid dead code), or restore and wire it up by
storing the panel (self.panel = panel), calling
panel.UpdateEvent:AddByKey(self.Name, function(panel, context) ... end) to check
context.tab and call panel:SetAvailableTech({ ["BUILD_TEMPLATES"] =
self.displayTemplates }), and implement a matching cleanup using
UpdateEvent:RemoveByKey(self.Name) (where the current RemoveByKey stub exists)
so the handler is unregistered on teardown.
- Around line 47-71: ConvertToBuildable currently relies on a numeric index i
bleeding between two pairs() loops which ties correctness to ordering in
PREFIXES; instead determine the tier key for the matched prefix and use that key
to index other factions. Change the first loop in ConvertToBuildable to find the
tierKey (e.g., "T1","T2", etc.) by iterating prefixes and comparing values to
pref (or update Contains to return the tierKey), then in the second loop use
prefixes[tierKey] to build newId; reference ConvertToBuildable, PREFIXES and
avoid using the shared numeric variable i.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6a83bdd6-af8d-4984-9abf-062d4d0fbc7e
📒 Files selected for processing (6)
mods/ReUI.Construction.Templates/BuildTemplatePreview.luamods/ReUI.Construction.Templates/FactoryTemplatePreview.luamods/ReUI.Construction.Templates/ModuleMeta.luamods/ReUI.Construction.Templates/Options.luamods/ReUI.Construction.Templates/Templates.luamods/ReUI.Construction.Templates/mod_info.lua
| local function GetSkirtCentreOffset(id) | ||
| local bp = __blueprints[id] | ||
| local w, h = bp.Footprint.SizeX or bp.SizeX or 1, bp.Footprint.SizeZ or bp.SizeZ or 1 | ||
| local sW, sH = GetUnitSkirtSizes(id) | ||
| local XSkirtO, ZSkirtO = bp.Physics.SkirtOffsetX, bp.Physics.SkirtOffsetZ | ||
| return ((XSkirtO + ((sW + XSkirtO) - w))) / 2, ((ZSkirtO + ((sH + ZSkirtO) - h))) / 2 | ||
| end |
There was a problem hiding this comment.
GetSkirtCentreOffset will crash on blueprints without SkirtOffsetX/Z.
bp.Physics.SkirtOffsetX / SkirtOffsetZ are not fallback-guarded, unlike the size lookups. For any blueprint where these fields are absent (common on non-structures), the subsequent arithmetic at line 43 will throw a nil-arithmetic error. Default to 0 to match the guard style of GetUnitSkirtSizes.
🛡️ Proposed fix
local function GetSkirtCentreOffset(id)
local bp = __blueprints[id]
local w, h = bp.Footprint.SizeX or bp.SizeX or 1, bp.Footprint.SizeZ or bp.SizeZ or 1
local sW, sH = GetUnitSkirtSizes(id)
- local XSkirtO, ZSkirtO = bp.Physics.SkirtOffsetX, bp.Physics.SkirtOffsetZ
+ local XSkirtO = bp.Physics.SkirtOffsetX or 0
+ local ZSkirtO = bp.Physics.SkirtOffsetZ or 0
return ((XSkirtO + ((sW + XSkirtO) - w))) / 2, ((ZSkirtO + ((sH + ZSkirtO) - h))) / 2
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local function GetSkirtCentreOffset(id) | |
| local bp = __blueprints[id] | |
| local w, h = bp.Footprint.SizeX or bp.SizeX or 1, bp.Footprint.SizeZ or bp.SizeZ or 1 | |
| local sW, sH = GetUnitSkirtSizes(id) | |
| local XSkirtO, ZSkirtO = bp.Physics.SkirtOffsetX, bp.Physics.SkirtOffsetZ | |
| return ((XSkirtO + ((sW + XSkirtO) - w))) / 2, ((ZSkirtO + ((sH + ZSkirtO) - h))) / 2 | |
| end | |
| local function GetSkirtCentreOffset(id) | |
| local bp = __blueprints[id] | |
| local w, h = bp.Footprint.SizeX or bp.SizeX or 1, bp.Footprint.SizeZ or bp.SizeZ or 1 | |
| local sW, sH = GetUnitSkirtSizes(id) | |
| local XSkirtO = bp.Physics.SkirtOffsetX or 0 | |
| local ZSkirtO = bp.Physics.SkirtOffsetZ or 0 | |
| return ((XSkirtO + ((sW + XSkirtO) - w))) / 2, ((ZSkirtO + ((sH + ZSkirtO) - h))) / 2 | |
| end |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mods/ReUI.Construction.Templates/BuildTemplatePreview.lua` around lines 38 -
44, GetSkirtCentreOffset can nil-arithmetic when bp.Physics.SkirtOffsetX/Z are
missing; change the locals in GetSkirtCentreOffset to defensive fallbacks (e.g.
read bp.Physics then set XSkirtO = bp.Physics and bp.Physics.SkirtOffsetX or 0,
and similarly ZSkirtO = bp.Physics and bp.Physics.SkirtOffsetZ or 0) so
arithmetic uses 0 when those fields are absent; keep the existing
GetUnitSkirtSizes call and size fallbacks as-is.
| local td = template.templateData | ||
| local xOffset, zOffset = { 0, 0 }, { 0, 0 } | ||
|
|
||
| for i = 3, table.getn(td) do | ||
| local id = td[i][1] | ||
| if __blueprints[id] then | ||
| local w, h = GetUnitSkirtSizes(id) | ||
| local posX, posZ = td[i][3], td[i][4] | ||
| local cOffX, cOffZ = GetSkirtCentreOffset(id) | ||
| xOffset[1] = math.min(xOffset[1], (posX - w / 2) + cOffX) | ||
| xOffset[2] = math.max(xOffset[2], (posX + w / 2) + cOffX) | ||
| zOffset[1] = math.min(zOffset[1], (posZ - h / 2) + cOffZ) | ||
| zOffset[2] = math.max(zOffset[2], (posZ + h / 2) + cOffZ) | ||
| end | ||
| end | ||
|
|
||
|
|
||
| self.xOffset, self.zOffset = (xOffset[1] + xOffset[2]) / 2, (zOffset[1] + zOffset[2]) / 2 | ||
|
|
||
| self.icons = {} | ||
| for i = 3, table.getn(td) do | ||
| local id = td[i][1] | ||
| self.icons[i] = Icon(self, id) | ||
| end | ||
| end, | ||
|
|
||
| ---@param self BuildTemplatePreview | ||
| ---@param layouter ReUI.UI.Layouter | ||
| InitLayout = function(self, layouter) | ||
|
|
||
| layouter(self._title) | ||
| :Color(UIUtil.factionTextColor) | ||
| :DropShadow(true) | ||
| :AnchorToTop(self, 2) | ||
| :AtHorizontalCenterIn(self) | ||
|
|
||
| local td = self.template.templateData | ||
| local gridscale = 10 | ||
| local tXgridSize = td[1] | ||
| local tZgridSize = td[2] | ||
| local tScale = 300 / (math.max(tXgridSize, tZgridSize) * gridscale) | ||
| local scale = gridscale * tScale | ||
|
|
||
| local xOffset, zOffset = self.xOffset, self.zOffset | ||
|
|
||
| for i = 3, table.getn(td) do | ||
| local id = td[i][1] | ||
| local icon = self.icons[i] | ||
| local w, h = GetUnitSkirtSizes(id) | ||
| local xCenOff, zCenOff = GetSkirtCentreOffset(id) | ||
|
|
||
| layouter(icon) | ||
| :Width(w * scale) | ||
| :Height(h * scale) | ||
| :AtCenterIn(self, | ||
| (td[i][4] - zOffset + zCenOff) * scale, | ||
| (td[i][3] - xOffset + xCenOff) * scale) | ||
| end |
There was a problem hiding this comment.
Inconsistent __blueprints[id] guarding between the two loops.
The offset-computation loop (lines 89-100) skips entries missing from __blueprints, but the icon-construction loop (lines 106-109) creates an Icon for every entry regardless. Icon.__init then calls GetBackgroundImage(id) → Logic.GetLayerGroup(id) and GetUnitImage(id), and InitLayout at lines 131-143 unconditionally calls GetUnitSkirtSizes(id) / GetSkirtCentreOffset(id), both of which index __blueprints[id] and will nil-deref for ids absent from blueprints. Either skip such entries in both later loops or require the first loop's filter as a precondition.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mods/ReUI.Construction.Templates/BuildTemplatePreview.lua` around lines 86 -
143, The code computes offsets only for entries where __blueprints[id] exists
but later always constructs Icons and calls
GetUnitSkirtSizes/GetSkirtCentreOffset unguarded, leading to nil-deref; fix by
applying the same guard to the icon creation and layout loops: in the loop that
fills self.icons and in InitLayout's loop (both iterating template.templateData
from index 3), skip any entry where not __blueprints[id] (i.e., wrap
Icon(self,id) creation and the calls to
GetUnitSkirtSizes/GetSkirtCentreOffset/GetSkirtCentreOffset inside an if
__blueprints[id] then ... end), or alternatively remove the initial
__blueprints[id] check and ensure all helper functions
(GetBackgroundImage/GetUnitImage/GetUnitSkirtSizes/GetSkirtCentreOffset) safely
handle missing blueprints; prefer adding the consistent if __blueprints[id]
guard around Icon construction and layout to match the offset computation.
| DisplayTemplate = function(self, template) | ||
|
|
||
| self._title:SetText(template.name) | ||
|
|
||
| local data = template.templateData | ||
| local count = table.getn(data) | ||
|
|
||
| local columns = math.min(count, MAX_COLUMNS) | ||
| local rows = math.ceil(count / columns) | ||
|
|
||
| self.Rows = rows | ||
| self.Columns = columns | ||
|
|
||
| local i = 1 | ||
| ---@param item FactoryTemplatePreview.Icon | ||
| self:IterateItemsVertically(function(grid, item, row, column) | ||
| local itemData = data[i] | ||
| if itemData then | ||
| item:Show() | ||
| item:Set(itemData) | ||
| else | ||
| item:Hide() | ||
| end | ||
| i = i + 1 | ||
| end) | ||
|
|
||
| end, |
There was a problem hiding this comment.
Guard against empty templateData to avoid NaN grid sizing.
If data is empty, count = 0 makes columns = math.min(0, MAX_COLUMNS) = 0 and rows = math.ceil(0 / 0) which is NaN. Assigning NaN/0 to Rows/Columns will almost certainly produce a broken grid and may throw downstream. Short-circuit the empty case (or at least floor to 1x1) before computing dimensions.
🔧 Proposed guard
local data = template.templateData
local count = table.getn(data)
+ if count == 0 then
+ self.Rows = 1
+ self.Columns = 1
+ self:IterateItemsVertically(function(grid, item, row, column)
+ item:Hide()
+ end)
+ return
+ end
+
local columns = math.min(count, MAX_COLUMNS)
local rows = math.ceil(count / columns)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mods/ReUI.Construction.Templates/FactoryTemplatePreview.lua` around lines 101
- 127, The DisplayTemplate function can produce NaN when template.templateData
is empty; before computing count/columns/rows (used to set self.Rows and
self.Columns and drive IterateItemsVertically), guard the empty-case: if
template.templateData is nil or has zero length, either return early (no items
to display) or set count = 1 and clamp columns/rows to at least 1 (e.g. columns
= math.max(1, math.min(count, MAX_COLUMNS)) and rows = math.max(1,
math.ceil(count / columns))). Update DisplayTemplate to handle a nil/empty
template.templateData and ensure self.Rows and self.Columns are never set to 0
or NaN.
| OnChecked = function(self, checkBox, isChecked) | ||
| Templates.CreateBuildTemplate() | ||
| print "Build template created" | ||
| checkBox:SetCheck(false, true) | ||
| end, |
There was a problem hiding this comment.
Remove debug print statements.
Lines 436 and 506 both emit print "<...> template created" on every checkbox toggle. Please drop them (or route through a proper log level) before release.
🧹 Proposed cleanup
OnChecked = function(self, checkBox, isChecked)
Templates.CreateBuildTemplate()
- print "Build template created"
checkBox:SetCheck(false, true)
end, FactoryTemplates.CreateBuildTemplate(currentCommandQueue)
-
- print "Factory template created"
checkBox:SetCheck(false, true)
end,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mods/ReUI.Construction.Templates/Templates.lua` around lines 434 - 438,
Remove the debug print statements that unconditionally emit "Build template
created" on checkbox toggle; specifically delete the print call in the OnChecked
handler that calls Templates.CreateBuildTemplate and the other identical print
at the second checkbox handler (the one around line 506), or replace them with a
proper logger call if needed (e.g., use the project's logging utility at an
appropriate log level). Ensure only the print statements are removed/changed and
leave the Templates.CreateBuildTemplate() calls and checkBox:SetCheck(false,
true) behavior intact.
| for _, handler in ReUI.Construction.Panel.PrimaryHandlers do | ||
| if handler.tab == "selection" then | ||
| handler.actions[1] = "build_template" | ||
| end | ||
| end |
There was a problem hiding this comment.
Mutating first action of every selection-tab handler is fragile.
This loop blindly overwrites handler.actions[1] (previously "repeatBuild" in ReUI.Construction) to "build_template" for every selection-tab handler — including UpgradeChain and Selection defined in mods/ReUI.Construction/Construction.lua:862-898. If another mod inserts a selection handler that doesn't want build_template, or if ReUI.Construction reorders its actions list, this silently clobbers the wrong slot. Consider replacing by value (if handler.actions[i] == "repeatBuild" then ... end) or scoping the mutation to a named handler list, and append rather than overwrite if the intent is to add the action.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mods/ReUI.Construction.Templates/Templates.lua` around lines 539 - 543, The
loop mutates handler.actions[1] blindly; instead locate selection-tab handlers
in ReUI.Construction.Panel.PrimaryHandlers and change only actions that match
the original value (e.g., replace "repeatBuild" with "build_template") or append
"build_template" if not present; specifically update the loop around handler.tab
and handler.actions to check handler.actions entries for the string
"repeatBuild" (or test for absence of "build_template") and perform a targeted
replace or append, and avoid hard-indexing [1]; this ensures UpgradeChain and
Selection (and any external handlers) are not incorrectly clobbered.
1 participant
An extension mod for ReUI.Construction that adds templates tab into construction panel.
It also serves as example of how to create extension mods for ReUI.Construction.
Summary by CodeRabbit