Conversation
Updated changelog with recent changes and deletions.
|
Texture Desync still broken, SetVariantPacket is redundant/broken, Fix before merging |
There was a problem hiding this comment.
Pull request overview
Updates BlueLib to 2.4.4 with a broad animation/keyframe refactor (renames + data-structure cleanup), adds a new variant sync S2C packet, and aligns release/version metadata.
Changes:
- Refactors animation keyframe data types (
AnimationPoint→InterpolationData,BoneSnapshot→BoneFrame,Stage→Frame) and restructures bone animation queues. - Introduces
AnimationSnapshot+AnimationExtraDatacomposition insideAnimationState, along with added validation/debugging helpers. - Adds S2C packet + handler to sync entity variant name changes to clients; bumps versions/changelog/release metadata to 2.4.4.
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle/libs.versions.toml | Bumps BlueLib version reference to 2.4.4. |
| common/src/main/java/software/bluelib/net/messages/client/variant/SetVariantPacket.java | Adds new S2C packet for variant synchronization. |
| common/src/main/java/software/bluelib/loader/model/BlueModel.java | Updates AnimationState tick assignment to use setter. |
| common/src/main/java/software/bluelib/loader/cache/model/BoneCache.java | Renames snapshot type to BoneFrame and updates snapshot methods. |
| common/src/main/java/software/bluelib/loader/cache/animations/AnimationCache.java | Updates WAIT animation generation to use Animation.Frame. |
| common/src/main/java/software/bluelib/loader/animation/math/Easing.java | Renames interpolation input type to InterpolationData and updates field access. |
| common/src/main/java/software/bluelib/loader/animation/keyframe/frame/AnimationFrameVector.java | Adds X/Y/Z grouped frame container for interpolation points. |
| common/src/main/java/software/bluelib/loader/animation/keyframe/frame/AnimationFrame.java | Moves/renames frame queue type to frame subpackage and updates generics. |
| common/src/main/java/software/bluelib/loader/animation/keyframe/InterpolationData.java | Adds new record replacing AnimationPoint, with progress helper + improved toString(). |
| common/src/main/java/software/bluelib/loader/animation/keyframe/BoneFrame.java | Renames/moves bone snapshot implementation. |
| common/src/main/java/software/bluelib/loader/animation/keyframe/BoneAnimationFrame.java | Refactors from record to final class; consolidates queues into AnimationFrameVector. |
| common/src/main/java/software/bluelib/loader/animation/keyframe/AnimationPoint.java | Deletes old interpolation record. |
| common/src/main/java/software/bluelib/loader/animation/AnimationState.java | Introduces snapshot/extra-data composition, stricter controller/tick handling, updated API. |
| common/src/main/java/software/bluelib/loader/animation/AnimationSnapshot.java | Adds immutable snapshot record for limb/movement inputs. |
| common/src/main/java/software/bluelib/loader/animation/AnimationProcessor.java | Updates animation queue building + bone interpolation processing for new types. |
| common/src/main/java/software/bluelib/loader/animation/AnimationExtraData.java | Adds helper wrapper around DataTicket map with null-removal behavior. |
| common/src/main/java/software/bluelib/loader/animation/AnimationController.java | Updates controller logic to use new keyframe/queue types and Frame naming. |
| common/src/main/java/software/bluelib/loader/animation/Animation.java | Renames stages→frames, adds validation, returns unmodifiable frames list, improves equals/hash/toString. |
| common/src/main/java/software/bluelib/loader/animatable/base/ContextAwareAnimatableManager.java | Updates snapshot collection type to BoneFrame. |
| common/src/main/java/software/bluelib/loader/animatable/base/AnimatableManager.java | Updates snapshot collection type to BoneFrame. |
| common/src/main/java/software/bluelib/internal/registry/BlueNetworkRegistry.java | Registers new SetVariantPacket in S2C packet provider list. |
| common/src/main/java/software/bluelib/config/LoggerConfig.java | Changes default logging flags to enabled. |
| common/src/main/java/software/bluelib/client/net/variant/SetVariantPacketHandler.java | Adds client handler applying synced variant to living entities. |
| common/src/main/java/software/bluelib/client/internal/registry/BlueClientNetworkRegistry.java | Registers new S2C packet with handler on client side. |
| common/src/main/java/software/bluelib/api/entity/variant/IVariantEntity.java | Sends new S2C packet when variant changes. |
| common/src/main/java/software/bluelib/BlueLibConstants.java | Bumps library version constant to 2.4.4. |
| changelog.md | Adds detailed 2.4.4 changelog entry for refactor + packet addition. |
| build.gradle.kts | Updates GitHub release tag/name to 2.4.4. |
| .gitignore | Ignores Fabric output directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if (stage.animationName() == Animation.Stage.WAIT) { | ||
| animationCache = AnimationCache.generateWaitAnimation(stage.additionalTicks()); | ||
| if (frame.animationName() == Animation.Frame.WAIT) { |
There was a problem hiding this comment.
frame.animationName() == Animation.Frame.WAIT uses reference equality for a String, so WAIT frames may not be detected unless the string is interned. Use Animation.Frame.WAIT.equals(frame.animationName()) or the new frame.isWait() helper instead.
| if (frame.animationName() == Animation.Frame.WAIT) { | |
| if (Animation.Frame.WAIT.equals(frame.animationName())) { |
| @NotNull | ||
| public AnimationController<T> getController() { | ||
| if (this.controller == null) { | ||
| throw new IllegalStateException("AnimationController has not been set"); | ||
| } | ||
| return this.controller; | ||
| } |
There was a problem hiding this comment.
getController() now throws when the controller hasn’t been set, but AnimationState is used during preAnimationSetup/MoLang query evaluation before withController(...) runs (e.g., controller speed queries). This can cause runtime crashes. Consider keeping a nullable accessor (e.g., @Nullable getControllerOrNull()/peekController()), or changing getController() to return nullable and introducing a separate requireController() for call sites that must have one, then update MoLang/query call sites to use hasController()/optional access.
| T entity = getEntity(); | ||
| int id = entity.getId(); | ||
| SetVariantPacket packet = new SetVariantPacket(id, pVariantName); | ||
| NetworkRegistry.sendToAllPlayers(entity.getServer(), packet); |
There was a problem hiding this comment.
entity.getServer() can be null (e.g., when called on the client), which would cause an NPE when calling NetworkRegistry.sendToAllPlayers(...). Also, sending to all players is broader than necessary for an entity-scoped update. Guard against a null server and prefer sending only to players tracking the entity (e.g., via sendToAllPlayersTrackingEntity).
| NetworkRegistry.sendToAllPlayers(entity.getServer(), packet); | |
| if (entity.getServer() != null) { | |
| NetworkRegistry.sendToAllPlayersTrackingEntity(entity, packet); | |
| } |
| // 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; |
There was a problem hiding this comment.
This flips both logging flags to true by default, which can significantly increase log volume (and potentially affect performance) for all consumers. The TODO directly above indicates BlueLib logging should remain disabled by default; either revert these defaults or document/configure an explicit opt-in path.
| @@ -48,7 +48,7 @@ public void removeController(@NotNull String pName) { | |||
| return getManagerForContext(getCurrentContext()).getAnimationControllers(); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
This method overrides AnimatableManager.getBoneSnapshotCollection; it is advisable to add an Override annotation.
| @Override |
No description provided.