Skip to content
Merged
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 @@ -219,6 +219,10 @@ private void registerAllManagers() {
com.pedrodalben.bigbangessentials.holograms.service.BigBangHologramsManager.class,
com.pedrodalben.bigbangessentials.holograms.service.BigBangHologramsManager::getInstance);

registry.registerManager("NpcManager", "npcs",
com.pedrodalben.bigbangessentials.npcs.service.NpcManager.class,
com.pedrodalben.bigbangessentials.npcs.service.NpcManager::getInstance);

// RankUp Manager
registry.registerManager("RankupManager", "rankup",
com.pedrodalben.bigbangessentials.rankup.RankupManager.class);
Expand All @@ -240,6 +244,7 @@ private void registerModules() {
modules.register("rankup", () -> com.pedrodalben.bigbangessentials.config.ConfigManager.isModuleEnabled("rankup"), "economy", "database");
modules.register("crates", () -> com.pedrodalben.bigbangessentials.config.ConfigManager.isModuleEnabled("crates"), "database");
modules.register("holograms", () -> com.pedrodalben.bigbangessentials.config.ConfigManager.isModuleEnabled("holograms"));
modules.register("npcs", () -> com.pedrodalben.bigbangessentials.config.ConfigManager.isModuleEnabled("npcs"), "holograms");
modules.register("shop", () -> com.pedrodalben.bigbangessentials.config.ConfigManager.isModuleEnabled("shop"), "economy", "database");
modules.register("adminshop", () -> com.pedrodalben.bigbangessentials.config.ConfigManager.isModuleEnabled("adminshop"), "economy", "database");
modules.register("cobblemon", com.pedrodalben.bigbangessentials.pokemarket.PokeMarketManager::isCobblemonPresent);
Expand Down Expand Up @@ -439,6 +444,18 @@ public static void onServerStarting(net.minecraft.server.MinecraftServer server)
ModuleManager.getInstance().failed("holograms", e);
}

if (ModuleManager.getInstance().prepare("npcs")) try {
LOGGER.info("⚙ Initializing NPC Module...");
com.pedrodalben.bigbangessentials.npcs.service.NpcManager.getInstance().initialize();
ManagerRegistry.getInstance().markInitialized("NpcManager");
ModuleManager.getInstance().started("npcs", 0);
LOGGER.info("✓ NPC Module initialized successfully");
} catch (Exception e) {
LOGGER.error("✗ NPC Module failed to initialize: {}", e.getMessage(), e);
ManagerRegistry.getInstance().markFailed("NpcManager", e.getMessage());
ModuleManager.getInstance().failed("npcs", e);
}

// Initialize FakePlayerIntegration
try {
LOGGER.info("⚙ Initializing FakePlayer Integration...");
Expand Down Expand Up @@ -640,6 +657,10 @@ public static void onPlayerLoggedIn(net.minecraft.server.level.ServerPlayer play
}
}
}

if (ModuleManager.getInstance().isActive("npcs")) {
com.pedrodalben.bigbangessentials.npcs.service.NpcManager.getInstance().onPlayerJoin(player);
}
}

public static void onPlayerLoggedOut(net.minecraft.server.level.ServerPlayer player) {
Expand All @@ -654,6 +675,10 @@ public static void onPlayerLoggedOut(net.minecraft.server.level.ServerPlayer pla
} catch (Exception e) {
LOGGER.debug("Failed to persist database-backed player state for {}: {}", player.getName().getString(), e.getMessage(), e);
}

if (ModuleManager.getInstance().isActive("npcs")) {
com.pedrodalben.bigbangessentials.npcs.service.NpcManager.getInstance().onPlayerLeave(player);
}
}

public static void onServerStopping(net.minecraft.server.MinecraftServer server) {
Expand Down Expand Up @@ -783,6 +808,13 @@ public static void onServerStopping(net.minecraft.server.MinecraftServer server)
LOGGER.error("Failed to shutdown BigBangHolograms", e);
}

