Skip to content

Commit f723b9e

Browse files
committed
Fix animation and tile lifecycle regressions
1 parent e25627e commit f723b9e

4 files changed

Lines changed: 362 additions & 53 deletions

File tree

common/src/main/java/com/loohp/interactionvisualizer/managers/DisplayManager.java

Lines changed: 103 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import java.util.LinkedHashSet;
5757
import java.util.List;
5858
import java.util.Map;
59+
import java.util.Objects;
5960
import java.util.Set;
6061
import java.util.UUID;
6162
import java.util.concurrent.ConcurrentHashMap;
@@ -489,11 +490,11 @@ private static void syncItem(Item logical) {
489490
boolean packetOnly = qualifiesForPacketOnlyStatic(logical);
490491
boolean wasPacketOnly = packetOnlyItems.contains(logical);
491492
org.bukkit.entity.Entity current = logical.getBukkitEntity().orElse(null);
493+
Location logicalLocation = logical.getLocation();
492494

493495
if (packetOnly) {
494496
PerformanceMetrics.packetOnlyItemSync();
495497
ItemStack itemStack = logical.getItemStack();
496-
Location location = logical.getLocation();
497498

498499
if (current != null) {
499500
discardActual(logical, current);
@@ -508,9 +509,10 @@ private static void syncItem(Item logical) {
508509
// Visualizer Item getters already return defensive copies. Retain those
509510
// snapshots directly instead of cloning them a second time on every sync.
510511
ItemStack previousItemStack = renderedItemStacks.put(logical, itemStack);
511-
Location previousLocation = renderedItemLocations.put(logical, location);
512+
Location previousLocation = renderedItemLocations.put(logical, logicalLocation);
512513
boolean itemChanged = previousItemStack == null || !previousItemStack.equals(itemStack);
513-
boolean locationChanged = previousLocation == null || !previousLocation.equals(location);
514+
boolean locationChanged = previousLocation == null || !previousLocation.equals(logicalLocation);
515+
index(logical, logicalLocation);
514516
if (wasPacketOnly && (itemChanged || locationChanged)) {
515517
respawnVirtualItems(logical);
516518
}
@@ -526,7 +528,7 @@ private static void syncItem(Item logical) {
526528
untrackActual(logical);
527529
}
528530

529-
if (current != null && (!current.getWorld().equals(logical.getWorld())
531+
if (current != null && (!current.getWorld().equals(logicalLocation.getWorld())
530532
|| !(current instanceof ItemDisplay))) {
531533
discardActual(logical, current);
532534
current = null;
@@ -537,7 +539,7 @@ private static void syncItem(Item logical) {
537539
actual = display;
538540
} else {
539541
clearViewerTracking(logical);
540-
actual = logical.getWorld().spawn(logical.getLocation(), ItemDisplay.class,
542+
actual = logicalLocation.getWorld().spawn(logicalLocation, ItemDisplay.class,
541543
DisplayManager::initializeDisplay);
542544
PerformanceMetrics.bukkitEntitySpawn();
543545
logical.bind(actual);
@@ -560,39 +562,54 @@ private static void syncItem(Item logical) {
560562
actual.setInterpolationDelay(0);
561563
actual.setInterpolationDuration(0);
562564
actual.setTeleportDuration(0);
563-
boolean anchorMoved = applyItemBase(actual, logical);
564565

565566
Vector velocity = logical.getVelocity();
566-
boolean animated = requiresItemAnimation(logical.hasGravity(), velocity);
567+
boolean gravity = logical.hasGravity();
568+
boolean animated = requiresItemAnimation(gravity, velocity);
567569
ItemAnimationState previousAnimation = itemAnimations.get(logical);
570+
Location previousPosition = previousAnimation == null ? null : previousAnimation.position;
571+
Location previousLogicalLocation = previousAnimation == null ? null : previousAnimation.logicalLocation;
572+
boolean applyLogicalLocation = shouldApplyItemLogicalLocation(
573+
animated, previousPosition, previousLogicalLocation, logicalLocation);
574+
boolean anchorMoved = applyItemBase(actual, logical, logicalLocation, applyLogicalLocation);
575+
576+
ItemAnimationState nextAnimation = null;
568577
if (animated) {
569-
Location position = previousAnimation == null ? actual.getLocation() : previousAnimation.position.clone();
570-
itemAnimations.put(logical, new ItemAnimationState(velocity, logical.hasGravity(), position,
578+
Location position = itemAnimationStartPosition(
579+
actual.getLocation(), previousPosition, previousLogicalLocation, logicalLocation);
580+
nextAnimation = new ItemAnimationState(velocity, gravity, position,
571581
useStaticAnchorForAnimation(InteractionVisualizer.staticVirtualItemAnchorsDuringAnimation,
572-
logical.isCustomNameVisible())));
582+
logical.isCustomNameVisible()), logicalLocation);
583+
itemAnimations.put(logical, nextAnimation);
573584
scheduleItemAnimationTick();
574585
} else {
575586
itemAnimations.remove(logical);
576587
}
577588

589+
Location anchorLocation = actual.getLocation();
590+
Location visualLocation = itemAnimationIndexLocation(
591+
nextAnimation != null && nextAnimation.staticAnchor,
592+
anchorLocation, nextAnimation == null ? null : nextAnimation.position);
593+
index(logical, visualLocation);
594+
578595
if (itemChanged) {
579596
respawnVirtualItems(logical);
580597
} else if (requiresVirtualItemMotionSync(anchorMoved, previousAnimation != null, animated)) {
581-
synchronizeVirtualItemMotion(logical, actual.getLocation());
598+
synchronizeVirtualItemMotion(logical, visualLocation);
582599
}
583600
}
584601

585602
static boolean requiresVirtualItemMotionSync(boolean anchorMoved, boolean wasAnimated, boolean animated) {
586603
return anchorMoved || wasAnimated || animated;
587604
}
588605

589-
private static boolean applyItemBase(org.bukkit.entity.Entity actual, Item logical) {
606+
private static boolean applyItemBase(org.bukkit.entity.Entity actual, Item logical,
607+
Location target, boolean applyLocation) {
590608
actual.setGlowing(logical.isGlowing());
591609
actual.customName(logical.getCustomName());
592610
actual.setCustomNameVisible(logical.isCustomNameVisible());
593-
Location target = logical.getLocation();
594-
boolean moved = !actual.getWorld().equals(target.getWorld())
595-
|| actual.getLocation().distanceSquared(target) > 1.0E-8;
611+
boolean moved = applyLocation && (!actual.getWorld().equals(target.getWorld())
612+
|| actual.getLocation().distanceSquared(target) > 1.0E-8);
596613
if (moved) {
597614
PerformanceMetrics.bukkitEntityTeleport();
598615
actual.teleport(target);
@@ -676,6 +693,43 @@ static boolean requiresItemAnimation(boolean gravity, Vector velocity) {
676693
return gravity || velocity.lengthSquared() > ITEM_ANIMATION_EPSILON;
677694
}
678695

696+
static boolean itemAnimationLogicalLocationChanged(Location previousLogicalLocation,
697+
Location logicalLocation) {
698+
if (previousLogicalLocation == null || logicalLocation == null) {
699+
return true;
700+
}
701+
return !Objects.equals(previousLogicalLocation.getWorld(), logicalLocation.getWorld())
702+
|| previousLogicalLocation.getX() != logicalLocation.getX()
703+
|| previousLogicalLocation.getY() != logicalLocation.getY()
704+
|| previousLogicalLocation.getZ() != logicalLocation.getZ();
705+
}
706+
707+
static boolean shouldApplyItemLogicalLocation(boolean animated,
708+
Location previousAnimationPosition,
709+
Location previousLogicalLocation,
710+
Location logicalLocation) {
711+
return previousAnimationPosition == null || !animated
712+
|| itemAnimationLogicalLocationChanged(previousLogicalLocation, logicalLocation);
713+
}
714+
715+
static Location itemAnimationStartPosition(Location actualLocation,
716+
Location previousAnimationPosition,
717+
Location previousLogicalLocation,
718+
Location logicalLocation) {
719+
Objects.requireNonNull(actualLocation, "actualLocation");
720+
if (previousAnimationPosition == null
721+
|| itemAnimationLogicalLocationChanged(previousLogicalLocation, logicalLocation)) {
722+
return actualLocation.clone();
723+
}
724+
return previousAnimationPosition.clone();
725+
}
726+
727+
static Location itemAnimationIndexLocation(boolean staticAnchor, Location anchorLocation,
728+
Location animationPosition) {
729+
Objects.requireNonNull(anchorLocation, "anchorLocation");
730+
return (staticAnchor || animationPosition == null ? anchorLocation : animationPosition).clone();
731+
}
732+
679733
static boolean useStaticAnchorForAnimation(boolean configured, boolean customNameVisible) {
680734
return configured && !customNameVisible;
681735
}
@@ -760,6 +814,11 @@ private static void tickItemAnimation(Item logical, ItemAnimationState animation
760814
return;
761815
}
762816
}
817+
boolean chunkChanged = index(logical, itemAnimationIndexLocation(
818+
animation.staticAnchor, actual.getLocation(), animation.position));
819+
if (chunkChanged) {
820+
reconcileViewers(logical);
821+
}
763822
if (animation.gravity) {
764823
// Heart's fake item is no-gravity. Absolute correction preserves the
765824
// vanilla gravity trajectory; no-gravity motion remains client-run.
@@ -773,6 +832,9 @@ private static void tickItemAnimation(Item logical, ItemAnimationState animation
773832
remove(null, logical, true);
774833
return;
775834
}
835+
if (index(logical, actual.getLocation())) {
836+
reconcileViewers(logical);
837+
}
776838
}
777839
synchronizeVirtualItemMotion(logical, destination);
778840
}
@@ -799,6 +861,7 @@ private static boolean spawnVirtualItem(Item logical, Player player) {
799861
org.bukkit.entity.Entity anchor = logical.getBukkitEntity().orElse(null);
800862
Set<UUID> shown = shownViewers.get(logical);
801863
boolean packetOnly = packetOnlyItems.contains(logical);
864+
ItemAnimationState animation = itemAnimations.get(logical);
802865
Location source;
803866
if (!active.containsKey(logical) || shown == null || !shown.contains(player.getUniqueId())
804867
|| !player.isOnline()) {
@@ -814,7 +877,7 @@ private static boolean spawnVirtualItem(Item logical, Player player) {
814877
if (!(anchor instanceof ItemDisplay) || !player.getWorld().equals(anchor.getWorld())) {
815878
return false;
816879
}
817-
source = anchor.getLocation();
880+
source = animation == null ? anchor.getLocation() : animation.position.clone();
818881
}
819882

820883
Map<UUID, Integer> ids = virtualItemIds.computeIfAbsent(logical, ignored -> new ConcurrentHashMap<>());
@@ -829,7 +892,6 @@ private static boolean spawnVirtualItem(Item logical, Player player) {
829892
player, logical.getItemStack(), source);
830893
PerformanceMetrics.virtualSpawnBundle();
831894
ids.put(viewer, spawnedId);
832-
ItemAnimationState animation = itemAnimations.get(logical);
833895
if (animation != null) {
834896
Vector motion = itemMovementForTick(animation.gravity, animation.velocity);
835897
if (motion.lengthSquared() > ITEM_ANIMATION_EPSILON) {
@@ -1054,19 +1116,34 @@ private static void untrackActual(VisualizerEntity logical) {
10541116
}
10551117

10561118
private static void index(VisualizerEntity logical) {
1057-
Location location = logical.getLocation();
1119+
if (logical instanceof Item item) {
1120+
ItemAnimationState animation = itemAnimations.get(item);
1121+
if (animation != null) {
1122+
Location anchorLocation = logical.getBukkitEntity()
1123+
.map(org.bukkit.entity.Entity::getLocation)
1124+
.orElseGet(logical::getLocation);
1125+
index(logical, itemAnimationIndexLocation(
1126+
animation.staticAnchor, anchorLocation, animation.position));
1127+
return;
1128+
}
1129+
}
1130+
index(logical, logical.getLocation());
1131+
}
1132+
1133+
static boolean index(VisualizerEntity logical, Location location) {
10581134
UUID world = location.getWorld().getUID();
10591135
int chunkX = location.getBlockX() >> 4;
10601136
int chunkZ = location.getBlockZ() >> 4;
10611137
ChunkKey previous = chunkByLogical.get(logical);
10621138
if (previous != null && previous.world().equals(world)
10631139
&& previous.x() == chunkX && previous.z() == chunkZ) {
1064-
return;
1140+
return false;
10651141
}
10661142
ChunkKey current = new ChunkKey(
10671143
world, chunkX, chunkZ);
10681144
previous = chunkByLogical.put(logical, current);
1069-
if (previous != null && !previous.equals(current)) {
1145+
boolean migrated = previous != null && !previous.equals(current);
1146+
if (migrated) {
10701147
Set<VisualizerEntity> previousEntries = logicalsByChunk.get(previous);
10711148
if (previousEntries != null) {
10721149
previousEntries.remove(logical);
@@ -1076,9 +1153,10 @@ private static void index(VisualizerEntity logical) {
10761153
}
10771154
}
10781155
logicalsByChunk.computeIfAbsent(current, ignored -> ConcurrentHashMap.newKeySet()).add(logical);
1156+
return migrated;
10791157
}
10801158

1081-
private static void unindex(VisualizerEntity logical) {
1159+
static void unindex(VisualizerEntity logical) {
10821160
ChunkKey key = chunkByLogical.remove(logical);
10831161
if (key == null) {
10841162
return;
@@ -1748,12 +1826,15 @@ private static final class ItemAnimationState {
17481826
private final boolean gravity;
17491827
private Location position;
17501828
private final boolean staticAnchor;
1829+
private final Location logicalLocation;
17511830

1752-
private ItemAnimationState(Vector velocity, boolean gravity, Location position, boolean staticAnchor) {
1831+
private ItemAnimationState(Vector velocity, boolean gravity, Location position,
1832+
boolean staticAnchor, Location logicalLocation) {
17531833
this.velocity = velocity.clone();
17541834
this.gravity = gravity;
17551835
this.position = position.clone();
17561836
this.staticAnchor = staticAnchor;
1837+
this.logicalLocation = logicalLocation.clone();
17571838
}
17581839
}
17591840

common/src/main/java/com/loohp/interactionvisualizer/managers/TileEntityManager.java

Lines changed: 55 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,18 @@
6969

7070
public class TileEntityManager implements Listener {
7171

72+
enum LifecycleChange {
73+
REMOVED,
74+
ADDED,
75+
ACTIVATED
76+
}
77+
78+
@FunctionalInterface
79+
interface LifecycleDispatcher<T> {
80+
81+
void dispatch(T value, LifecycleChange change, TileEntityType type);
82+
}
83+
7284
private static final Plugin plugin = InteractionVisualizer.plugin;
7385
private static final TileEntityType[] tileEntityTypes = TileEntityType.values();
7486
private static final Map<TileEntityType, Set<Block>> active = new EnumMap<>(TileEntityType.class);
@@ -185,16 +197,6 @@ private synchronized static void addTileEntities(ChunkPosition chunk) {
185197
Block block = state.getBlock();
186198
TileEntityType type = TileEntity.getTileEntityType(state.getType());
187199
if (type != null) {
188-
if (activate && active.get(type).add(block)) {
189-
if (InteractionVisualizer.eventDrivenBlockUpdates) {
190-
TileEntityType lastActiveType = lastActiveTypes.put(block, type);
191-
if (type.equals(lastActiveType)) {
192-
callActivatedEvent(block, type);
193-
} else {
194-
callAddedEvent(block, type);
195-
}
196-
}
197-
}
198200
newBlocks.put(block, type);
199201
blocks.add(block);
200202
}
@@ -205,23 +207,50 @@ private synchronized static void addTileEntities(ChunkPosition chunk) {
205207
TileEntityType type = newBlocks.get(block);
206208
if (type == null) {
207209
itr.remove();
208-
if (InteractionVisualizer.eventDrivenBlockUpdates) {
209-
lastActiveTypes.remove(block);
210-
}
211-
for (TileEntityType t : tileEntityTypes) {
212-
if (active.get(t).remove(block)) {
213-
Bukkit.getPluginManager().callEvent(new TileEntityRemovedEvent(block, t));
214-
}
215-
}
216-
} else {
217-
for (TileEntityType t : tileEntityTypes) {
218-
if (!t.equals(type)) {
219-
if (active.get(t).remove(block)) {
220-
Bukkit.getPluginManager().callEvent(new TileEntityRemovedEvent(block, t));
221-
}
222-
}
223-
}
224210
}
211+
reconcileActiveType(block, type, activate, InteractionVisualizer.eventDrivenBlockUpdates,
212+
active, lastActiveTypes, TileEntityManager::dispatchLifecycleChange);
213+
}
214+
}
215+
216+
static <T> void reconcileActiveType(T value, TileEntityType currentType, boolean activate,
217+
boolean trackLifecycleEvents,
218+
Map<TileEntityType, Set<T>> activeByType,
219+
Map<T, TileEntityType> lastActiveByValue,
220+
LifecycleDispatcher<T> dispatcher) {
221+
if (currentType == null && trackLifecycleEvents) {
222+
// Preserve the legacy removal contract for re-entrant listeners:
223+
// a removed tile is no longer considered last-active when notified.
224+
lastActiveByValue.remove(value);
225+
}
226+
for (TileEntityType type : tileEntityTypes) {
227+
if (type.equals(currentType)) {
228+
continue;
229+
}
230+
Set<T> values = activeByType.get(type);
231+
if (values != null && values.remove(value)) {
232+
dispatcher.dispatch(value, LifecycleChange.REMOVED, type);
233+
}
234+
}
235+
236+
if (currentType == null) {
237+
return;
238+
}
239+
240+
Set<T> currentValues = activeByType.get(currentType);
241+
if (!activate || currentValues == null || !currentValues.add(value) || !trackLifecycleEvents) {
242+
return;
243+
}
244+
TileEntityType lastActiveType = lastActiveByValue.put(value, currentType);
245+
dispatcher.dispatch(value, currentType.equals(lastActiveType)
246+
? LifecycleChange.ACTIVATED : LifecycleChange.ADDED, currentType);
247+
}
248+
249+
private static void dispatchLifecycleChange(Block block, LifecycleChange change, TileEntityType type) {
250+
switch (change) {
251+
case REMOVED -> Bukkit.getPluginManager().callEvent(new TileEntityRemovedEvent(block, type));
252+
case ADDED -> callAddedEvent(block, type);
253+
case ACTIVATED -> callActivatedEvent(block, type);
225254
}
226255
}
227256

0 commit comments

Comments
 (0)