Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions lua/SimCallbacks.lua
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,38 @@ do

end

do

---@param data { Enable: boolean, ShowMsg: boolean }
---@param selection Unit[]
Callbacks.ForceReclaim = function(data, selection)
-- verify selection
selection = SecureUnits(selection)
if (not selection) or TableEmpty(selection) then
return
end

-- verify we have engineers
local engineers = EntityCategoryFilterDown(categories.ENGINEER, selection)
if TableEmpty(engineers) then
return
end

for k, unit in engineers do
unit:ForceReclaim(data.Enable)
end

if data.ShowMsg then
if data.Enable == true then
print(string.format("Force reclaim ENABLED for %d engineers", table.getn(engineers)))
else
print(string.format("Force reclaim DISABLED for %d engineers", table.getn(engineers)))
end
end
end
end
Comment on lines +829 to +858
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for ForceReclaim references in the codebase
rg -i "forcereclaim" --type lua -B 2 -A 2

Repository: FAForever/fa

Length of output: 2057


🏁 Script executed:

#!/bin/bash
# Check for similar patterns where methods are called on units in SimCallbacks
rg "unit:" lua/SimCallbacks.lua -A 1 | head -50

Repository: FAForever/fa

Length of output: 582


🏁 Script executed:

#!/bin/bash
# Look for defensive checks or error handling patterns in SimCallbacks
rg "and.*\." lua/SimCallbacks.lua | head -20

Repository: FAForever/fa

Length of output: 1653


🌐 Web query:

FAForever FA-Binary-Patches PR 130 ForceReclaim

💡 Result:

FAForever FA-Binary-Patches PR #130 is titled “sim Unit:ForceReclaim(bool)” and is currently Open (opened Jan 23, 2026) by Strogoo. [1]

What it does:

  • Adds a new Lua-exposed unit API: Unit:ForceReclaim(bool).
  • Purpose: when enabled, engineers continue reclaiming even if storage is full. [2]

Implementation notes (high level):

  • Introduces new assembly hooks and C++ support files to store/check a per-unit “force reclaim” flag and expose it to Lua. [2]

CI / review status:

  • CodeRabbit shows 1 passed check (title), and 2 warnings (missing template sections in description; docstring coverage below threshold). [2]

Sources: PR list page [1], PR #130 conversation/details [2].


🏁 Script executed:

#!/bin/bash
# Check if there are other guards for patch-dependent features in similar callbacks
rg "WARN|error|if not.*then.*return" lua/SimCallbacks.lua -B 2 -A 2 | head -40

Repository: FAForever/fa

Length of output: 1403


🏁 Script executed:

#!/bin/bash
# Search for any documentation or comments about patch dependencies in the codebase
rg -i "patch|binary|binary-patch|forcereclaim" lua/ --type lua | grep -i "comment\|--\|doc"

Repository: FAForever/fa

Length of output: 6192


Guard against missing ForceReclaim method to prevent sim crash.

unit:ForceReclaim() will cause a hard Lua error if the engine patch isn't present, since the method will be nil. This is a real risk given that FA-Binary-Patches PR #130 (which adds this method) is currently open and not yet deployed. Add a defensive check before the loop to prevent crash on mismatched deployments.

Suggested fix
+        if not (engineers[1] and engineers[1].ForceReclaim) then
+            WARN("Force reclaim is unavailable. Verify the FA-Binary-Patches update is deployed.")
+            return
+        end
+
         for k, unit in engineers do
             unit:ForceReclaim(data.Enable)
         end
🤖 Prompt for AI Agents
In `@lua/SimCallbacks.lua` around lines 829 - 858, The loop in
Callbacks.ForceReclaim calls unit:ForceReclaim which can be nil on deployments
without the engine patch; update the function to guard before invoking the
method by checking the method exists on each unit (or on the prototype) and only
calling it when present—e.g., verify unit.ForceReclaim (or check the first
engineer) before the for k, unit in engineers loop and skip units lacking the
method (optionally log or count skipped units) to avoid a hard Lua error when
ForceReclaim is unavailable.



--#endregion


Expand Down
18 changes: 17 additions & 1 deletion lua/keymap/keyactions.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1653,7 +1653,23 @@ local keyActionsOrdersAdvanced = {
['shift_discharge'] = {
action = 'UI_Lua import("/lua/keymap/misckeyactions.lua").DischargeShields()',
category = 'ordersAdvanced',
}
},
['enable_force_reclaim'] = {
action = 'UI_Lua import("/lua/keymap/misckeyactions.lua").SetForceReclaim(true)',
category = 'ordersAdvanced',
},
['shift_enable_force_reclaim'] = {
action = 'UI_Lua import("/lua/keymap/misckeyactions.lua").SetForceReclaim(true)',
category = 'ordersAdvanced',
},
['disable_force_reclaim'] = {
action = 'UI_Lua import("/lua/keymap/misckeyactions.lua").SetForceReclaim(false)',
category = 'ordersAdvanced',
},
['shift_disable_force_reclaim'] = {
action = 'UI_Lua import("/lua/keymap/misckeyactions.lua").SetForceReclaim(false)',
category = 'ordersAdvanced',
},
}

local keyActionsOrdersQueueBased = {
Expand Down
5 changes: 5 additions & 0 deletions lua/keymap/keydescriptions.lua
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,11 @@ keyDescriptions = {
['shift_load_transports_clear'] = '<LOC key_desc_shift_load_transports_clear>Load into transports. Applies immediately',
['copy_orders'] = '<LOC key_desc_copy_orders>Copy orders of the unit the mouse is on top of',
['shift_copy_orders'] = '<LOC key_desc_shift_copy_orders>Copy orders of the unit the mouse is on top of',

['enable_force_reclaim'] = '<LOC key_desc_enable_force_reclaim>Enable "Force reclaim" for selected units.',
['disable_force_reclaim'] = '<LOC key_desc_disable_force_reclaim>Disable "Force reclaim" for selected units.',
['shift_enable_force_reclaim'] = '<LOC key_desc_shift_enable_force_reclaim>Enable "Force reclaim" for selected units.',
['shift_disable_force_reclaim'] = '<LOC key_desc_shift_disable_force_reclaim>Disable "Force reclaim" for selected units',

['select_surface_bombers'] = '<LOC key_desc_0407>Select all Bombers (Normal)',
['select_torpedo_bombers'] = '<LOC key_desc_0408>Select all Bombers (Torpedo)',
Expand Down
4 changes: 4 additions & 0 deletions lua/keymap/misckeyactions.lua
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,10 @@ AIPlatoonSimpleStructureBehavior = function()
SimCallback({ Func = 'AIPlatoonSimpleStructureBehavior', Args = {} }, true)
end

function SetForceReclaim(enable, showMsg)
SimCallback({ Func = 'ForceReclaim', Args = { Enable = enable, ShowMsg = showMsg or true} }, true)
end
Comment on lines +560 to +562
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

showMsg default logic always forces true.

Passing false is ignored because showMsg or true always evaluates to true. If silent mode is expected, this breaks it.

✅ Proposed fix
 function SetForceReclaim(enable, showMsg)
-    SimCallback({ Func = 'ForceReclaim', Args = { Enable = enable, ShowMsg = showMsg or true} }, true)
+    local show = (showMsg ~= false)
+    SimCallback({ Func = 'ForceReclaim', Args = { Enable = enable == true, ShowMsg = show } }, true)
 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.

Suggested change
function SetForceReclaim(enable, showMsg)
SimCallback({ Func = 'ForceReclaim', Args = { Enable = enable, ShowMsg = showMsg or true} }, true)
end
function SetForceReclaim(enable, showMsg)
local show = (showMsg ~= false)
SimCallback({ Func = 'ForceReclaim', Args = { Enable = enable == true, ShowMsg = show } }, true)
end
🤖 Prompt for AI Agents
In `@lua/keymap/misckeyactions.lua` around lines 560 - 562, The SetForceReclaim
function always forces showMsg to true because it uses "showMsg or true"; change
the defaulting so only nil/undefined becomes true and explicit false is
preserved—modify the call around SimCallback in SetForceReclaim to pass ShowMsg
= (showMsg == nil) and true or showMsg (or equivalent nil-coalescing logic) so
that passing false results in ShowMsg = false while nil still yields true;
ensure you update the symbol SetForceReclaim and its use of SimCallback
accordingly.


StoreCameraPosition = function()
local camera = GetCamera('WorldCamera')
local settings = camera:SaveSettings()
Expand Down
Loading