if (ModuleManager.getInstance().isActive("npcs")) try {
LOGGER.info("Shutting down NPC Module...");
com.pedrodalben.bigbangessentials.npcs.service.NpcManager.getInstance().shutdown();
} catch (Exception e) {
LOGGER.error("Failed to shutdown NPC Module", e);
}

Comment on lines +811 to +817

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Shut down the NPC module before the holograms module.

registerModules declares npcs as dependent on holograms (line 247). Shutdown runs in the opposite order: holograms stop at lines 804-809, then NPCs at lines 811-816. NpcManager.shutdown does not call the hologram API today, so no failure occurs now. Move the NPC block above the holograms block to keep shutdown order the reverse of dependency order and to prevent breakage if NPC shutdown later touches holograms.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@common/src/main/java/com/pedrodalben/bigbangessentials/BigBangEssentials.java`
around lines 811 - 817, Move the NPC shutdown block identified by
NpcManager.getInstance().shutdown() above the holograms shutdown block in the
shutdown sequence. Preserve its active-module check, logging, and exception
handling, so dependents stop before the holograms module they rely on.

// Shutdown FakeTpaManager
try {
LOGGER.info("Shutting down Fake TPA Manager...");
Expand Down Expand Up @@ -1378,6 +1410,11 @@ private static void registerAllCommands(CommandDispatcher<CommandSourceStack> di
com.pedrodalben.bigbangessentials.holograms.command.HologramCommand.register(dispatcher);
}

if (ModuleManager.getInstance().isActive("npcs")) {
registry.registerCommand("npc", "Manage virtual NPCs", "npcs");
com.pedrodalben.bigbangessentials.npcs.command.NpcCommand.register(dispatcher);
}

// ========== TABLIST MODULE ==========
if (ModuleManager.getInstance().isActive("tablist")) {
registry.registerCommand("tablist", "Manage tablist configuration, reload, debug");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.pedrodalben.bigbangessentials.npcs.api;

import com.pedrodalben.bigbangessentials.npcs.service.NpcManager;

public final class BigBangNpcs {
private BigBangNpcs() {}

public static NpcService getApi() {
return NpcManager.getInstance();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package com.pedrodalben.bigbangessentials.npcs.api;

import java.util.Objects;

public final class NpcAction {
private final NpcActionType type;
private final String command;

public NpcAction(NpcActionType type, String command) {
this.type = type != null ? type : NpcActionType.NONE;
this.command = command != null ? command.trim() : "";
}
Comment on lines +9 to +12

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Normalize commands in the public constructor.

The constructor preserves a leading /, but playerCommand and consoleCommand remove it. A persistence loader or external caller can create a command with a different format from the factory output.

Proposed fix
-        this.command = command != null ? command.trim() : "";
+        this.command = stripSlash(command);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@common/src/main/java/com/pedrodalben/bigbangessentials/npcs/api/NpcAction.java`
around lines 9 - 12, Update the public NpcAction constructor to normalize
command strings the same way as playerCommand and consoleCommand, including
removing any leading “/” after trimming. Preserve the existing null-to-empty
behavior and ensure constructor-created actions match factory output.


public static NpcAction none() {
return new NpcAction(NpcActionType.NONE, "");
}

public static NpcAction playerCommand(String command) {
return new NpcAction(NpcActionType.PLAYER_COMMAND, stripSlash(command));
}

public static NpcAction consoleCommand(String command) {
return new NpcAction(NpcActionType.CONSOLE_COMMAND, stripSlash(command));
}

private static String stripSlash(String cmd) {
if (cmd == null) return "";
String trimmed = cmd.trim();
return trimmed.startsWith("/") ? trimmed.substring(1).trim() : trimmed;
}

public NpcActionType type() { return type; }
public String command() { return command; }

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof NpcAction that)) return false;
return type == that.type && command.equals(that.command);
}

