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

Stopping a Goal can create an error #11962

Open
ShaneBeee opened this issue Jan 12, 2025 · 3 comments
Open

Stopping a Goal can create an error #11962

ShaneBeee opened this issue Jan 12, 2025 · 3 comments

Comments

@ShaneBeee
Copy link
Contributor

Expected behavior

No errors and no vanishing piggy would be nice 😊

Observed/Actual behavior

I recently had an issue where stopping a goal of an entity, cause a stack trace in the console (and then the entity vanished).
The goal specifically was the FollowParentGoal.

The error states that this.parent is null.
After doing some digging I came to the realization that the PaperVanillaGoal had wrapped the goal itself, and not the (NMS)WrappedGoal.
Issue here is WrappedGoal#stop stops the goal from running and also stops the goal itself.
But Paper isn't returning the WrappedGoal therefor the WrappedGoal still thinks its running.
As seen here (in PaperMobGoals):
Image

Inside FollowParentGoal:
Image

Inside WrappedGoal:
(This is what we're missing access to)
Image

When Minecraft attempts to tick the goal, it first checks if it's running.
Since the WrappedGoal isn't stopped properly, it'll continue thinking its running, and therefor error.

Suggested Change:
(In PaperMobGoals return the WrappedGoal instead of the goal itself directly)

- goals.add(item.getGoal().asPaperVanillaGoal());
+ goals.add(item.asPaperVanillaGoal());

The error in question:

[12:49:13 ERROR]: Entity threw exception at world:-763.4517434552514,67.0,57948.012728867085
java.lang.NullPointerException: Cannot invoke "net.minecraft.world.entity.Entity.blockPosition()" because "entity" is null
	at net.minecraft.world.entity.ai.navigation.GroundPathNavigation.createPath(GroundPathNavigation.java:85) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.entity.ai.navigation.PathNavigation.moveTo(PathNavigation.java:228) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.entity.ai.goal.FollowParentGoal.tick(FollowParentGoal.java:80) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.entity.ai.goal.WrappedGoal.tick(WrappedGoal.java:63) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.entity.ai.goal.GoalSelector.tickRunningGoals(GoalSelector.java:128) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.entity.ai.goal.GoalSelector.tick(GoalSelector.java:119) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.entity.Mob.serverAiStep(Mob.java:874) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.entity.LivingEntity.aiStep(LivingEntity.java:3396) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.entity.Mob.aiStep(Mob.java:616) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.entity.AgeableMob.aiStep(AgeableMob.java:148) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.entity.animal.Animal.aiStep(Animal.java:65) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.entity.LivingEntity.tick(LivingEntity.java:3148) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.entity.Mob.tick(Mob.java:398) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.server.level.ServerLevel.tickNonPassenger(ServerLevel.java:1269) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.level.Level.guardEntityTick(Level.java:1499) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.server.level.ServerLevel.lambda$tick$4(ServerLevel.java:817) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.world.level.entity.EntityTickList.forEach(EntityTickList.java:39) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.server.level.ServerLevel.tick(ServerLevel.java:799) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.server.MinecraftServer.tickChildren(MinecraftServer.java:1724) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.server.MinecraftServer.tickServer(MinecraftServer.java:1529) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1251) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at net.minecraft.server.MinecraftServer.lambda$spin$2(MinecraftServer.java:310) ~[paper-1.21.4.jar:1.21.4-100-86c6308]
	at java.base/java.lang.Thread.run(Thread.java:1570) ~[?:?]

Steps/models to reproduce

Just a little something:

private void stopPigFollowingParent(Mob mob) {
    if (mob instanceof Animals animal && !animal.isAdult()) {
        Goal<Animals> goal = Bukkit.getMobGoals().getGoal(animal, VanillaGoal.FOLLOW_PARENT);
        if (goal != null)
            goal.stop();
    }
}

Stop the goal of a baby pig following an adult pig.
You should see an error in the console and the pig will most likely vanish.

Plugin and Datapack List

[12:59:52 INFO]: Server Plugins (5):
[12:59:52 INFO]: Paper Plugins:
[12:59:52 INFO]:  - CorePlugin
[12:59:52 INFO]: Bukkit Plugins:
[12:59:52 INFO]:  - BeeConomy, PermissionsEx, StressTestBots(not enabled), Vault
> datapack list
[13:00:11 INFO]: There are 3 data pack(s) enabled: [vanilla (built-in)], [file/bukkit (world)], [paper (built-in)]
[13:00:11 INFO]: There are no more data packs available

Paper version

running Paper version 1.21.4-100-main@86c6308 (2025-01-12T17:10:40Z) (Implementing API version 1.21.4-R0.1-SNAPSHOT)

Other

No response

@Lulu13022002 Lulu13022002 added status: accepted Disputed bug is accepted as valid or Feature accepted as desired to be added. version: 1.21.4 and removed status: needs triage labels Jan 14, 2025
@Lulu13022002
Copy link
Contributor

Can replicate but the suggested change can't work directly otherwise getGoal will always return null since it will fetch from the wrong key (minecraft:wrapped)

@ShaneBeee
Copy link
Contributor Author

Can replicate but the suggested change can't work directly otherwise getGoal will always return null since it will fetch from the wrong key (minecraft:wrapped)

I did a test changing it (stupid me didn't keep my changes), but basically I think what I did is within the PaperVanillaGoal class, when getting the key from the Goal, I just checked if it as instance of wrapped goal, grabbed the inner goal to get the key from that.

I'll look into maybe doing a PR soon, and doing a bunch of testing.

@Lulu13022002
Copy link
Contributor

Hmm it's possibly a worked as intended if those methods shouldn't be called by the consumer (just callback) and they follow vanilla there, the goal will just first check canUse and then canContinueUse and the game will call stop on the goal itself without your help.

@Lulu13022002 Lulu13022002 added status: needs triage and removed status: accepted Disputed bug is accepted as valid or Feature accepted as desired to be added. labels Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants