Also I have come up with better reproduction steps
Note that the screenshot is to show how to aim oneself to reproduce the issue, it does not show the issue itself.
It seems unintended based on it going unmentioned.
I am not sure this is a bug, however I can confirm that this does happen.
Please rename this report. It is unrelated to the minecraft:minimum_attack_charge component, rather, this issue arises as the stabAttack() method itself ignores attack cooldown as it is both:
Unnecessary for the Spear’s jab attack.
Used for the Spear’s charge attack.
The solution to this, in theory, is adding a parameter to stabAttack() for attack strength scale, and providing the attack strength scale as one in KineticWeapon, and the actual player attack strength scale in PiercingWeapon
Also this report is mislabeled as affecting 25w42a
In theory this could be solved with adding a condition for checking food data instead of the way that they did implement it (of course, also adding a condition, unless one already exists, to check for creative to bypass it)…
By the way, can you update the to reproduce, expected result, and actual result sections to include more detail? This is what I have in mind:
Steps to reproduce:
Set the game’s tick rate to 5 TPS.
Hit a mob.
Observe that the mob does not visually move on the client for an arbitrary delay.
Repeat.
Notice that the delay varies in length.
Expected result:
The mob should move instantly on the client.
Actual result:
The mob experiences variable degrees of delay for the knockback to occur.
Also in 1.21.10
Confirmed in 25w42a
Neither of these reports are attribute swapping, the Spear just provides its Attack Damage modifier to the off-hand…
To be more specific, the spear does check if a block is in the way of its attack, however it is between min reach and max reach. If the block is in front of the player but at a distance less than the min reach, it cannot be picked and thus fails. This should be easy to fix, make it pick blocks from the entity’s eye position to its max reach instead and cancel it if the block hit result is less than the min reach.
I should note, easier fix: just move the call for ChunkMap.tick() to the end of ServerLevel.tick(BooleanSupplier booleanSupplier). This is what I did in my PR for Paper, and it works flawlessly.
If I had to speculate, the performance impact was more substantial back when the game was older, so these checks were an optimisation made to account for that. my preferred fix would be bumping the base tick speed for the game up to either run entirely on delta time or to run at a higher tick rate since this also minimises tick jitter and makes the update intervals less important.
But I do agree, though it might have minor performance impacts this would in general be an improvement, but not necessarily a bug since it’s how the server code was intentionally designed and has always worked, 1.3 just made it impact singleplayer.
Oh, okay, I would like to note that the way entities are updated actually predates even 1.3 from the server side, 1.2.5 singleplayer is indeed much more responsive however that is because of no integrated server. This is from 1.2.5 server code using MCP mappings:
// EntityTrackerEntry.java
if (this.updateCounter++ % this.field_9234_e == 0 || this.trackedEntity.isAirBorne)
{
int var2 = MathHelper.floor_double(this.trackedEntity.posX * 32.0D);
int var3 = MathHelper.floor_double(this.trackedEntity.posY * 32.0D);
int var4 = MathHelper.floor_double(this.trackedEntity.posZ * 32.0D);
int var5 = MathHelper.floor_float(this.trackedEntity.rotationYaw * 256.0F / 360.0F);
int var6 = MathHelper.floor_float(this.trackedEntity.rotationPitch * 256.0F / 360.0F);
int var7 = var2 - this.encodedPosX;
int var8 = var3 - this.encodedPosY;
int var9 = var4 - this.encodedPosZ;
Object var10 = null;
boolean var11 = Math.abs(var7) >= 4 || Math.abs(var8) >= 4 || Math.abs(var9) >= 4;
boolean var12 = Math.abs(var5 - this.encodedRotationYaw) >= 4 || Math.abs(var6 - this.encodedRotationPitch) >= 4;
if (var7 >= -128 && var7 < 128 && var8 >= -128 && var8 < 128 && var9 >= -128 && var9 < 128 && this.field_28165_t <= 400)
{
if (var11 && var12)
{
var10 = new Packet33RelEntityMoveLook(this.trackedEntity.entityId, (byte)var7, (byte)var8, (byte)var9, (byte)var5, (byte)var6);
}
else if (var11)
{
var10 = new Packet31RelEntityMove(this.trackedEntity.entityId, (byte)var7, (byte)var8, (byte)var9);
}
else if (var12)
{
var10 = new Packet32EntityLook(this.trackedEntity.entityId, (byte)var5, (byte)var6);
}
}
else
{
this.field_28165_t = 0;
this.trackedEntity.posX = (double)var2 / 32.0D;
this.trackedEntity.posY = (double)var3 / 32.0D;
this.trackedEntity.posZ = (double)var4 / 32.0D;
var10 = new Packet34EntityTeleport(this.trackedEntity.entityId, var2, var3, var4, (byte)var5, (byte)var6);
}there is a slight difference in that the update counter is updated before the check rather than after however this should not result in a substantial gameplay difference.
This was not changed in 1.3, as can be seen here:
if (this.ticks++ % this.updateFrequency == 0 || this.myEntity.isAirBorne)
{
int var2 = this.myEntity.myEntitySize.multiplyBy32AndRound(this.myEntity.posX);
int var3 = MathHelper.floor_double(this.myEntity.posY * 32.0D);
int var4 = this.myEntity.myEntitySize.multiplyBy32AndRound(this.myEntity.posZ);
int var5 = MathHelper.floor_float(this.myEntity.rotationYaw * 256.0F / 360.0F);
int var6 = MathHelper.floor_float(this.myEntity.rotationPitch * 256.0F / 360.0F);
int var7 = var2 - this.lastScaledXPosition;
int var8 = var3 - this.lastScaledYPosition;
int var9 = var4 - this.lastScaledZPosition;
Object var10 = null;
boolean var11 = Math.abs(var7) >= 4 || Math.abs(var8) >= 4 || Math.abs(var9) >= 4;
boolean var12 = Math.abs(var5 - this.lastYaw) >= 4 || Math.abs(var6 - this.lastPitch) >= 4;
if (var7 >= -128 && var7 < 128 && var8 >= -128 && var8 < 128 && var9 >= -128 && var9 < 128 && this.ticksSinceLastForcedTeleport <= 400)
{
if (var11 && var12)
{
var10 = new Packet33RelEntityMoveLook(this.myEntity.entityId, (byte)var7, (byte)var8, (byte)var9, (byte)var5, (byte)var6);
}
else if (var11)
{
var10 = new Packet31RelEntityMove(this.myEntity.entityId, (byte)var7, (byte)var8, (byte)var9);
}
else if (var12)
{
var10 = new Packet32EntityLook(this.myEntity.entityId, (byte)var5, (byte)var6);
}
}
else
{
this.ticksSinceLastForcedTeleport = 0;
var10 = new Packet34EntityTeleport(this.myEntity.entityId, var2, var3, var4, (byte)var5, (byte)var6);
}Though by 1.8 they did add more conditions, oddly enough:
if (this.updateCounter % this.updateFrequency == 0 || this.trackedEntity.isAirBorne || this.trackedEntity.getDataWatcher().hasObjectChanged())
{
if (this.trackedEntity.ridingEntity == null)
{
++this.ticksSinceLastForcedTeleport;
int k = MathHelper.floor_double(this.trackedEntity.posX * 32.0D);
int j1 = MathHelper.floor_double(this.trackedEntity.posY * 32.0D);
int k1 = MathHelper.floor_double(this.trackedEntity.posZ * 32.0D);
int l1 = MathHelper.floor_float(this.trackedEntity.rotationYaw * 256.0F / 360.0F);
int i2 = MathHelper.floor_float(this.trackedEntity.rotationPitch * 256.0F / 360.0F);
int j2 = k - this.encodedPosX;
int k2 = j1 - this.encodedPosY;
int i = k1 - this.encodedPosZ;
Packet packet1 = null;
boolean flag = Math.abs(j2) >= 4 || Math.abs(k2) >= 4 || Math.abs(i) >= 4 || this.updateCounter % 60 == 0;
boolean flag1 = Math.abs(l1 - this.encodedRotationYaw) >= 4 || Math.abs(i2 - this.encodedRotationPitch) >= 4;
if (this.updateCounter > 0 || this.trackedEntity instanceof EntityArrow)
{
if (j2 >= -128 && j2 < 128 && k2 >= -128 && k2 < 128 && i >= -128 && i < 128 && this.ticksSinceLastForcedTeleport <= 400 && !this.ridingEntity && this.onGround == this.trackedEntity.onGround)
{
if ((!flag || !flag1) && !(this.trackedEntity instanceof EntityArrow))
{
if (flag)
{
packet1 = new S14PacketEntity.S15PacketEntityRelMove(this.trackedEntity.getEntityId(), (byte)j2, (byte)k2, (byte)i, this.trackedEntity.onGround);
}
else if (flag1)
{
packet1 = new S14PacketEntity.S16PacketEntityLook(this.trackedEntity.getEntityId(), (byte)l1, (byte)i2, this.trackedEntity.onGround);
}
}
else
{
packet1 = new S14PacketEntity.S17PacketEntityLookMove(this.trackedEntity.getEntityId(), (byte)j2, (byte)k2, (byte)i, (byte)l1, (byte)i2, this.trackedEntity.onGround);
}
}
else
{
this.onGround = this.trackedEntity.onGround;
this.ticksSinceLastForcedTeleport = 0;
packet1 = new S18PacketEntityTeleport(this.trackedEntity.getEntityId(), k, j1, k1, (byte)l1, (byte)i2, this.trackedEntity.onGround);
}
}The differences between versions show that with only the fix to tracker update timings you should have the same core functionality if not more fluid in modern versions. Thus, this is more a suggestion than a bug.
So, few complaints: This is a duplicate of MC-297196 and also is incorrect.
Whilst your code analysis is fundamentally true, all behaviour mentioned in the code analysis already existed before 1.14 and was merely updated across versions. The actual changes in 1.14 responsible for this are to do with the entity trackers being moved to the ChunkMap class and thus happening before the entity ticks. The fix you have described treats the symptoms, but not the actual bug.
The order is in fact how it was done before 1.14, when this bug was introduced. Therefore, any other logical changes are how they were intended and byproducts of them moving it as well.
Code of the “issue”:
However, it seems to be a feature considering miss time is introduced solely to make this happen, and it is added specifically to non-creative players.