Skip to content

fix: citizen AI and combat improvements#11641

Draft
ZeOs360 wants to merge 2 commits intoldtteam:version/mainfrom
ZeOs360:fix/perf-safety-audit
Draft

fix: citizen AI and combat improvements#11641
ZeOs360 wants to merge 2 commits intoldtteam:version/mainfrom
ZeOs360:fix/perf-safety-audit

Conversation

@ZeOs360
Copy link
Copy Markdown

@ZeOs360 ZeOs360 commented Apr 30, 2026

getCapability - LazyOptional.of() was being called on every invocation. It has been cached as a field and is now invalidated in setRemoved().
callForHelp - this.blockPosition() was being called on every comparison in the sort comparator. It is now pre-fetched as selfPos.
checkIfValidDamageSource - Although addPlayer() is server-side, it was also running on the client side. !level.isClientSide has been added.
BasicStateMachine - DateTimeFormatter was being created separately for each instance. It has been made static final.

Closes #
Closes #

Changes proposed in this pull request

Review please

ThreatTable - dead entities were remaining in the list without an isAlive() check, causing memory leaks and index shifting during combat. removeIf and reference-based index recovery have been added to addThreat().
getCapability - LazyOptional.of() was being called on every invocation. It has been cached as a field and is now invalidated in setRemoved().
callForHelp - this.blockPosition() was being called on every comparison in the sort comparator. It is now pre-fetched as selfPos.
checkIfValidDamageSource - Although addPlayer() is server-side, it was also running on the client side. !level.isClientSide has been added.
BasicStateMachine - DateTimeFormatter was being created separately for each instance. It has been made static final.
@github-actions
Copy link
Copy Markdown

In order for this pull request to be merged, make sure you test whether your changes work.

If the changes are working as intended, remove the https://github.com/ldtteam/minecolonies/labels/undefined label from the pull request.
As long as this label is on the pull request, it will not be merged.
If your pull request made no changes to the source code, the label will not be automatically added to the pull request.

Contributors, please review this pull request!

@ZeOs360 ZeOs360 marked this pull request as draft April 30, 2026 23:54
@Raycoms
Copy link
Copy Markdown
Contributor

Raycoms commented May 1, 2026

Did you use AI to aid you in this PR?

final ThreatTableEntry currentTarget = (currentTargetIndex >= 0 && currentTargetIndex < threatList.size())
? threatList.get(currentTargetIndex) : null;
threatList.removeIf(entry -> !entry.getEntity().isAlive());
currentTargetIndex = currentTarget != null ? Math.max(0, threatList.indexOf(currentTarget)) : 0;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it is intentionally not checking the full list on threat being added as threat added occurs very frequently, instead sanity checks like is alive are done when retrieving a new target.

Copy link
Copy Markdown
Author

@ZeOs360 ZeOs360 May 2, 2026

Choose a reason for hiding this comment

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

thanks for the context I'll revert the changes.

@ZeOs360
Copy link
Copy Markdown
Author

ZeOs360 commented May 2, 2026

Did you use AI to aid you in this PR?

I used AI for analysis. The ThreatTable change was a mistake I didn't verify it against the existing design intent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants