mojira.dev

Rachel Arevalo

Assigned

No issues.

Reported

View all
MC-304761 Spears' charge animation takes too long to rotate sideways for slower spears Fixed MC-304550 Items with the `minecraft:attack_range` component do not have an increased pick radius for redirectable projectiles Confirmed MC-303181 Spears have an attribute modifier for attack_damage for both hands, even though the charge attack does not use it Fixed MCPE-229475 Spears have maximum reach of 4 blocks Fixed MC-302165 Bows/tridents/crossbows are able to be charged earlier than should be possible on a singleplayer world Community Consensus

Comments

Code of the “issue”:

switch (this.hitResult.getType()) {
	case ENTITY:
		...
	case BLOCK:
		...
	case MISS:
		if (this.gameMode.hasMissTime()) {
			this.missTime = 10; // Delay in client ticks to be able to hit again
		}

		this.player.resetAttackStrengthTicker();
}

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.

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:

  1. Unnecessary for the Spear’s jab attack.

  2. 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:

  1. Set the game’s tick rate to 5 TPS.

  2. Hit a mob.

  3. Observe that the mob does not visually move on the client for an arbitrary delay.

  4. Repeat.

  5. 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.

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.

Load more comments