Skip to content
Merged

Fix #216

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,14 @@ public boolean isNewNMSVersion() {
return version >= V_1_17.version;
}

public boolean is1_11OrNewer() {
return version >= V_1_11.version;
}

public boolean is1_12OrNewer() {
return version >= V_1_12.version;
}

public boolean isNewItemStackAPI() {
return version >= V_1_21.version;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ public void onPacketSend(PacketSendEvent event) {
}
};

if (Bukkit.isPrimaryThread() || this.plugin.isEnabled()) {
if (Bukkit.isPrimaryThread()) {
task.run();
} else {
} else if (this.plugin.isEnabled()) {
this.plugin.getScheduler().runNextTick(w -> task.run());
}
Comment on lines +93 to 97
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When onPacketSend is invoked off the primary thread, this branch schedules task but the method still reads/writes playerAnimationData (a plain HashMap) on the packet thread while task mutates it on the Bukkit thread. This is a data race. Use a ConcurrentHashMap (and ensure PlayerAnimationData updates are thread-safe) or move all access to playerAnimationData into the scheduled main-thread task so it’s single-threaded.

Copilot uses AI. Check for mistakes.
} else if (packetType == PacketType.Play.Server.CLOSE_WINDOW){
Expand All @@ -110,9 +110,9 @@ public void onPacketSend(PacketSendEvent event) {
}
};

if (Bukkit.isPrimaryThread() || this.plugin.isEnabled()) {
if (Bukkit.isPrimaryThread()) {
task.run();
} else {
} else if (this.plugin.isEnabled()) {
this.plugin.getScheduler().runNextTick(w -> task.run());
}
} else if (packetType == PacketType.Play.Server.WINDOW_ITEMS){
Expand Down Expand Up @@ -148,9 +148,9 @@ public void onPacketSend(PacketSendEvent event) {
}
};

if (Bukkit.isPrimaryThread() || !this.plugin.isEnabled()) {
if (Bukkit.isPrimaryThread()) {
task.run();
} else {
} else if (this.plugin.isEnabled()) {
this.plugin.getScheduler().runNextTick(w -> task.run());
}
Comment on lines +151 to 155
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the WINDOW_ITEMS case, event.setCancelled(true) happens inside task. If onPacketSend is called off-thread (which this code path handles), cancelling on the next tick is typically too late because the packet may already have been sent and the event object may no longer be valid. Cancel the event synchronously before scheduling (e.g., by storing a “shouldHideItems” flag when the title animation starts and checking it immediately here), and use the scheduled task only for Bukkit/main-thread state updates.

Copilot uses AI. Check for mistakes.
}
Expand Down
53 changes: 39 additions & 14 deletions src/main/java/fr/maxlego08/menu/ZComponentsManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import fr.maxlego08.menu.itemstack.components.spigot.SpigotVariantComponent;
import fr.maxlego08.menu.loader.components.paper.*;
import fr.maxlego08.menu.loader.components.spigot.*;
import fr.maxlego08.menu.zcore.logger.Logger;
import org.jetbrains.annotations.NotNull;

