Add temporary template feature - #7185
Conversation
improve organization Use simple alignment offset instead of centering offset since centering is done by the skirt rect min/max logic, and I was only using bp center offset because I misunderstood the problems I was having
lL1l1
added
area: ui
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughAdds temporary build-template persistence, alignment, validation, factory integration, and a new advanced-order hotkey that creates templates from selected structures or reuses saved templates in build or cheat spawn modes. ChangesTemporary build-template workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Player
participant Keymap
participant TemporaryTemplates
participant TemplateUtils
participant BuildMode
Player->>Keymap: Press temporary template hotkey
Keymap->>TemporaryTemplates: UseOrCreateTemporaryTemplate()
TemporaryTemplates->>BuildMode: Read selection
TemporaryTemplates->>TemplateUtils: Center and validate template
TemplateUtils-->>TemporaryTemplates: Validated template
TemporaryTemplates->>BuildMode: Start build or cheat spawn mode
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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: 2
🧹 Nitpick comments (4)
engine/User.lua (1)
127-138: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePosition fields typed
integerbut can hold.5offsets.
templateUtils.lua's alignment helpers (GetTemplateAlignmentAxisOffsetForBp,CenterTemplateForBuildModeBp,OffsetTemplate) add0or0.5offsets directly ontobuildData[3]/buildData[4](template building x/z). WithUIBuildTemplateBuilding.[3]/[4]typed asinteger, the type checker will report these as always-integer even though runtime values can bex.5.📝 Proposed fix
---@class UIBuildTemplateBuilding ---@field [1] UnitId ---@field [2] integer # build order ----@field [3] integer # position x ----@field [4] integer # position z +---@field [3] number # position x +---@field [4] number # position z🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@engine/User.lua` around lines 127 - 138, Update the UIBuildTemplateBuilding type definition so fields [3] and [4] use a numeric type that permits fractional values such as x.5, matching the offsets applied by GetTemplateAlignmentAxisOffsetForBp, CenterTemplateForBuildModeBp, and OffsetTemplate. Leave the build order field typed as integer.lua/ui/templates_factory.lua (2)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
SetTemplateKey/ClearTemplateKeyare duplicated verbatim across the two template files. This PR already consolidated other template logic (alignment, centering, verification) intotemplateUtils.lua, but the key-management functions were left duplicated.
lua/ui/templates_factory.lua#L110-139: move the uniqueness-check/set/clear logic into a sharedtemplateUtils.luahelper (e.g.TemplateUtils.SetTemplateKey(templates, templateID, key)/ClearTemplateKey) and have this file's wrapper call it, persisting tobuild_templates_factory.lua/ui/game/build_templates.lua#L98-119: same — call the shared helper and persist tobuild_templates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lua/ui/templates_factory.lua` at line 1, Move the duplicated uniqueness, set, and clear logic from SetTemplateKey and ClearTemplateKey in both template files into shared helpers in templateUtils.lua. Update each file’s wrappers to delegate to TemplateUtils while persisting to its respective build_templates_factory or build_templates store, preserving existing behavior.
6-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
UIBuildTemplateDatatype is missingkeyand doesn't reflect the'default'icon sentinel.
SetTemplateKey/ClearTemplateKey(lines 116-139) read/writetemplate.key, but the class has nokeyfield. AlsoGetInitialIconcan return the literal'default'(line 43), buticonis typed as plainUnitId—build_templates.lua's equivalent function correctly types this as'default' | UnitId.📝 Proposed fix
---@class UIBuildTemplateData ---@field templateData UIBuildTemplate ---@field name string ----@field icon UnitId +---@field icon 'default' | UnitId +---@field key? Keycode🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lua/ui/templates_factory.lua` around lines 6 - 12, Update the UIBuildTemplateData annotation to declare the optional key field used by SetTemplateKey and ClearTemplateKey, and change icon’s type to accept both the literal 'default' sentinel and UnitId, matching GetInitialIcon and the equivalent build_templates.lua typing.lua/ui/templateUtils.lua (1)
166-191: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMagic sentinel values for min/max bounds.
10000/-10000are used as sentinel initial bounds for the skirt-rect min/max scan.math.huge/-math.hugewould be more robust against edge cases on very large maps/templates and self-documenting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lua/ui/templateUtils.lua` around lines 166 - 191, Update the initial x0Min, z0Min, x1Max, and z1Max bounds in the skirt-rectangle scan to use math.huge and -math.huge instead of fixed sentinel values. Preserve the existing aggregation logic in the template-offset loop and center calculation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lua/ui/game/hotkeys/temporary-templates.lua`:
- Line 7: Validate the persisted value assigned to TempTemplate by routing it
through TemplateUtils.VerifyTemplate during initialization, matching the
verification already performed by SaveTemporaryTemplate. Ensure invalid or stale
preferences do not remain available to TryBuildTemporaryTemplateForUnits, while
preserving valid restored templates.
In `@lua/ui/templates_factory.lua`:
- Line 1: Hoist every inline import identified in the review into top-level
locals: add the construction import in templates_factory.lua and replace its
inline RefreshUI call with the local; add the construction, announcement, and
gamemain imports in build_templates.lua for ReceiveTemplate/Init; and add the
Exit import in temporary-templates.lua, using that local for AddOnExitCallback.
---
Nitpick comments:
In `@engine/User.lua`:
- Around line 127-138: Update the UIBuildTemplateBuilding type definition so
fields [3] and [4] use a numeric type that permits fractional values such as
x.5, matching the offsets applied by GetTemplateAlignmentAxisOffsetForBp,
CenterTemplateForBuildModeBp, and OffsetTemplate. Leave the build order field
typed as integer.
In `@lua/ui/templates_factory.lua`:
- Line 1: Move the duplicated uniqueness, set, and clear logic from
SetTemplateKey and ClearTemplateKey in both template files into shared helpers
in templateUtils.lua. Update each file’s wrappers to delegate to TemplateUtils
while persisting to its respective build_templates_factory or build_templates
store, preserving existing behavior.
- Around line 6-12: Update the UIBuildTemplateData annotation to declare the
optional key field used by SetTemplateKey and ClearTemplateKey, and change
icon’s type to accept both the literal 'default' sentinel and UnitId, matching
GetInitialIcon and the equivalent build_templates.lua typing.
In `@lua/ui/templateUtils.lua`:
- Around line 166-191: Update the initial x0Min, z0Min, x1Max, and z1Max bounds
in the skirt-rectangle scan to use math.huge and -math.huge instead of fixed
sentinel values. Preserve the existing aggregation logic in the template-offset
loop and center calculation.
🪄 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: ff4286cf-44b1-475b-8433-0fe351ab91b4
📒 Files selected for processing (12)
changelog/snippets/features.7185.mdengine/Core.luaengine/Core/Blueprints/EntityBlueprint.luaengine/Core/Blueprints/UnitBlueprint.luaengine/User.lualua/keymap/hotbuild.lualua/keymap/keyactions.lualua/keymap/keydescriptions.lualua/ui/game/build_templates.lualua/ui/game/hotkeys/temporary-templates.lualua/ui/templateUtils.lualua/ui/templates_factory.lua
|
|
||
| local cheatsEnabled = SessionGetScenarioInfo().Options.CheatsEnabled | ||
| ---@type UIBuildTemplate | nil | ||
| TempTemplate = Prefs.GetFromCurrentProfile('build_template_temporary') |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
TempTemplate loaded from prefs is never verified.
Unlike templates set via SaveTemporaryTemplate (which runs TemplateUtils.VerifyTemplate), the value restored from Prefs.GetFromCurrentProfile('build_template_temporary') at load time is used as-is. A stale/corrupted persisted template (e.g. from an older format) could later reach TryBuildTemporaryTemplateForUnits unverified.
🛡️ Proposed fix
TempTemplate = Prefs.GetFromCurrentProfile('build_template_temporary')
+if TempTemplate and not TemplateUtils.VerifyTemplate(TempTemplate) then
+ TempTemplate = nil
+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.
| TempTemplate = Prefs.GetFromCurrentProfile('build_template_temporary') | |
| TempTemplate = Prefs.GetFromCurrentProfile('build_template_temporary') | |
| if TempTemplate and not TemplateUtils.VerifyTemplate(TempTemplate) then | |
| TempTemplate = nil | |
| end |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lua/ui/game/hotkeys/temporary-templates.lua` at line 7, Validate the
persisted value assigned to TempTemplate by routing it through
TemplateUtils.VerifyTemplate during initialization, matching the verification
already performed by SaveTemporaryTemplate. Ensure invalid or stale preferences
do not remain available to TryBuildTemporaryTemplateForUnits, while preserving
valid restored templates.
| ---@field [2] number # height that is used when drag building the template | ||
| ---@field [3] UIBuildTemplateBuilding | ||
| --@field [...] UIBuildTemplateBuilding | ||
| local Prefs = import("/lua/user/prefs.lua") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Inline import() calls scattered through function bodies instead of top-of-file. All three sites call import(...) from inside a function (or at the tail of the module) rather than assigning a top-of-file local, obscuring the file's actual dependencies.
lua/ui/templates_factory.lua#L54-65: hoistimport("/lua/ui/game/construction.lua")to a top-level local (e.g.local Construction = import(...)) and callConstruction.RefreshUI()instead of the inline call at line 61.lua/ui/game/build_templates.lua#L122-137: hoist theconstruction.lua,announcement.lua, andgamemain.luaimports used inReceiveTemplate/Initto top-of-file locals.lua/ui/game/hotkeys/temporary-templates.lua#L97-100: hoistimport('/lua/ui/override/Exit.lua')to a top-of-file local alongside the existingPrefs/CM/TemplateUtilsimports, then call.AddOnExitCallback(...)on that local.
Based on path instructions for lua/ui/**/*.lua: "Import models and controllers at the top of any file that needs them — never thread them through constructors or callback tables. Direct imports keep dependencies visible at the top of the file."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lua/ui/templates_factory.lua` at line 1, Hoist every inline import identified
in the review into top-level locals: add the construction import in
templates_factory.lua and replace its inline RefreshUI call with the local; add
the construction, announcement, and gamemain imports in build_templates.lua for
ReceiveTemplate/Init; and add the Exit import in temporary-templates.lua, using
that local for AddOnExitCallback.
Source: Path instructions
2 participants
Description of the proposed changes
Testing done on the proposed changes
Bind the hotkey
Spawn some SMD (even footprint size, odd skirt size structure), T3 power generators (odd footprint size, even skirt size), and walls (odd footprint and skirt sizes) at various distances from each other.
Press the hotkey without selection or saved template.
Select the units and press the hotkey to save them.
Press with no selection to cheat spawn units.
Press with engineers selected to build the template. There is a message instead if the engineers cannot build the template.
Checklist
Summary by CodeRabbit
New Features
Improvements