1.19.X/Improve flying scrolls performance (#460)

* Improve flying scrolls performance * Reduce tick timer from not meeting scroll flight requirements, which I had missed in the first pass * `Entity#level` -> `Entity#level()` --------- Co-authored-by: Aizistral <[email protected]>
Aizistral-Studios · Dec 22, 2023 · 910c64e · 910c64e
1 parent be154f4
commit 910c64e
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 46 deletions.
diff --git a/src/main/java/com/aizistral/enigmaticlegacy/items/FabulousScroll.java b/src/main/java/com/aizistral/enigmaticlegacy/items/FabulousScroll.java
@@ -49,48 +49,33 @@ public void appendHoverText(ItemStack stack, @Nullable Level worldIn, List<Compo
 
 	@Override
 	public void curioTick(SlotContext context, ItemStack stack) {
-		if (context.entity().level.isClientSide)
+		if (context.entity().level().isClientSide)
 			return;
 
 		if (context.entity() instanceof Player player) {
+			//don't check in range every tick as it is expensive. Particularly for multiple people.
+			//instead, check once per second, based on the player's tick count
+			if (player.tickCount % 20 != 0)
+				return;
+
 			boolean inRange = SuperpositionHandler.isInBeaconRange(player);
 
-			if (!SuperpositionHandler.isInBeaconRange(player))
-				if (Math.random() <= (this.baseXpConsumptionProbability*8) * xpCostModifier.getValue() && player.getAbilities().flying) {
-					ExperienceHelper.drainPlayerXP(player, 1);
-				}
+			final int fabConsumptionProbMod = 8;
+			//only check xp drain if flying.
+			//because we're only checking once per 20 ticks, increase the probability by 20
+			if (this.shouldCheckXpDrain(player) && !inRange && Math.random() <= ((this.baseXpConsumptionProbability * fabConsumptionProbMod) * 20)) {
+				//cost modifier hooked up to drain xp cost
+				ExperienceHelper.drainPlayerXP(player, (int) (xpCostModifier.getValue()));
+			}
 
-			this.handleFabulousFlight(player, inRange);
+			this.handleFlight(player, inRange);
 		}
 	}
 
-	protected void handleFabulousFlight(Player player, boolean inRange) {
-		try {
-			if (ExperienceHelper.getPlayerXP(player) > 0 || inRange) {
-
-				if (!player.getAbilities().mayfly) {
-					player.getAbilities().mayfly = true;
-				}
-
-				player.onUpdateAbilities();
-				this.flyMap.put(player, 100);
-
-			} else if (this.flyMap.get(player) > 1) {
-				this.flyMap.put(player, this.flyMap.get(player)-1);
-			} else if (this.flyMap.get(player) == 1) {
-				if (!player.isCreative()) {
-					player.getAbilities().mayfly = false;
-					player.getAbilities().flying = false;
-					player.onUpdateAbilities();
-					player.addEffect(new MobEffectInstance(MobEffects.SLOW_FALLING, 200, 0, true, false));
-				}
-
-				this.flyMap.put(player, 0);
-			}
-
-		} catch (NullPointerException ex) {
-			this.flyMap.put(player, 0);
-		}
+	@Override
+	protected boolean canFly(Player player, boolean inRangeCheckedAndSucceeded)
+	{
+		return inRangeCheckedAndSucceeded || ExperienceHelper.getPlayerXP(player) > 0 || (SuperpositionHandler.isInBeaconRange(player));
 	}
 
 	@Override

diff --git a/src/main/java/com/aizistral/enigmaticlegacy/items/HeavenScroll.java b/src/main/java/com/aizistral/enigmaticlegacy/items/HeavenScroll.java
@@ -70,31 +70,49 @@ public void appendHoverText(ItemStack stack, @Nullable Level worldIn, List<Compo
 		}
 	}
 
+	// Don't charge xp if the player is in creative mode for some reason
+	// but do only charge them when they are actually flying.
+	protected boolean shouldCheckXpDrain(Player player) {
+		return !player.isCreative() && player.getAbilities().flying;
+	}
+
+	protected boolean canFly(Player player, boolean inRangeCheckedAndSucceeded)
+	{
+		return ExperienceHelper.getPlayerXP(player) > 0 && (inRangeCheckedAndSucceeded || SuperpositionHandler.isInBeaconRange(player));
+	}
+
 	@Override
 	public void curioTick(SlotContext context, ItemStack stack) {
-		if (context.entity().level.isClientSide)
+		if (context.entity().level().isClientSide)
 			return;
 
 		if (context.entity() instanceof Player player) {
-			if (Math.random() <= (this.baseXpConsumptionProbability * xpCostModifier.getValue()) && player.getAbilities().flying) {
-				ExperienceHelper.drainPlayerXP(player, 1);
+			//since we don't need to check if in range of beacon here, we can run this xp check every frame.
+			if (this.shouldCheckXpDrain(player) && Math.random() <= this.baseXpConsumptionProbability) {
+				//hook into xp cost modifier from config.
+				ExperienceHelper.drainPlayerXP(player, (int) xpCostModifier.getValue());
 			}
 
-			this.handleFlight(player);
+			// we pass false here, because we only want heaven scroll to check beacon
+			// range once per 20 ticks which is handled in the below function.
+			this.handleFlight(player, false);
 		}
-
 	}
 
-	protected void handleFlight(Player player) {
-		try {
-			if (ExperienceHelper.getPlayerXP(player) > 0 && SuperpositionHandler.isInBeaconRange(player)) {
-
-				if (!player.getAbilities().mayfly) {
-					player.getAbilities().mayfly = true;
-				}
+	protected void handleFlight(Player player, boolean inRangeCheckedAndSucceeded) {
+		//don't check in range every tick as it is expensive. Particularly for multiple people.
+		//instead, check once per second, based on the player's tick count
+		//If we're here from fabulous scroll, we automatically know we can skip this
+		if (player.tickCount % 20 != 0)
+			return;
 
+		try {
+			if (this.canFly(player, inRangeCheckedAndSucceeded)) {
+				player.getAbilities().mayfly = true;
+				//since we're only updating once per 20 ticks, we can leave this here as we won't be spamming update packets
 				player.onUpdateAbilities();
-				this.flyMap.put(player, 100);
+				//reduced from 100 down to 5, as we now only run this code once per second.
+				this.flyMap.put(player, 5);
 
 			} else if (this.flyMap.get(player) > 1) {
 				this.flyMap.put(player, this.flyMap.get(player)-1);