import java.util.HashMap;
Expand All @@ -28,7 +29,14 @@ private boolean isPaperAndMiniMessageEnabled(MenuPlugin plugin){
public void initializeDefaultComponents(MenuPlugin plugin) {
NmsVersion currentVersion = NmsVersion.getCurrentVersion();
if (currentVersion.isAttributItemStack()){ // 1.20.5+
this.initializeVariantComponents(plugin);
try {
this.initializeVariantComponents(plugin);
} catch (Exception e) {
if (Configuration.enableDebug) {
Logger.info("Failed to initialize variant item components:");
e.printStackTrace();
}
Comment on lines +34 to +38
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exception handling here only logs when Configuration.enableDebug is true, so production failures during variant component initialization become silent and can leave the plugin partially initialized with no indication why. Consider always logging at least a WARNING/ERROR (and use the project Logger rather than printStackTrace()), and if the intent is to guard against version/linkage problems, catch Throwable (or at minimum LinkageError) so missing API symbols don’t still crash startup.

Copilot uses AI. Check for mistakes.
}

this.registerComponent(new SpigotBlockStateItemComponentLoader());
this.registerComponent(new SpigotAttributeModifiersItemComponentLoader(plugin));
Expand Down Expand Up @@ -131,32 +139,49 @@ public void initializeDefaultComponents(MenuPlugin plugin) {
}

private void initializeVariantComponents(MenuPlugin plugin) {
NmsVersion currentVersion = NmsVersion.getCurrentVersion();
VariantItemComponentLoaderFactory loaderFactory =
plugin.isPaper() ? new PaperVariantItemComponentLoader(new PaperVariantComponent())
: new SpigotVariantItemComponentLoader(new SpigotVariantComponent());

this.registerComponent(loaderFactory.getLoaderAxolotl());
this.registerComponent(loaderFactory.getLoaderCatCollar());
this.registerComponent(loaderFactory.getLoaderCatVariant());
this.registerComponent(loaderFactory.getLoaderChicken());
this.registerComponent(loaderFactory.getLoaderCow());
this.registerComponent(loaderFactory.getLoaderFox());
this.registerComponent(loaderFactory.getLoaderFrog());
this.registerComponent(loaderFactory.getLoaderHorse());
this.registerComponent(loaderFactory.getLoaderLlama());
this.registerComponent(loaderFactory.getLoaderMushroomCow());
this.registerComponent(loaderFactory.getLoaderPainting());
this.registerComponent(loaderFactory.getLoaderParrot());
this.registerComponent(loaderFactory.getLoaderPig());
this.registerComponent(loaderFactory.getLoaderRabbit());
this.registerComponent(loaderFactory.getLoaderSalmon());
this.registerComponent(loaderFactory.getLoaderSheep());
this.registerComponent(loaderFactory.getLoaderShulkerBox());
this.registerComponent(loaderFactory.getLoaderTropicalFishBaseColor());
this.registerComponent(loaderFactory.getLoaderTropicalFishPatternColor());
this.registerComponent(loaderFactory.getLoaderVillager());
this.registerComponent(loaderFactory.getLoaderWolfCollar());
this.registerComponent(loaderFactory.getLoaderWolfVariant());

if (currentVersion.isNewMaterial()){ // 1.13+
this.registerComponent(loaderFactory.getLoaderFox());
this.registerComponent(loaderFactory.getLoaderMushroomCow());
}
if (currentVersion.isNewNMSVersion()){ // 1.17+
this.registerComponent(loaderFactory.getLoaderAxolotl());
}
if (currentVersion.isAttributItemStack()){ // 1.20.5+
this.registerComponent(loaderFactory.getLoaderWolfCollar());
this.registerComponent(loaderFactory.getLoaderWolfVariant());
this.registerComponent(loaderFactory.getLoaderPainting());
}
if (currentVersion.is1_21_5OrNewer()){ // 1.21.5+
this.registerComponent(loaderFactory.getLoaderChicken());
this.registerComponent(loaderFactory.getLoaderCow());
this.registerComponent(loaderFactory.getLoaderPig());
}
if (currentVersion.isNewNBTVersion()) { // 1.18+
this.registerComponent(loaderFactory.getLoaderFrog());
}
if (currentVersion.is1_11OrNewer()){ // 1.11+
this.registerComponent(loaderFactory.getLoaderLlama());
this.registerComponent(loaderFactory.getLoaderShulkerBox());
}
if (currentVersion.is1_12OrNewer()) { // 1.12+
this.registerComponent(loaderFactory.getLoaderParrot());
}
Comment on lines +156 to +183
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initializeVariantComponents() is only called from the isAttributItemStack() (1.20.5+) branch, so version gates like isNewMaterial() / isNewNMSVersion() are redundant as written and can mislead future maintenance. Either remove the redundant checks, or move version gating fully into this method and call it unconditionally (ensuring incompatible loaders are never instantiated on older versions).

Suggested change
if (currentVersion.isNewMaterial()){ // 1.13+
this.registerComponent(loaderFactory.getLoaderFox());
this.registerComponent(loaderFactory.getLoaderMushroomCow());
}
if (currentVersion.isNewNMSVersion()){ // 1.17+
this.registerComponent(loaderFactory.getLoaderAxolotl());
}
if (currentVersion.isAttributItemStack()){ // 1.20.5+
this.registerComponent(loaderFactory.getLoaderWolfCollar());
this.registerComponent(loaderFactory.getLoaderWolfVariant());
this.registerComponent(loaderFactory.getLoaderPainting());
}
if (currentVersion.is1_21_5OrNewer()){ // 1.21.5+
this.registerComponent(loaderFactory.getLoaderChicken());
this.registerComponent(loaderFactory.getLoaderCow());
this.registerComponent(loaderFactory.getLoaderPig());
}
if (currentVersion.isNewNBTVersion()) { // 1.18+
this.registerComponent(loaderFactory.getLoaderFrog());
}
if (currentVersion.is1_11OrNewer()){ // 1.11+
this.registerComponent(loaderFactory.getLoaderLlama());
this.registerComponent(loaderFactory.getLoaderShulkerBox());
}
if (currentVersion.is1_12OrNewer()) { // 1.12+
this.registerComponent(loaderFactory.getLoaderParrot());
}
this.registerComponent(loaderFactory.getLoaderFox());
this.registerComponent(loaderFactory.getLoaderMushroomCow());
this.registerComponent(loaderFactory.getLoaderAxolotl());
this.registerComponent(loaderFactory.getLoaderWolfCollar());
this.registerComponent(loaderFactory.getLoaderWolfVariant());
this.registerComponent(loaderFactory.getLoaderPainting());
this.registerComponent(loaderFactory.getLoaderFrog());
this.registerComponent(loaderFactory.getLoaderLlama());
this.registerComponent(loaderFactory.getLoaderShulkerBox());
this.registerComponent(loaderFactory.getLoaderParrot());
if (currentVersion.is1_21_5OrNewer()){ // 1.21.5+
this.registerComponent(loaderFactory.getLoaderChicken());
this.registerComponent(loaderFactory.getLoaderCow());
this.registerComponent(loaderFactory.getLoaderPig());
}

Copilot uses AI. Check for mistakes.

}


Expand Down
Loading