@Override
public int hashCode() {
return Objects.hash(type, command);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.pedrodalben.bigbangessentials.npcs.api;

public enum NpcActionType {
NONE,
PLAYER_COMMAND,
CONSOLE_COMMAND
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package com.pedrodalben.bigbangessentials.npcs.api;

import java.util.Objects;
import java.util.regex.Pattern;

public final class NpcDefinition {
private static final Pattern VALID_ID = Pattern.compile("^[a-z0-9_-]{1,64}$");

private final String id;
private final boolean enabled;
private final String displayName;
private final NpcLocation location;
private final NpcSkin skin;
private final NpcAction action;
private final NpcHologramConfig hologram;
private final NpcLookSettings lookSettings;
private final double viewDistance;
private final double despawnDistance;
private final NpcInteractionConfig interaction;

public NpcDefinition(String id, boolean enabled, String displayName, NpcLocation location,
NpcSkin skin, NpcAction action, NpcHologramConfig hologram,
NpcLookSettings lookSettings, double viewDistance, double despawnDistance,
NpcInteractionConfig interaction) {
this.id = normalizeId(id);
this.enabled = enabled;
this.displayName = displayName != null ? displayName : id;
this.location = Objects.requireNonNull(location, "location");
this.skin = Objects.requireNonNull(skin, "skin");
this.action = action != null ? action : NpcAction.none();
this.hologram = hologram != null ? hologram : NpcHologramConfig.disabled();
this.lookSettings = lookSettings != null ? lookSettings : NpcLookSettings.disabled();
this.viewDistance = Math.max(1.0, viewDistance);
this.despawnDistance = Math.max(viewDistance, despawnDistance);
Comment on lines +33 to +34

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep despawnDistance at or above the normalized view distance.

When viewDistance is less than 1.0, Line 34 compares against the raw value. For example, inputs of 0.0 and 0.0 create viewDistance == 1.0 and despawnDistance == 0.0.

Proposed fix
         this.viewDistance = Math.max(1.0, viewDistance);
-        this.despawnDistance = Math.max(viewDistance, despawnDistance);
+        this.despawnDistance = Math.max(this.viewDistance, despawnDistance);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this.viewDistance = Math.max(1.0, viewDistance);
this.despawnDistance = Math.max(viewDistance, despawnDistance);
this.viewDistance = Math.max(1.0, viewDistance);
this.despawnDistance = Math.max(this.viewDistance, despawnDistance);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@common/src/main/java/com/pedrodalben/bigbangessentials/npcs/api/NpcDefinition.java`
around lines 33 - 34, Update the constructor normalization in NpcDefinition so
despawnDistance is clamped against the normalized this.viewDistance rather than
the raw viewDistance parameter. Preserve the existing minimum viewDistance of
1.0 and ensure despawnDistance never falls below it.

this.interaction = interaction != null ? interaction : NpcInteractionConfig.defaults();
}

public static String normalizeId(String id) {
if (id == null) throw new IllegalArgumentException("NPC id cannot be null");
String normalized = id.trim().toLowerCase();
if (!VALID_ID.matcher(normalized).matches()) {
throw new IllegalArgumentException("Invalid NPC id: '" + id + "'. Must match [a-z0-9_-]{1,64}");
}
return normalized;
Comment on lines +38 to +44

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use locale-independent ID normalization.

String.toLowerCase() uses the server default locale. Under a Turkish locale, an input such as I becomes ı and fails the ASCII ID pattern.

Proposed fix
 import java.util.Objects;
+import java.util.Locale;
 import java.util.regex.Pattern;
 
-        String normalized = id.trim().toLowerCase();
+        String normalized = id.trim().toLowerCase(Locale.ROOT);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@common/src/main/java/com/pedrodalben/bigbangessentials/npcs/api/NpcDefinition.java`
around lines 38 - 44, Update NpcDefinition.normalizeId to lowercase IDs with a
locale-independent mechanism, such as an explicitly specified locale, while
preserving the existing trimming, validation, exception behavior, and normalized
return value.

}

public String id() { return id; }
public boolean enabled() { return enabled; }
public String displayName() { return displayName; }
public NpcLocation location() { return location; }
public NpcSkin skin() { return skin; }
public NpcAction action() { return action; }
public NpcHologramConfig hologram() { return hologram; }
public NpcLookSettings lookSettings() { return lookSettings; }
public double viewDistance() { return viewDistance; }
public double despawnDistance() { return despawnDistance; }
public NpcInteractionConfig interaction() { return interaction; }

public NpcDefinition withEnabled(boolean enabled) {
return new NpcDefinition(id, enabled, displayName, location, skin, action, hologram,
lookSettings, viewDistance, despawnDistance, interaction);
}

public NpcDefinition withLocation(NpcLocation newLocation) {
return new NpcDefinition(id, enabled, displayName, newLocation, skin, action, hologram,
lookSettings, viewDistance, despawnDistance, interaction);
}

public NpcDefinition withSkin(NpcSkin newSkin) {
return new NpcDefinition(id, enabled, displayName, location, newSkin, action, hologram,
lookSettings, viewDistance, despawnDistance, interaction);
}

public NpcDefinition withAction(NpcAction newAction) {
return new NpcDefinition(id, enabled, displayName, location, skin, newAction, hologram,
lookSettings, viewDistance, despawnDistance, interaction);
}

public NpcDefinition withDisplayName(String newDisplayName) {
return new NpcDefinition(id, enabled, newDisplayName, location, skin, action, hologram,
lookSettings, viewDistance, despawnDistance, interaction);
}

public NpcDefinition withHologram(NpcHologramConfig newHologram) {
return new NpcDefinition(id, enabled, displayName, location, skin, action, newHologram,
lookSettings, viewDistance, despawnDistance, interaction);
}

public NpcDefinition withLookSettings(NpcLookSettings newLook) {
return new NpcDefinition(id, enabled, displayName, location, skin, action, hologram,
newLook, viewDistance, despawnDistance, interaction);
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof NpcDefinition that)) return false;
return id.equals(that.id);
}

@Override
public int hashCode() {
return id.hashCode();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package com.pedrodalben.bigbangessentials.npcs.api;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

public final class NpcHologramConfig {
private final boolean enabled;
private final List<String> lines;
private final double offsetY;
private final double viewDistance;
private final boolean shadow;
private final boolean seeThrough;

public NpcHologramConfig(boolean enabled, List<String> lines, double offsetY, double viewDistance, boolean shadow, boolean seeThrough) {
this.enabled = enabled;
this.lines = Collections.unmodifiableList(new ArrayList<>(lines != null ? lines : List.of()));
this.offsetY = Math.max(0.0, offsetY);
this.viewDistance = Math.max(1.0, viewDistance);
this.shadow = shadow;
this.seeThrough = seeThrough;
}
Comment on lines +15 to +22

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not allow an enabled hologram with no lines.

This constructor permits an enabled empty configuration. NpcHologramService.createOrUpdate in common/src/main/java/com/pedrodalben/bigbangessentials/npcs/hologram/NpcHologramService.java:22-65 returns without deleting an existing hologram for that state. Normalize this state to disabled, or reject it.

Proposed fix
-        this.enabled = enabled;
         this.lines = Collections.unmodifiableList(new ArrayList<>(lines != null ? lines : List.of()));
+        this.enabled = enabled && !this.lines.isEmpty();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public NpcHologramConfig(boolean enabled, List<String> lines, double offsetY, double viewDistance, boolean shadow, boolean seeThrough) {
this.enabled = enabled;
this.lines = Collections.unmodifiableList(new ArrayList<>(lines != null ? lines : List.of()));
this.offsetY = Math.max(0.0, offsetY);
this.viewDistance = Math.max(1.0, viewDistance);
this.shadow = shadow;
this.seeThrough = seeThrough;
}
public NpcHologramConfig(boolean enabled, List<String> lines, double offsetY, double viewDistance, boolean shadow, boolean seeThrough) {
this.lines = Collections.unmodifiableList(new ArrayList<>(lines != null ? lines : List.of()));
this.enabled = enabled && !this.lines.isEmpty();
this.offsetY = Math.max(0.0, offsetY);
this.viewDistance = Math.max(1.0, viewDistance);
this.shadow = shadow;
this.seeThrough = seeThrough;
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@common/src/main/java/com/pedrodalben/bigbangessentials/npcs/api/NpcHologramConfig.java`
around lines 15 - 22, Update the NpcHologramConfig constructor to prevent an
enabled configuration when lines is null or empty: either normalize enabled to
false or reject the input, while preserving existing line copying and other
field normalization.


public static NpcHologramConfig defaults(String displayName) {
return new NpcHologramConfig(true, List.of(displayName), 2.25, 32.0, true, false);
}

public static NpcHologramConfig disabled() {
return new NpcHologramConfig(false, List.of(), 2.25, 32.0, true, false);
}

public boolean enabled() { return enabled; }
public List<String> lines() { return lines; }
public double offsetY() { return offsetY; }
public double viewDistance() { return viewDistance; }
public boolean shadow() { return shadow; }
public boolean seeThrough() { return seeThrough; }

public NpcHologramConfig withEnabled(boolean enabled) {
return new NpcHologramConfig(enabled, lines, offsetY, viewDistance, shadow, seeThrough);
}

public NpcHologramConfig withLines(List<String> lines) {
return new NpcHologramConfig(enabled, lines, offsetY, viewDistance, shadow, seeThrough);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.pedrodalben.bigbangessentials.npcs.api;

public final class NpcInteractionConfig {
private final double distance;
private final long cooldownMillis;
private final String permission;

public NpcInteractionConfig(double distance, long cooldownMillis, String permission) {
this.distance = Math.max(0.5, distance);
this.cooldownMillis = Math.max(0, cooldownMillis);
this.permission = permission != null ? permission.trim() : "";
}

public static NpcInteractionConfig defaults() {
return new NpcInteractionConfig(4.5, 750, "");
}

public double distance() { return distance; }
public long cooldownMillis() { return cooldownMillis; }
public String permission() { return permission; }
public boolean hasPermission() { return !permission.isEmpty(); }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package com.pedrodalben.bigbangessentials.npcs.api;

import net.minecraft.resources.ResourceLocation;
import java.util.Objects;

public final class NpcLocation {
private final ResourceLocation dimension;
private final double x;
private final double y;
private final double z;
private final float yaw;
private final float pitch;

public NpcLocation(ResourceLocation dimension, double x, double y, double z, float yaw, float pitch) {
this.dimension = Objects.requireNonNull(dimension, "dimension");
this.x = x;
this.y = y;
this.z = z;
this.yaw = yaw;
this.pitch = pitch;
}

public ResourceLocation dimension() { return dimension; }
public double x() { return x; }
public double y() { return y; }
public double z() { return z; }
public float yaw() { return yaw; }
public float pitch() { return pitch; }

public NpcLocation withYawPitch(float yaw, float pitch) {
return new NpcLocation(dimension, x, y, z, yaw, pitch);
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof NpcLocation that)) return false;
return Double.compare(that.x, x) == 0 && Double.compare(that.y, y) == 0
&& Double.compare(that.z, z) == 0 && Float.compare(that.yaw, yaw) == 0
&& Float.compare(that.pitch, pitch) == 0 && dimension.equals(that.dimension);
}

@Override
public int hashCode() {
return Objects.hash(dimension, x, y, z, yaw, pitch);
}
}
Loading