Animation Cleanup (2.4.4)

.gitignore

@@ -62,3 +62,4 @@ common/src/main/resources/data/bluelib/controller/*
 common/src/main/resources/assets/bluelib/textures/*
 /.env
 /neoforge
+/fabric/out

build.gradle.kts

@@ -14,9 +14,9 @@ githubRelease {
     token(System.getenv("GITHUB") ?: "Invalid/No API Token Found")
     owner.set("MeAlam1")
     repo.set("BlueLib")
-    tagName.set("2.4.0")
+    tagName.set("2.4.4")
     targetCommitish.set("1.21-1.21.3")
-    releaseName.set("BlueLib 2.4.0")
+    releaseName.set("BlueLib 2.4.4")
     body.set(rootProject.file("changelog.md").readText())
     draft.set(false)
     prerelease.set(false)

changelog.md

@@ -1,12 +1,49 @@
-# 2.4.3
+# 2.4.4
+
+## Added
+
+- New `AnimationExtraData` class for managing animation-related data with type-safe `DataTicket` support
+- New `AnimationSnapshot` record to encapsulate immutable animation state (limbSwing, limbSwingAmount, partialTick,
+  isMoving)
+- New `InterpolationData` record replacing `AnimationPoint` with added `getProgress()` method for calculating normalized
+  interpolation progress
+- New `AnimationFrameVector` record to group X, Y, Z animation frames together
+- Utility methods `isEmpty()` and `size()` added to `Animation` class
+- `isWait()` helper method added to `Animation.Frame` record
+- `toString()` method added to `Animation` and `AnimationState` for better debugging
+- Input validation with `IllegalArgumentException` for negative ticks, non-positive play counts, and NaN animation
+  values
+- New packet that syncs the Variant Name with all clients when changed on the server
+    - Shouldn't be necessary to add this manually, but due to a high amount of reports about desyncs, we added it just
+      to be sure
 
 ## Changed
 
-* Massive cleanup in the way we register Codecs and Data ComponentTypes.
+- Renamed `Animation.Stage` to `Animation.Frame` for clearer semantics
+- Renamed `AnimationPoint` to `InterpolationData` with updated field names (`animationStartValue` → `startValue`,
+  `animationEndValue` → `endValue`)
+- Renamed `AnimationPointFrame` to `AnimationFrame` and moved to `frame` subpackage
+- Renamed `getAnimationStages()` to `getAnimationFrames()` in `Animation` class
+- Renamed `BoneSnapshot` to `BoneFrame` for consistency
+- Refactored `BoneAnimationFrame` to be a final class instead of record to avoid a misleading immutable structure.
+- Refactored `BoneAnimationFrame` to use `AnimationFrameVector` instead of individual X/Y/Z queues
+- Renamed methods in `BoneAnimationFrame`: `addRotations` → `addNextRotation`, `addPositions` → `addNextPosition`,
+  `addScales` → `addNextScale`
+- `AnimationState` now uses composition with `AnimationSnapshot` and `AnimationExtraData` instead of individual fields
+- `getAnimationFrames()` now returns an unmodifiable list
+- `getController()` in `AnimationState` now throws `IllegalStateException` if controller is not set
+- `setControllerSpeed()` parameter changed from `Double` to primitive `double`
+- `animationTick` field in `AnimationState` is now private with getter/setter methods
+- Improved `equals()` and `hashCode()` implementations for `Animation` and `Animation.Frame`
+
+## Deleted
+
+- Removed `AnimationPoint` class (replaced by `InterpolationData`)
+- Removed individual rotation/position/scale queue fields from `BoneAnimationFrame` (consolidated into
+  `AnimationFrameVector`)
+- Removed individual `addRotationXPoint`, `addRotationYPoint`, etc. methods (replaced with vector-based methods)
 
 ## Bug Fixes
 
-* Fixed a critical issue where Fabric Server where enable to be started due to trying to load Client sided Code on the
-  Server.
-* Fixed a critical issue where Clients would crash when trying to send a Packet.
-* Fixed a crash where the Client was looking for the Controller file.
+- Fixed `equals()` method in `Animation` and `Animation.Frame` to properly compare field values instead of just hashCode
+- Added proper null handling in `AnimationExtraData.set()` method

common/src/main/java/software/bluelib/BlueLibConstants.java

@@ -51,7 +51,7 @@ public static <T> ServiceLoader<T> loadAll(@NotNull Class<T> pClazz) {
 	public static final String MOD_NAME = "BlueLib";
 
 	@NotNull
-	public static final String VERSION = "2.4.2";
+	public static final String VERSION = "2.4.4";
 
 	@Nullable
 	public static MinecraftServer server;

common/src/main/java/software/bluelib/api/entity/variant/IVariantEntity.java

@@ -15,11 +15,13 @@
 import net.minecraft.world.entity.Entity;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import software.bluelib.api.net.NetworkRegistry;
 import software.bluelib.api.utils.logging.BaseLogLevel;
 import software.bluelib.api.utils.logging.BaseLogger;
 import software.bluelib.api.utils.variant.ParameterUtils;
 import software.bluelib.entity.variant.IVariantAccessor;
 import software.bluelib.internal.BlueTranslation;
+import software.bluelib.net.messages.client.variant.SetVariantPacket;
 
 @SuppressWarnings("unused")
 public interface IVariantEntity<T extends Entity> {
@@ -56,6 +58,9 @@ default String getVariantName() {
 
 	default void setVariantName(@NotNull String pVariantName) {
 		T entity = getEntity();
+		int id = entity.getId();
+		SetVariantPacket packet = new SetVariantPacket(id, pVariantName);
+		NetworkRegistry.sendToAllPlayers(entity.getServer(), packet);
 		((IVariantAccessor) entity).setEntityVariantName(pVariantName);
 	}
 }

common/src/main/java/software/bluelib/client/internal/registry/BlueClientNetworkRegistry.java

@@ -14,9 +14,11 @@
 import software.bluelib.api.net.PacketProvider;
 import software.bluelib.client.net.OpenLoggerPacketHandler;
 import software.bluelib.client.net.loader.*;
+import software.bluelib.client.net.variant.SetVariantPacketHandler;
 import software.bluelib.net.PacketRegisterInfo;
 import software.bluelib.net.messages.client.OpenLoggerPacket;
 import software.bluelib.net.messages.client.loader.*;
+import software.bluelib.net.messages.client.variant.SetVariantPacket;
 import software.bluelib.net.messages.server.TestPacket;
 
 @ApiStatus.Internal
@@ -35,6 +37,9 @@ public class BlueClientNetworkRegistry implements PacketProvider {
 	public @NotNull List<PacketRegisterInfo<?>> getS2CPackets() {
 		List<PacketRegisterInfo<?>> list = new ArrayList<>();
 
+		// Variants
+		list.add(new PacketRegisterInfo<>(SetVariantPacket.ID, SetVariantPacket::decode, SetVariantPacketHandler::new));
+
 		// Logger
 		list.add(new PacketRegisterInfo<>(OpenLoggerPacket.ID, OpenLoggerPacket::decode, OpenLoggerPacketHandler::new));
 

common/src/main/java/software/bluelib/client/net/variant/SetVariantPacketHandler.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2024 BlueLib Contributors
+ *
+ * This Source Code Form is subject to the terms of the MIT License.
+ * If a copy of the MIT License was not distributed with this file,
+ * You can obtain one at https://opensource.org/licenses/MIT.
+ */
+package software.bluelib.client.net.variant;
+
+import net.minecraft.client.Minecraft;
+import net.minecraft.client.multiplayer.ClientLevel;
+import net.minecraft.world.entity.Entity;
+import net.minecraft.world.entity.LivingEntity;
+import org.jetbrains.annotations.NotNull;
+import software.bluelib.api.net.ClientNetworkPacketHandler;
+import software.bluelib.entity.variant.IVariantAccessor;
+import software.bluelib.net.messages.client.variant.SetVariantPacket;
+
+public class SetVariantPacketHandler implements ClientNetworkPacketHandler<SetVariantPacket> {
+
+	@Override
+	public void handle(@NotNull SetVariantPacket pPacket, @NotNull Minecraft pClient) {
+		pClient.execute(() -> {
+			ClientLevel level = pClient.level;
+			if (level == null) return;
+
+			Entity e = level.getEntity(pPacket.entityId());
+			if (!(e instanceof LivingEntity living)) return;
+
+			((IVariantAccessor) living).setEntityVariantName(pPacket.variant());
+		});
+	}
+}

