Some entities randomly stop attacking or fleeing during pathfinding

This relates to MC-159299.

Can you upload the video on YouTube as unlisted then post the link her

I've already posted the link in the description. Its too big so it is in the description. Nevermind, trimmed a video.

According to Adrian on MC-147516 it's probably just MC-2310

This is NOT MC-2310. That bug talks about mobs hitting through blocks whilst this talks about mobs stopping pathfinding rnadomly.

But it can be caused by the "wrong attack radius calculation"

Okay, I guess you are right... I will be having this resolved as a duplicate of MC-2310.

Please don't add the "duplicate" label

Sorry. I didn’t think MC-2310 caused this though as that talks about mobs hitting through blocks.

In MC-147516 he also says that it could be caused by different issue, and MC-2310's root cause is told to be the set radius (and not accounting for the blocks), which is irrelevant to this issue

So no, not a duplicate of MC-2310

Is anybody able to reproduce? Sorry for the terrible description beforehand.

I can confirm for 1.16.2

villagers... can't... attack...

i can confirm for silverfish

Requesting ownership as the current owner has been inactive since late 2015

Can confirm in 21w03a.

Can confirm in 21w05b.

Can confirm in 21w06a.

Can confirm in 21w07a.

This should be labeled as vanilla-parity because this is fixed in Bedrock.

How do villagers melee attack?

The mobs that have this issue all use the MeleeAttackGoal class or their own class that extends it. There is a field called lastCanUseCheck, which is the first check if the goal can be used. This field is used to ensure if at least one second has passed since the last time this goal executed before any other checks are done.

For the goal to execute, the mob then must have a target that is alive and the mob must either have a path to the target, or be within melee range. While the goal is running, a field called followingTargetEvenIfNotSeen is checked to allow the mob to pathfind toward the target even if it cannot be seen. Some mobs have this set to false, and others set to true.

The method MeleeAttackGoal::canContinueToUse is called each tick while the goal is running to check if the goal should continue. The field followingTargetEvenIfNotSeen is checked again in this method and if it's not true, the goal will stop if the mob does not have a path. Which means that when mobs occasionally can't find a path to the target and have followingTargetEvenIfNotSeen set to false, the goal stops and begins the checks to start again as described in the first and second paragraph.

The field lastCanUseCheck should be removed entirely, and the check for followingTargetEvenIfNotSeen in the method MeleeAttackGoal::canContinueToUse should also be removed.

This issue is directly responsible for MC-191642. The field that controls the attack timer is called ticksUntilNextAttack. When the goal starts, this field is set to zero, which, as a result of the above description, causes the timer to reset and allow the mob to attack twice in a row. The method MeleeAttackGoal::start should remove where the field ticksUntilNextAttack is set to zero.

@@unknown I updated the title and description to reflect that, thanks.

In 1.20

Can reproduce the bug with Wolves easily

Can confirm in 23w33a

In 1.20.2 Pre-Release 2

After some testing, I determined a slight issue with Chumbanotz's fix: while the followingTargetEvenIfNotSeen check is problematic as it causes a short circuit (since it forces a return statement for all mobs not having this flag set to true), removing the check itself causes the goal not to stop when the entity has reached its target. This is problematic because for a target which doesn't move, there are situations where the mob would create an improper path where it wouldn't be able to reach the entity within its hitting distance, and it would be stuck not being able to attack, virtually causing MC-147516 again. (The details of whether and why this happen is unclear to me though). Rather than removing the check, it would be better to replace it something in the lines of:

...
else if (!followingTargetEvenIfNotSeen && this.mob.getNavigation().isDone()) {
    return false;
}
...

This forces an attacker which has reached its target to properly update its path towards the target as it hits by constantly cancelling and reactivating the goal (but only works properly if the lastCanUseCheck check is removed as the original fix suggested), which is probably what this check was originally intended to do.