Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate tick count to ensure vanilla parity #12077

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

HaHaWTH
Copy link
Contributor

@HaHaWTH HaHaWTH commented Feb 7, 2025

Closes #12075.

This pull request introduces another entity tick counter totalTickCount for api usage, reset vanilla tickCount instead of restoring value from Spigot's Spigot.ticksLived tag to keep vanilla parity.

Analysis

Spigot's restore tickCount behavior(See original patch) causes potential issues with Wither's ai, see code below, and lead to wither loses its target when entities are reloaded:
image

Api Changes

Now Entity#getTicksLived gets value from totalTickCount to avoid api behavior breakage, Entity#setTicksLived sets both tickCount and totalTickCount.

Currently the totalTickCount is only used for apis, if the breakage is fine, the extra counter can be removed.

@HaHaWTH HaHaWTH requested a review from a team as a code owner February 7, 2025 18:39
@lynxplay lynxplay force-pushed the fix/entity-behavior-parity branch from 9254a91 to f28a08b Compare February 9, 2025 12:12
@Warriorrrr Warriorrrr added the type: bug Something doesn't work as it was intended to. label Feb 10, 2025
Copy link
Member

@Warriorrrr Warriorrrr left a comment

Choose a reason for hiding this comment

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

In the despawn time check inside Entity#tick, totalEntityAge should also be used instead if we don't want unloading to affect their configured despawn time.

@@ -860,7 +861,7 @@
+ // CraftBukkit start
+ // Spigot start
+ if (this instanceof net.minecraft.world.entity.LivingEntity) {
+ this.tickCount = compound.getInt("Spigot.ticksLived");
+ this.totalEntityAge = compound.getInt("Spigot.ticksLived"); // Paper
Copy link
Member

@Warriorrrr Warriorrrr Feb 10, 2025

Choose a reason for hiding this comment

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

Kinda unrelated to this PR but, is there still a reason to only read this if the entity is a living entity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Entity despawn now uses the totalEntityAge

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kinda unrelated to this PR but, is there still a reason to only read this if the entity is a living entity?

Spigot api resets tickCount for non-living entities, so leaving it here just for api behavior is same as spigot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something doesn't work as it was intended to.
Projects
Status: Awaiting final testing
Development

Successfully merging this pull request may close these issues.

The Wither's mob AI behavior is inconsistent with the vanilla Minecraft.
5 participants