From c8efaa46cbaab915a76fd7feca591440cbe35aee Mon Sep 17 00:00:00 2001 From: Bjarne Koll Date: Mon, 16 Sep 2024 23:07:29 +0200 Subject: [PATCH] Remove wall-time / unused skip tick protection Spigot still maintains some partial implementation of "tick skipping", a practice in which the MinecraftServer.currentTick field is updated not by an increment of one per actual tick, but instead set to System.currentTimeMillis() / 50. This behaviour means that the tracked tick may "skip" a tick value in case a previous tick took more than the expected 50ms. To compensate for this in important paths, spigot/craftbukkit implements "wall-time". Instead of incrementing/decrementing ticks on block entities/entities by one for each call to their tick() method, they instead increment/decrement important values, like an ItemEntity's age or pickupDelay, by the difference of `currentTick - lastTick`, where `lastTick` is the value of `currentTick` during the last tick() call. These "fixes" however do not play nicely with minecraft's simulation distance as entities/block entities implementing the above behaviour would "catch up" their values when moving from a non-ticking chunk to a ticking one as their `lastTick` value remains stuck on the last tick in a ticking chunk and hence lead to a large "catch up" once ticked again. Paper completely removes the "tick skipping" behaviour (See patch "Further-improve-server-tick-loop"), making the above precautions completely unnecessary, which also rids paper of the previous described incompatibility with non-ticking chunks. --- .../world/entity/item/ItemEntity.java.patch | 64 +++++++++---------- .../world/entity/monster/Zombie.java.patch | 35 +++++----- .../entity/BrewingStandBlockEntity.java.patch | 18 +++--- 3 files changed, 55 insertions(+), 62 deletions(-) diff --git a/paper-server/patches/sources/net/minecraft/world/entity/item/ItemEntity.java.patch b/paper-server/patches/sources/net/minecraft/world/entity/item/ItemEntity.java.patch index 5a90b965e8..0bc136847c 100644 --- a/paper-server/patches/sources/net/minecraft/world/entity/item/ItemEntity.java.patch +++ b/paper-server/patches/sources/net/minecraft/world/entity/item/ItemEntity.java.patch @@ -59,7 +59,7 @@ @Nullable public UUID target; public final float bobOffs; -+ private int lastTick = MinecraftServer.currentTick - 1; // CraftBukkit ++ // private int lastTick = MinecraftServer.currentTick - 1; // CraftBukkit // Paper - remove anti tick skipping measures / wall time + public boolean canMobPickup = true; // Paper - Item#canEntityPickup + private int despawnRate = -1; // Paper - Alternative item-despawn-rate + public net.kyori.adventure.util.TriState frictionState = net.kyori.adventure.util.TriState.NOT_SET; // Paper - Friction API @@ -80,7 +80,7 @@ } public ItemEntity(Level world, double x, double y, double z, ItemStack stack, double velocityX, double velocityY, double velocityZ) { -@@ -133,12 +150,15 @@ +@@ -133,12 +150,14 @@ @Override public void tick() { if (this.getItem().isEmpty()) { @@ -88,19 +88,15 @@ + this.discard(EntityRemoveEvent.Cause.DESPAWN); // CraftBukkit - add Bukkit remove cause } else { super.tick(); -- if (this.pickupDelay > 0 && this.pickupDelay != 32767) { -- --this.pickupDelay; -- } -+ // CraftBukkit start - Use wall time for pickup and despawn timers -+ int elapsedTicks = MinecraftServer.currentTick - this.lastTick; -+ if (this.pickupDelay != 32767) this.pickupDelay -= elapsedTicks; -+ if (this.age != -32768) this.age += elapsedTicks; -+ this.lastTick = MinecraftServer.currentTick; -+ // CraftBukkit end ++ // Paper start - remove anti tick skipping measures / wall time - revert to vanilla + if (this.pickupDelay > 0 && this.pickupDelay != 32767) { + --this.pickupDelay; + } ++ // Paper end - remove anti tick skipping measures / wall time - revert to vanilla this.xo = this.getX(); this.yo = this.getY(); -@@ -162,12 +182,16 @@ +@@ -162,12 +181,16 @@ } } @@ -119,11 +115,11 @@ f = this.level().getBlockState(this.getBlockPosBelowThatAffectsMyMovement()).getBlock().getFriction() * 0.98F; } -@@ -188,9 +212,11 @@ +@@ -188,9 +211,11 @@ this.mergeWithNeighbours(); } -+ /* CraftBukkit start - moved up ++ // Paper - remove anti tick skipping measures / wall time - revert to vanilla /* CraftBukkit start - moved up if (this.age != -32768) { ++this.age; } @@ -131,7 +127,7 @@ this.hasImpulse |= this.updateInWaterStateAndDoFluidPushing(); if (!this.level().isClientSide) { -@@ -201,14 +227,42 @@ +@@ -201,14 +226,44 @@ } } @@ -153,12 +149,14 @@ + // Spigot start - copied from above @Override + public void inactiveTick() { -+ // CraftBukkit start - Use wall time for pickup and despawn timers -+ int elapsedTicks = MinecraftServer.currentTick - this.lastTick; -+ if (this.pickupDelay != 32767) this.pickupDelay -= elapsedTicks; -+ if (this.age != -32768) this.age += elapsedTicks; -+ this.lastTick = MinecraftServer.currentTick; -+ // CraftBukkit end ++ // Paper start - remove anti tick skipping measures / wall time - copied from above ++ if (this.pickupDelay > 0 && this.pickupDelay != 32767) { ++ --this.pickupDelay; ++ } ++ if (this.age != -32768) { ++ ++this.age; ++ } ++ // Paper end - remove anti tick skipping measures / wall time - copied from above + + if (!this.level().isClientSide && this.age >= this.despawnRate) { // Spigot // Paper - Alternative item-despawn-rate + // CraftBukkit start - fire ItemDespawnEvent @@ -176,7 +174,7 @@ public BlockPos getBlockPosBelowThatAffectsMyMovement() { return this.getOnPos(0.999999F); } -@@ -229,7 +283,10 @@ +@@ -229,7 +284,10 @@ private void mergeWithNeighbours() { if (this.isMergable()) { @@ -188,7 +186,7 @@ return entityitem != this && entityitem.isMergable(); }); Iterator iterator = list.iterator(); -@@ -238,6 +295,14 @@ +@@ -238,6 +296,14 @@ ItemEntity entityitem = (ItemEntity) iterator.next(); if (entityitem.isMergable()) { @@ -203,7 +201,7 @@ this.tryToMerge(entityitem); if (this.isRemoved()) { break; -@@ -251,7 +316,7 @@ +@@ -251,7 +317,7 @@ private boolean isMergable() { ItemStack itemstack = this.getItem(); @@ -212,7 +210,7 @@ } private void tryToMerge(ItemEntity other) { -@@ -259,7 +324,7 @@ +@@ -259,7 +325,7 @@ ItemStack itemstack1 = other.getItem(); if (Objects.equals(this.target, other.target) && ItemEntity.areMergable(itemstack, itemstack1)) { @@ -221,7 +219,7 @@ ItemEntity.merge(this, itemstack, other, itemstack1); } else { ItemEntity.merge(other, itemstack1, this, itemstack); -@@ -287,11 +352,16 @@ +@@ -287,11 +353,16 @@ } private static void merge(ItemEntity targetEntity, ItemStack targetStack, ItemEntity sourceEntity, ItemStack sourceStack) { @@ -239,7 +237,7 @@ } } -@@ -320,12 +390,17 @@ +@@ -320,12 +391,17 @@ } else if (!this.getItem().canBeHurtBy(source)) { return false; } else { @@ -258,7 +256,7 @@ } return true; -@@ -339,6 +414,11 @@ +@@ -339,6 +415,11 @@ @Override public void addAdditionalSaveData(CompoundTag nbt) { @@ -270,7 +268,7 @@ nbt.putShort("Health", (short) this.health); nbt.putShort("Age", (short) this.age); nbt.putShort("PickupDelay", (short) this.pickupDelay); -@@ -381,23 +461,98 @@ +@@ -381,23 +462,98 @@ this.setItem(ItemStack.EMPTY); } @@ -315,12 +313,12 @@ + if (flyAtPlayer) { + player.take(this, i); + } - ++ + return; + } + } + // Paper end - PlayerAttemptPickupItemEvent -+ + + if (this.pickupDelay <= 0 && canHold > 0) { + itemstack.setCount(canHold); + // Call legacy event @@ -372,7 +370,7 @@ itemstack.setCount(i); } -@@ -438,6 +593,7 @@ +@@ -438,6 +594,7 @@ public void setItem(ItemStack stack) { this.getEntityData().set(ItemEntity.DATA_ITEM, stack); @@ -380,7 +378,7 @@ } @Override -@@ -492,7 +648,7 @@ +@@ -492,7 +649,7 @@ public void makeFakeItem() { this.setNeverPickUp(); diff --git a/paper-server/patches/sources/net/minecraft/world/entity/monster/Zombie.java.patch b/paper-server/patches/sources/net/minecraft/world/entity/monster/Zombie.java.patch index a57ad7e512..4b95368547 100644 --- a/paper-server/patches/sources/net/minecraft/world/entity/monster/Zombie.java.patch +++ b/paper-server/patches/sources/net/minecraft/world/entity/monster/Zombie.java.patch @@ -57,7 +57,7 @@ private boolean canBreakDoors; private int inWaterTime; public int conversionTime; -+ private int lastTick = MinecraftServer.currentTick; // CraftBukkit - add field ++ // private int lastTick = MinecraftServer.currentTick; // CraftBukkit - add field // Paper - remove anti tick skipping measures / wall time + private boolean shouldBurnInDay = true; // Paper - Add more Zombie API public Zombie(EntityType type, Level world) { @@ -115,27 +115,24 @@ } } -@@ -203,7 +217,10 @@ +@@ -203,7 +217,7 @@ public void tick() { if (!this.level().isClientSide && this.isAlive() && !this.isNoAi()) { if (this.isUnderWaterConverting()) { - --this.conversionTime; -+ // CraftBukkit start - Use wall time instead of ticks for conversion -+ int elapsedTicks = MinecraftServer.currentTick - this.lastTick; -+ this.conversionTime -= elapsedTicks; -+ // CraftBukkit end ++ --this.conversionTime; // Paper - remove anti tick skipping measures / wall time if (this.conversionTime < 0) { this.doUnderWaterConversion(); } -@@ -220,6 +237,7 @@ +@@ -220,6 +234,7 @@ } super.tick(); -+ this.lastTick = MinecraftServer.currentTick; // CraftBukkit ++ // this.lastTick = MinecraftServer.currentTick; // CraftBukkit // Paper - remove anti tick skipping measures / wall time } @Override -@@ -253,7 +271,14 @@ +@@ -253,7 +268,14 @@ super.aiStep(); } @@ -146,11 +143,11 @@ + } + // Paper end - Add more Zombie API public void startUnderWaterConversion(int ticksUntilWaterConversion) { -+ this.lastTick = MinecraftServer.currentTick; // CraftBukkit ++ // this.lastTick = MinecraftServer.currentTick; // CraftBukkit // Paper - remove anti tick skipping measures / wall time this.conversionTime = ticksUntilWaterConversion; this.getEntityData().set(Zombie.DATA_DROWNED_CONVERSION_ID, true); } -@@ -267,32 +292,51 @@ +@@ -267,32 +289,51 @@ } protected void convertToZombieType(EntityType entityType) { @@ -215,7 +212,7 @@ @Override public boolean hurtServer(ServerLevel world, DamageSource source, float amount) { if (!super.hurtServer(world, source, amount)) { -@@ -323,10 +367,10 @@ +@@ -323,10 +364,10 @@ if (SpawnPlacements.isSpawnPositionOk(entitytypes, world, blockposition) && SpawnPlacements.checkSpawnRules(entitytypes, world, EntitySpawnReason.REINFORCEMENT, blockposition, world.random)) { entityzombie.setPos((double) i1, (double) j1, (double) k1); @@ -229,7 +226,7 @@ AttributeInstance attributemodifiable = this.getAttribute(Attributes.SPAWN_REINFORCEMENTS_CHANCE); AttributeModifier attributemodifier = attributemodifiable.getModifier(Zombie.REINFORCEMENT_CALLER_CHARGE_ID); double d0 = attributemodifier != null ? attributemodifier.amount() : 0.0D; -@@ -352,7 +396,14 @@ +@@ -352,7 +393,14 @@ float f = this.level().getCurrentDifficultyAt(this.blockPosition()).getEffectiveDifficulty(); if (this.getMainHandItem().isEmpty() && this.isOnFire() && this.random.nextFloat() < f * 0.3F) { @@ -245,7 +242,7 @@ } } -@@ -385,7 +436,7 @@ +@@ -385,7 +433,7 @@ @Override public EntityType getType() { @@ -254,7 +251,7 @@ } protected boolean canSpawnInLiquids() { -@@ -414,6 +465,7 @@ +@@ -414,6 +462,7 @@ nbt.putBoolean("CanBreakDoors", this.canBreakDoors()); nbt.putInt("InWaterTime", this.isInWater() ? this.inWaterTime : -1); nbt.putInt("DrownedConversionTime", this.isUnderWaterConverting() ? this.conversionTime : -1); @@ -262,7 +259,7 @@ } @Override -@@ -425,6 +477,11 @@ +@@ -425,6 +474,11 @@ if (nbt.contains("DrownedConversionTime", 99) && nbt.getInt("DrownedConversionTime") > -1) { this.startUnderWaterConversion(nbt.getInt("DrownedConversionTime")); } @@ -274,7 +271,7 @@ } -@@ -432,10 +489,8 @@ +@@ -432,10 +486,8 @@ public boolean killedEntity(ServerLevel world, LivingEntity other) { boolean flag = super.killedEntity(world, other); @@ -287,7 +284,7 @@ if (this.convertVillagerToZombieVillager(world, entityvillager)) { flag = false; -@@ -468,7 +523,7 @@ +@@ -468,7 +520,7 @@ float f = difficulty.getSpecialMultiplier(); if (spawnReason != EntitySpawnReason.CONVERSION) { @@ -296,7 +293,7 @@ } if (object == null) { -@@ -496,7 +551,7 @@ +@@ -496,7 +548,7 @@ entitychicken1.finalizeSpawn(world, difficulty, EntitySpawnReason.JOCKEY, (SpawnGroupData) null); entitychicken1.setChickenJockey(true); this.startRiding(entitychicken1); diff --git a/paper-server/patches/sources/net/minecraft/world/level/block/entity/BrewingStandBlockEntity.java.patch b/paper-server/patches/sources/net/minecraft/world/level/block/entity/BrewingStandBlockEntity.java.patch index 635b5aa5eb..380ffa3d40 100644 --- a/paper-server/patches/sources/net/minecraft/world/level/block/entity/BrewingStandBlockEntity.java.patch +++ b/paper-server/patches/sources/net/minecraft/world/level/block/entity/BrewingStandBlockEntity.java.patch @@ -34,7 +34,7 @@ public int fuel; protected final ContainerData dataAccess; + // CraftBukkit start - add fields and methods -+ private int lastTick = MinecraftServer.currentTick; ++ // private int lastTick = MinecraftServer.currentTick; // Paper - remove anti tick skipping measures / wall time + public List transaction = new java.util.ArrayList(); + private int maxStack = MAX_STACK; @@ -89,18 +89,16 @@ setChanged(world, pos, state); } -@@ -116,12 +170,17 @@ +@@ -116,12 +170,15 @@ boolean flag1 = blockEntity.brewTime > 0; ItemStack itemstack1 = (ItemStack) blockEntity.items.get(3); -+ // CraftBukkit start - Use wall time instead of ticks for brewing -+ int elapsedTicks = MinecraftServer.currentTick - blockEntity.lastTick; -+ blockEntity.lastTick = MinecraftServer.currentTick; ++ // Paper - remove anti tick skipping measures / wall time + if (flag1) { - --blockEntity.brewTime; - boolean flag2 = blockEntity.brewTime == 0; -+ blockEntity.brewTime -= elapsedTicks; ++ --blockEntity.brewTime; // Paper - remove anti tick skipping measures / wall time - revert to vanilla + boolean flag2 = blockEntity.brewTime <= 0; // == -> <= + // CraftBukkit end @@ -110,7 +108,7 @@ } else if (!flag || !itemstack1.is(blockEntity.ingredient)) { blockEntity.brewTime = 0; } -@@ -129,7 +188,11 @@ +@@ -129,7 +186,11 @@ setChanged(world, pos, state); } else if (flag && blockEntity.fuel > 0) { --blockEntity.fuel; @@ -123,7 +121,7 @@ blockEntity.ingredient = itemstack1.getItem(); setChanged(world, pos, state); } -@@ -185,14 +248,36 @@ +@@ -185,14 +246,36 @@ } } @@ -163,7 +161,7 @@ itemstack.shrink(1); ItemStack itemstack1 = itemstack.getItem().getCraftingRemainder(); -@@ -200,12 +285,12 @@ +@@ -200,12 +283,12 @@ if (itemstack.isEmpty()) { itemstack = itemstack1; } else { @@ -179,7 +177,7 @@ } @Override -@@ -231,12 +316,12 @@ +@@ -231,12 +314,12 @@ @Override public boolean canPlaceItem(int slot, ItemStack stack) {