common/src/main/java/software/bluelib/config/LoggerConfig.java

@@ -12,8 +12,8 @@
 public class LoggerConfig {
 
 	// TODO: BlueLib Logging should remain false by default
-	public static boolean isBlueLibLoggingEnabled = false;
-	public static boolean isLoggingEnabled = false;
+	public static boolean isBlueLibLoggingEnabled = true;
+	public static boolean isLoggingEnabled = true;
 	@ApiStatus.Internal
 	public static final boolean isExampleEnabled = false;
 }

common/src/main/java/software/bluelib/internal/registry/BlueNetworkRegistry.java

@@ -15,6 +15,7 @@
 import software.bluelib.net.PacketRegisterInfo;
 import software.bluelib.net.messages.client.OpenLoggerPacket;
 import software.bluelib.net.messages.client.loader.*;
+import software.bluelib.net.messages.client.variant.SetVariantPacket;
 import software.bluelib.net.messages.server.TestPacket;
 import software.bluelib.net.serverHandling.TestPacketHandler;
 
@@ -34,6 +35,8 @@ public class BlueNetworkRegistry implements PacketProvider {
 	public @NotNull List<PacketRegisterInfo<?>> getS2CPackets() {
 		List<PacketRegisterInfo<?>> list = new ArrayList<>();
 
+		list.add(new PacketRegisterInfo<>(SetVariantPacket.ID, SetVariantPacket::decode));
+
 		list.add(new PacketRegisterInfo<>(OpenLoggerPacket.ID, OpenLoggerPacket::decode));
 
 		list.add(new PacketRegisterInfo<>(ControllerCachePacket.ID, ControllerCachePacket::decode));

common/src/main/java/software/bluelib/loader/animatable/base/AnimatableManager.java

@@ -17,15 +17,15 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import software.bluelib.loader.animation.AnimationController;
-import software.bluelib.loader.animation.bone.BoneSnapshot;
+import software.bluelib.loader.animation.keyframe.BoneFrame;
 import software.bluelib.loader.cache.controller.ControllerCache;
 import software.bluelib.loader.controller.ControllerManager;
 import software.bluelib.loader.geckolib.constant.dataticket.DataTicket;
 
 public class AnimatableManager<T extends BlueAnimatable> {
 
 	@NotNull
-	private final Map<String, BoneSnapshot> boneSnapshotCollection = new Object2ObjectOpenHashMap<>();
+	private final Map<String, BoneFrame> boneSnapshotCollection = new Object2ObjectOpenHashMap<>();
 	@NotNull
 	private final Map<String, AnimationController<T>> animationControllers;
 	@Nullable
@@ -60,7 +60,7 @@ public Map<String, AnimationController<T>> getAnimationControllers() {
 	}
 
 	@NotNull
-	public Map<String, BoneSnapshot> getBoneSnapshotCollection() {
+	public Map<String, BoneFrame> getBoneSnapshotCollection() {
 		return this.boneSnapshotCollection;
 	}
 

common/src/main/java/software/bluelib/loader/animatable/base/ContextAwareAnimatableManager.java

@@ -11,7 +11,7 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import software.bluelib.loader.animation.AnimationController;
-import software.bluelib.loader.animation.bone.BoneSnapshot;
+import software.bluelib.loader.animation.keyframe.BoneFrame;
 import software.bluelib.loader.geckolib.constant.dataticket.DataTicket;
 
 public abstract class ContextAwareAnimatableManager<T extends BlueAnimatable, C> extends AnimatableManager<T> {
@@ -48,7 +48,7 @@ public void removeController(@NotNull String pName) {
 		return getManagerForContext(getCurrentContext()).getAnimationControllers();
 	}
 
-	public @NotNull Map<String, BoneSnapshot> getBoneSnapshotCollection() {
+	public @NotNull Map<String, BoneFrame> getBoneSnapshotCollection() {
 		return getManagerForContext(getCurrentContext()).getBoneSnapshotCollection();
 	}
 

common/src/main/java/software/bluelib/loader/animation/Animation.java

@@ -8,6 +8,7 @@
 package software.bluelib.loader.animation;
 
 import it.unimi.dsi.fastutil.objects.ObjectArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import org.jetbrains.annotations.NotNull;
@@ -17,9 +18,8 @@
 public final class Animation {
 
 	@NotNull
-	private final List<Stage> animationList = new ObjectArrayList<>();
+	private final List<Frame> animationList = new ObjectArrayList<>();
 
-	// Private constructor to force usage of factory for logical operations
 	private Animation() {}
 
 	@NotNull
@@ -39,85 +39,105 @@ public Animation thenLoop(@NotNull String pAnimationName) {
 
 	@NotNull
 	public Animation thenWait(int pTicks) {
-		this.animationList.add(new Stage(Stage.WAIT, AnimationCache.LoopType.PLAY_ONCE, pTicks));
-
+		if (pTicks < 0) {
+			throw new IllegalArgumentException("Ticks cannot be negative");
+		}
+		this.animationList.add(new Frame(Frame.WAIT, AnimationCache.LoopType.PLAY_ONCE, pTicks));
 		return this;
 	}
 
 	@NotNull
-	public Animation thenPlayAndHold(@NotNull String pAnimation) {
-		return then(pAnimation, AnimationCache.LoopType.HOLD_ON_LAST_FRAME);
+	public Animation thenPlayAndHold(@NotNull String pAnimationName) {
+		return then(pAnimationName, AnimationCache.LoopType.HOLD_ON_LAST_FRAME);
 	}
 
 	@NotNull
 	public Animation thenPlayXTimes(@NotNull String pAnimationName, int pPlayCount) {
+		if (pPlayCount <= 0) {
+			throw new IllegalArgumentException("Play count must be positive");
+		}
 		for (int i = 0; i < pPlayCount; i++) {
 			then(pAnimationName, i == pPlayCount - 1 ? AnimationCache.LoopType.DEFAULT : AnimationCache.LoopType.PLAY_ONCE);
 		}
-
 		return this;
 	}
 
 	@NotNull
 	public Animation then(@NotNull String pAnimationName, @NotNull AnimationCache.LoopType pLoopType) {
-		this.animationList.add(new Stage(pAnimationName, pLoopType));
-
+		this.animationList.add(new Frame(pAnimationName, pLoopType));
 		return this;
 	}
 
 	@NotNull
-	public List<Stage> getAnimationStages() {
-		return this.animationList;
+	public List<Frame> getAnimationFrames() {
+		return Collections.unmodifiableList(this.animationList);
 	}
 
 	@NotNull
 	public static Animation copyOf(@NotNull Animation pOther) {
 		Animation newInstance = Animation.begin();
-
 		newInstance.animationList.addAll(pOther.animationList);
-
 		return newInstance;
 	}
 
-	@Override
-	public boolean equals(@Nullable Object pObj) {
-		if (this == pObj)
-			return true;
+	public boolean isEmpty() {
+		return this.animationList.isEmpty();
+	}
 
-		if (pObj == null || getClass() != pObj.getClass())
-			return false;
+	public int size() {
+		return this.animationList.size();
+	}
 
-		return hashCode() == pObj.hashCode();
+	@Override
+	public boolean equals(@Nullable Object pObj) {
+		if (this == pObj) return true;
+		if (pObj == null || getClass() != pObj.getClass()) return false;
+		Animation animation = (Animation) pObj;
+		return Objects.equals(this.animationList, animation.animationList);
 	}
 
 	@Override
 	public int hashCode() {
 		return Objects.hash(this.animationList);
 	}
 
-	public record Stage(@NotNull String animationName, @NotNull AnimationCache.LoopType loopType, int additionalTicks) {
+	@Override
+	public String toString() {
+		return "Animation{stages=" + this.animationList + "}";
+	}
+
+	public record Frame(@NotNull String animationName, @NotNull AnimationCache.LoopType loopType, int additionalTicks) {
 
 		@NotNull
 		public static final String WAIT = "internal.wait";
 
-		public Stage(@NotNull String pAnimationName, @NotNull AnimationCache.LoopType pLoopType) {
+		public Frame {
+			if (additionalTicks < 0) {
+				throw new IllegalArgumentException("Additional ticks cannot be negative");
+			}
+		}
+
+		public Frame(@NotNull String pAnimationName, @NotNull AnimationCache.LoopType pLoopType) {
 			this(pAnimationName, pLoopType, 0);
 		}
 
+		public boolean isWait() {
+			return WAIT.equals(this.animationName);
+		}
+
 		@Override
 		public boolean equals(@Nullable Object pObj) {
-			if (this == pObj)
-				return true;
-
-			if (pObj == null || getClass() != pObj.getClass())
-				return false;
-
-			return hashCode() == pObj.hashCode();
+			if (this == pObj) return true;
+			if (pObj == null || getClass() != pObj.getClass()) return false;
+			Frame frame = (Frame) pObj;
+			return this.additionalTicks == frame.additionalTicks
+					&& Objects.equals(this.animationName, frame.animationName)
+					&& this.loopType == frame.loopType;
 		}
 
 		@Override
 		public int hashCode() {
-			return Objects.hash(this.animationName, this.loopType);
+			return Objects.hash(this.animationName, this.loopType, this.additionalTicks);
 		}
 	}
 }