Skip to content

Implement Structure.isActive() and ERR_RCL_NOT_ENOUGH checks - #110

Merged
laverdet merged 14 commits into
laverdet:mainfrom
misterwise:feature/is-active
Apr 3, 2026
Merged

Implement Structure.isActive() and ERR_RCL_NOT_ENOUGH checks#110
laverdet merged 14 commits into
laverdet:mainfrom
misterwise:feature/is-active

Conversation

@misterwise

@misterwise misterwise commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Implements Structure.isActive() with distance-based tiebreaking for excess structures on RCL downgrade
  • Adds checkIsActive to all structure intent chains (spawn, tower, lab, link, terminal, observer, extractor harvest)
  • Filters inactive spawns/extensions from energyAvailable, energyCapacityAvailable, energy drain, and spawn regen
  • Removes now-redundant isActive() overrides on StructureSpawn and StructureObserver
  • Checks input labs and link targets for isActive (stricter than official engine — inactive structures shouldn't participate in operations)
  • Closes [Bug] Not-active extensions still count towards energyCapacityAvailable #33

Verification

  • Excess spawns: exactly 1 active at RCL 1 with 2 spawns, closer to controller wins
  • Inactive spawn returns ERR_RCL_NOT_ENOUGH on spawnCreep, but destroy still works
  • energyAvailable excludes inactive extensions
  • Controller owner mismatch makes structures inactive
  • Observer inactive at RCL 7, active at RCL 8 (single-structure shortcut path)
  • Tower attack and mineral harvest return ERR_RCL_NOT_ENOUGH when structure inactive
  • Inactive spawns don't regenerate energy

🤖 Generated with Claude Code

@misterwise

Copy link
Copy Markdown
Contributor Author

Pushed an additional commit (misterwise/xxscreeps@806799c) that gates creep.transfer() and creep.withdraw() on isActive() — both now return ERR_RCL_NOT_ENOUGH for inactive structures (e.g. extensions after RCL downgrade).

This closes the remaining gap from #33: the prior commits fixed energyAvailable/energyCapacityAvailable and the spawn energy draw list, but direct transfer/withdraw intents against inactive structures were still succeeding.

Tests included and validated against unfixed code to confirm they catch the bug.

Comment thread src/mods/defense/test.ts Outdated

@laverdet laverdet left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

#active flag should be stored on relevant buildings.

Comment thread src/mods/chemistry/lab.ts Outdated
Comment thread src/mods/chemistry/lab.ts Outdated
Comment thread src/mods/defense/test.ts Outdated
Comment thread src/mods/logistics/link.ts Outdated
Comment thread src/mods/creep/creep.ts Outdated
Comment thread src/mods/structure/structure.ts Outdated
Comment thread src/mods/structure/structure.ts Outdated
Comment thread src/mods/structure/structure.ts Outdated
Comment thread src/mods/structure/structure.ts Outdated
Comment thread src/mods/creep/creep.ts Outdated
@laverdet

laverdet commented Apr 1, 2026

Copy link
Copy Markdown
Owner

Come to think of it the event would also need to run on destruction of other buildings. The point stands that this function should run in constant time.

@tiennou

tiennou commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Also, it might be worth considering something like this, which fixes the issue of isActive being a CPU sink in favor of precalculating and caching the active state before each tick in the engine itself.

@misterwise

misterwise commented Apr 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review — pushed a rework addressing all the feedback. I now understand the rationale for inactive buildings. They work as stores without needing to be active (withdraw, transfer), but the source labs for a reaction was the biggest surprised to me. All integratd in alignment with vanilla now.

Core architecture: cached #active flag

Reworked per your guidance. isActive() now returns this['#active'] ?? true in constant time. The computation moves to a batch checkActiveStructures(room) function (matching the approach in screeps/engine#150):

  • Single pass over #objects to group owned structures by type
  • Per type: if count ≤ maxCount, mark all active; if excess, sort by getRangeTo(controller) and mark closest N active
  • Stable sort handles the distance tiebreaker (oldest structure wins at equal range)
  • Called from updateRoomStatus on level/owner changes

For destruction: OwnedStructure.#destroy invalidates #active on same-type structures (sets to undefined). After the next #flushObjects removes the destroyed structure, the lazy fallback in isActive() recomputes with the correct count. No per-tick overhead.

Vanilla-correctness fixes

  • Removed checkIsActive on source labs (runReaction, reverseReaction) — only the output lab checks
  • Removed checkIsActive on link transfer target — only the sending link checks
  • Removed checkIsActive from creep transfer/withdraw — stores on inactive buildings still work
  • Tests updated: transfer/withdraw assert C.OK, renamed to "stores on inactive buildings still work"

Style/lint fixes

  • sspawn/structure for id-length
  • Multi-line #user assignment → single-line
  • Level-8 extensions in level-1 test rooms → level-1
  • Extension #roomStatusDidChange signature unchanged (batch handles #active separately)

@misterwise
misterwise requested a review from laverdet April 1, 2026 20:31

@laverdet laverdet left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Just some minor nitpicks for now.

There's some other things which would be better for later. I think what I want would require a more significant lifecycle hook adjustment so it's fine to put a pin in that.

Comment thread src/mods/structure/structure.ts Outdated
Comment thread src/mods/structure/structure.ts Outdated
Comment thread src/mods/structure/structure.ts Outdated
Comment thread src/mods/structure/structure.ts
Comment thread src/mods/structure/structure.ts
misterwise and others added 14 commits April 2, 2026 16:52
Replace the stubbed isActive() with a proper implementation that checks
CONTROLLER_STRUCTURES for the current RCL level, including excess-structure
handling for RCL downgrades (sorted by id, matching official engine).

Add checkIsActive guard to all structure action chains (tower, spawn, lab,
link, terminal, mineral extractor) immediately after ownership checks.
Filter inactive structures from energyAvailable/energyCapacityAvailable
and spawn energy consumption. Remove now-redundant isActive overrides
from observer and spawn.

7 new tests covering inactive structures, ERR_RCL_NOT_ENOUGH returns,
destroyability of inactive structures, and energy accounting.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Official engine does not validate extractor isActive during harvest —
ERR_RCL_NOT_ENOUGH is not a documented return code for Creep.harvest().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move the inline isActive check from checkObserveRoom into the
chainIntentChecks chain via checkIsActive, matching the consistent
pattern used by all other structures: checkMyStructure → checkIsActive →
method-specific checks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…controller owner check

- Use Chebyshev distance to controller (with ID tiebreaker) instead of
  ID-only sort when determining which excess structures remain active
- Return false when structure owner differs from controller owner
  (e.g. after room conquest)
- Add missing extractor isActive check to mineral harvest

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ests

- Use FIND_STRUCTURES filtered by structure owner instead of
  FIND_MY_STRUCTURES in isActive(), which was context-dependent on the
  calling player
- Match official check ordering: tower checks energy before RCL, lab
  runReaction/reverseReaction check cooldown before RCL
- Add tests for controller owner mismatch, distance-based tiebreaker,
  and extractor harvest with inactive extractor

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Skip the excess-structure scan for types that only ever allow one
(storage, terminal, observer, etc.) — if maxCount > 0 and max at
RCL 8 is 1, the structure is always active.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Drop redundant "active at sufficient RCL" test (covered by spawn
isActive tests). Clarify that the remaining test gives the tower
energy so ERR_RCL_NOT_ENOUGH specifically exercises the checkIsActive
placement in the intent chain.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… gate, test reorganization

- Split no-controller into separate `return true` guard matching official engine
- Add checkIsActive on input labs (runReaction, reverseReaction) and link target
- Gate spawn energy regen on isActive to prevent energyAvailable drift
- Remove unused checkIsActive import from mineral.ts
- Move generic isActive tests to structure/test.ts, observer tests stay in observer/test.ts
- Register structure mod test in manifest
- Rename spawn describe block and remove redundant assertion

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ccess

Closes the remaining gap from issue laverdet#33: creep.transfer() and
creep.withdraw() now return ERR_RCL_NOT_ENOUGH for inactive structures
(e.g. extensions after RCL downgrade).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add '#active' to OwnedStructure schema, computed by checkActiveStructures
- Batch approach matching official engine PR laverdet#150: group structures by type,
  sort each group by distance to controller, mark closest N active
- checkActiveStructures called from updateRoomStatus on level/owner changes
- OwnedStructure.#destroy invalidates same-type '#active' flags so the lazy
  fallback recomputes correctly after the destroyed structure is flushed
- isActive() returns cached value in constant time; lazy-computes on first
  access when unset (test setup, new construction)
- Remove non-vanilla isActive checks: source labs, link targets,
  transfer, withdraw — stores on inactive buildings still work
- Fix id-length, line breaks, level-8 extensions in level-1 test rooms

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…dundant ternary, add TODOs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@misterwise
misterwise requested a review from laverdet April 2, 2026 23:01
@laverdet
laverdet merged commit 5c59ff7 into laverdet:main Apr 3, 2026
1 check passed
@misterwise
misterwise deleted the feature/is-active branch April 22, 2026 05:25
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.

[Bug] Not-active extensions still count towards energyCapacityAvailable

3 participants