Skip to content

Comments

sim Unit:ForceReclaim(bool)#130

Open
Strogoo wants to merge 1 commit intoFAForever:masterfrom
Strogoo:forceReclaim
Open

sim Unit:ForceReclaim(bool)#130
Strogoo wants to merge 1 commit intoFAForever:masterfrom
Strogoo:forceReclaim

Conversation

@Strogoo
Copy link

@Strogoo Strogoo commented Jan 23, 2026

Added Unit:ForceReclaim(bool)

When enabled engineer will continue reclaiming even if storage is full.

Summary by CodeRabbit

  • New Features

    • Added a Lua-accessible Force Reclaim action for units.
    • Improved unit construction initialization to propagate new unit flags.
  • Behaviour Changes

    • Entity footprint handling now bypasses the previous alternate-footprint path.
  • Chores

    • Added underlying entity variable support to store new unit flags.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

Warning

Rate limit exceeded

@Strogoo has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 16 minutes and 32 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

Adds assembly hook snippets and two new C++ sources: one initializes unit boolean flags during construction; the other exposes a Lua binding to set a unit's isForceReclaim flag and an assembly hook that checks that flag and clears storage-full indicators before resuming original execution.

Changes

Cohort / File(s) Summary
Assembly hooks
hooks/MohoUnitConstructor.hook, hooks/UnitForceReclaim.hook, hooks/MohoEntityVariableData.hook, hooks/EntityGetFootprint.hook
New and modified hook snippets: insert jumps to asm__UnitConstructor and asm__IsForceReclaim, add Moho::SSTIEntityVariableData assignment (mov [eax+0x0A4], ecx documenting boolean offsets), and remove prior SSTI block that set forceAltFootprint.
C++ unit logic & Lua binding
section/MohoUnitConstructor.cpp, section/UnitForceReclaim.cpp
New sources: asm__UnitConstructor() writes flag bytes for new units and jumps back; asm__IsForceReclaim() reads isForceReclaim and clears energy/mass storage-full bits conditionally. Adds ForceReclaim(lua_State *L) binding, UnitMethodReg ForceReclaimReg, and a type alias UnitMethodReg.

Sequence Diagram

sequenceDiagram
    participant Lua as "Lua"
    participant Cpp as "ForceReclaim()"
    participant Memory as "Unit Memory"
    participant Asm as "asm__IsForceReclaim"

    Lua->>Cpp: Call ForceReclaim(unit, flag)
    Cpp->>Cpp: validate args, resolve Unit pointer
    Cpp->>Memory: write boolean -> unit + 0x11E (isForceReclaim)
    Note over Memory,Asm: Later execution hits hooked address
    Memory-->>Asm: asm__IsForceReclaim invoked
    Asm->>Memory: read isForceReclaim flag
    alt flag == true
        Asm->>Memory: clear energy/mass storage-full bits
    end
    Asm->>Asm: jump to original/default code path
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hop in code, I nudge a bit,

A flag set true with a tiny split.
Assembly hums, the unit wakes—
I tidy stores for safety's sakes.
🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete. It lacks the required sections from the template: guides confirmation, variable naming verification, header file updates documentation, README updates, and test instructions. Complete the checklist items: confirm guides reading, verify naming conventions, document moho.h/global.h/Info.txt changes, update README.md with change description, and provide test hints.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'sim Unit:ForceReclaim(bool)' is specific and directly related to the main change—adding a ForceReclaim method to the Unit class.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@section/MohoUnitConstructor.cpp`:
- Around line 2-10: asm__UnitConstructor is currently compiled with a normal
prologue/epilogue so its use of ebp+0x687/0x688 targets the wrong stack slot;
mark the function as naked and C-linked to preserve caller frame layout (e.g.
declare with extern "C" and __attribute__((naked))) and change the inline
assembly to use __asm__ __volatile__ with the same instructions so the compiler
does not emit prologue/epilogue code; apply the same change to the analogous
function in UnitForceReclaim.cpp.
🧹 Nitpick comments (1)
section/UnitForceReclaim.cpp (1)

25-36: Extract the 0x687 field offset into a named constant.

Keeping the offset as a magic number makes future layout changes easy to miss. A shared constant (even if only used on the C++ side) improves auditability.

Suggested refactor
+#if !defined(__UNIT_FORCE_RECLAIM_OFFSETS__)
+#define __UNIT_FORCE_RECLAIM_OFFSETS__
+namespace {
+constexpr int kUnitForceReclaimOffset = 0x687;
+}
+#endif
@@
-    GetField<bool>(unit, 0x687) = flag;
+    GetField<bool>(unit, kUnitForceReclaimOffset) = flag;

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@section/UnitForceReclaim.cpp`:
- Around line 6-22: Mark the naked assembly hook functions so the compiler won’t
emit prologue/epilogue: add __attribute__((naked)) to asm__IsForceReclaim (and
apply the same change to other hook functions asm__UnitConstructor and
asm__GetFootprint) so they don't generate push ebp/mov ebp,esp; also standardize
redundant segment prefixes in the inline asm (remove or make consistent between
ds: and ss: for stack/heap accesses) to avoid confusion while keeping the same
memory operands and the final jmp target intact.
🧹 Nitpick comments (1)
section/UnitForceReclaim.cpp (1)

42-46: Consider adding documentation for the Lua binding.

The registration has an empty documentation string. A brief description would help modders understand the function's purpose.

📝 Suggested documentation
 UnitMethodReg ForceReclaimReg{
     "ForceReclaim",
-    "",
+    "Unit:ForceReclaim(bool) - When enabled, engineer continues reclaiming even if storage is full",
     ForceReclaim,
     "Unit"};

Addded Unit:ForceReclaim(bool)

When enabled engineer will continue reclaiming even if storage is full.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant