Skip to content
Open
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 @@ -2,7 +2,7 @@

import com.eternalcode.core.bridge.dynmap.DynmapBridgeInitializer;
import com.eternalcode.core.bridge.placeholderapi.PlaceholderApiExtension;
import com.eternalcode.core.bridge.placeholderapi.PlaceholderApiReplacer;
import com.eternalcode.core.bridge.placeholderapi.PlaceholderAPIPlaceholder;
import com.eternalcode.core.feature.vanish.VanishService;
import com.eternalcode.core.placeholder.PlaceholderRegistry;
import org.bukkit.Server;
Expand Down Expand Up @@ -38,7 +38,7 @@ class BridgeManager {

void init() {
this.setupBridge("PlaceholderAPI", () -> {
this.placeholderRegistry.register(new PlaceholderApiReplacer());
this.placeholderRegistry.register(new PlaceholderAPIPlaceholder());
new PlaceholderApiExtension(this.placeholderRegistry, this.pluginDescriptionFile).initialize();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import me.clip.placeholderapi.PlaceholderAPI;
import org.bukkit.entity.Player;

public class PlaceholderApiReplacer implements Placeholder {
public class PlaceholderAPIPlaceholder implements Placeholder {
Copy link
Member

Choose a reason for hiding this comment

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

Rename to PlaceholderApiPlaceholder, or rename other domain classes to have PlaceholderAPI instead of the Api


@Override
public String apply(String text, Player targetPlayer) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.eternalcode.core.bridge.placeholderapi;

import com.eternalcode.core.bridge.BridgeInitializer;
import com.eternalcode.core.placeholder.PlaceholderRaw;
import com.eternalcode.core.placeholder.NamedPlaceholder;
import com.eternalcode.core.placeholder.PlaceholderRegistry;
import me.clip.placeholderapi.expansion.PlaceholderExpansion;
import org.bukkit.entity.Player;
Expand All @@ -23,10 +23,10 @@ public PlaceholderApiExtension(PlaceholderRegistry placeholderRegistry, PluginDe

@Override
public @Nullable String onPlaceholderRequest(Player player, @NotNull String params) {
Optional<PlaceholderRaw> optional = this.placeholderRegistry.getRawPlaceholder(params);
Optional<NamedPlaceholder> optional = this.placeholderRegistry.getNamedPlaceholder(params);

if (optional.isPresent()) {
return optional.get().rawApply(player);
return optional.get().provideValue(player);
}

return "Unknown placeholder!";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
import com.eternalcode.core.feature.afk.AfkService;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Controller;
import com.eternalcode.core.placeholder.PlaceholderRegistry;
import com.eternalcode.core.placeholder.Placeholder;
import com.eternalcode.core.placeholder.PlaceholderRegistry;
import com.eternalcode.core.publish.Subscribe;
import com.eternalcode.core.publish.event.EternalInitializeEvent;
import com.eternalcode.core.translation.Translation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
import com.eternalcode.annotations.scan.placeholder.PlaceholdersDocs.Entry.Type;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Controller;
import com.eternalcode.core.placeholder.PlaceholderRegistry;
import com.eternalcode.core.placeholder.Placeholder;
import com.eternalcode.core.placeholder.PlaceholderRegistry;
import com.eternalcode.core.publish.Subscribe;
import com.eternalcode.core.publish.event.EternalInitializeEvent;
import com.eternalcode.core.translation.Translation;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package com.eternalcode.core.feature.msg;

import com.eternalcode.core.feature.msg.toggle.MsgState;
import com.eternalcode.core.feature.msg.toggle.MsgToggleRepository;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Controller;
import com.eternalcode.core.placeholder.PlaceholderRegistry;
import com.eternalcode.core.placeholder.Placeholder;
import com.eternalcode.core.placeholder.watcher.PlaceholderWatcherKey;
import com.eternalcode.core.publish.Subscribe;
import com.eternalcode.core.publish.event.EternalInitializeEvent;
import com.eternalcode.core.translation.Translation;
import com.eternalcode.core.translation.TranslationManager;
import java.util.UUID;

@Controller
public class MsgPlaceholderSetup {

public static final PlaceholderWatcherKey<MsgState> MSG_STATE = PlaceholderWatcherKey.of("msg_state", MsgState.class);

private final MsgService msgService;
private final MsgToggleRepository msgToggleRepository;
private final TranslationManager translationManager;

@Inject
MsgPlaceholderSetup(
MsgService msgService,
MsgToggleRepository msgToggleRepository,
TranslationManager translationManager
) {
this.msgService = msgService;
this.msgToggleRepository = msgToggleRepository;
this.translationManager = translationManager;
}

@Subscribe(EternalInitializeEvent.class)
void setUpPlaceholders(PlaceholderRegistry registry) {
Translation translation = this.translationManager.getMessages();

registry.register(Placeholder.ofBoolean(
"socialspy_status",
player -> this.msgService.isSpy(player.getUniqueId())
));

registry.register(Placeholder.of(
"socialspy_status_formatted",
player -> {
UUID uuid = player.getUniqueId();
return this.msgService.isSpy(uuid)
? translation.msg().placeholders().socialSpyEnabled()
: translation.msg().placeholders().socialSpyDisabled();
}
));

registry.register(Placeholder.async(
"msg_status",
MSG_STATE,
player -> msgToggleRepository.getPrivateChatState(player),
state -> state.name().toLowerCase()
));

registry.register(Placeholder.async(
"msg_status_formatted",
MSG_STATE,
player -> msgToggleRepository.getPrivateChatState(player),
state -> state == MsgState.ENABLED
? translation.msg().placeholders().msgEnabled()
: translation.msg().placeholders().msgDisabled()
)
);
Comment on lines +55 to +70
Copy link
Contributor

Choose a reason for hiding this comment

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

high

You are registering two asynchronous placeholders, msg_status and msg_status_formatted, which both depend on the same underlying data from msgToggleRepository.getPrivateChatState(player). However, each call to Placeholder.async() creates a new PlaceholderAsync instance with its own separate cache and loading function. This will lead to duplicate database calls when both placeholders are requested for a player for the first time.

To fix this, the data loading logic should be shared. A possible solution is to adjust the async placeholder API to allow sharing a data source or cache for a given PlaceholderWatcherKey.

The best approach is to fix the root cause in the async placeholder design to avoid redundant data fetching for related placeholders.

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,22 @@ public class ENMsgMessages extends OkaeriConfig implements MsgMessages {
Notice socialSpyEnable = Notice.chat("<color:#9d6eef>► <white>SocialSpy has been {STATE}<white>!");
Notice socialSpyDisable = Notice.chat("<red>► <white>SocialSpy has been {STATE}<white>!");

@Comment("# Formatowanie placeholderów")
Copy link
Member

Choose a reason for hiding this comment

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

This is EN translation, so we use English comments

Suggested change
@Comment("# Formatowanie placeholderów")
@Comment("# Placeholders formatting")

public ENPlaceholders placeholders = new ENPlaceholders();

@Getter
@Accessors(fluent = true)
public static class ENPlaceholders extends OkaeriConfig implements MsgMessages.Placeholders {
private String loading = "<yellow>Loading...";

private String msgEnabled = "<green>Enabled";
private String msgDisabled = "<red>Disabled";
private String socialSpyEnabled = "<green>Enabled";
private String socialSpyDisabled = "<red>Disabled";
}

public ENPlaceholders placeholders() {
return this.placeholders;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,16 @@ public interface MsgMessages {
Notice otherMessagesDisabled();
Notice otherMessagesEnabled();

Placeholders placeholders();

interface Placeholders {
String loading();

String msgEnabled();
String msgDisabled();

String socialSpyEnabled();
String socialSpyDisabled();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,21 @@ public class PLMsgMessages extends OkaeriConfig implements MsgMessages {
Notice otherMessagesEnabled = Notice.chat(
"<color:#9d6eef>► <white>Wiadomości prywatne zostały <color:#9d6eef>włączone <white>dla gracza <color:#9d6eef>{PLAYER}<white>!");

@Comment("# Formatowanie placeholderów")
public PLPlaceholders placeholders = new PLPlaceholders();

@Getter
@Accessors(fluent = true)
public static class PLPlaceholders extends OkaeriConfig implements MsgMessages.Placeholders {
private String loading = "<yellow>Ładowanie...";

private String msgEnabled = "<green>Włączone";
private String msgDisabled = "<red>Wyłączone";
private String socialSpyEnabled = "<green>Włączony";
private String socialSpyDisabled = "<red>Wyłączony";
}

public PLPlaceholders placeholders() {
return this.placeholders;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
import java.util.UUID;
import java.util.concurrent.CompletableFuture;

interface MsgToggleRepository {
public interface MsgToggleRepository {

CompletableFuture<MsgState> getPrivateChatState(UUID uuid);

CompletableFuture<Void> setPrivateChatState(UUID uuid, MsgState toggledOff);
CompletableFuture<MsgState> setPrivateChatState(UUID uuid, MsgState state);

}
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ public CompletableFuture<MsgState> getPrivateChatState(UUID uuid) {
}

@Override
public CompletableFuture<Void> setPrivateChatState(UUID uuid, MsgState state) {
public CompletableFuture<MsgState> setPrivateChatState(UUID uuid, MsgState state) {
return this.save(MsgStateTable.class, new MsgStateTable(uuid, state))
.thenApply(status -> null);
.thenApply(status -> state);
}
}
Original file line number Diff line number Diff line change
@@ -1,51 +1,43 @@
package com.eternalcode.core.feature.msg.toggle;

import com.eternalcode.core.feature.msg.MsgPlaceholderSetup;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Service;
import com.eternalcode.core.placeholder.PlaceholderRegistry;
import com.eternalcode.core.placeholder.watcher.PlaceholderWatcher;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;

@Service
class MsgToggleServiceImpl implements MsgToggleService {

private final MsgToggleRepository msgToggleRepository;
private final ConcurrentHashMap<UUID, MsgState> cachedToggleStates;
private final PlaceholderWatcher<MsgState> watcher;

@Inject
MsgToggleServiceImpl(MsgToggleRepository msgToggleRepository) {
this.cachedToggleStates = new ConcurrentHashMap<>();
MsgToggleServiceImpl(MsgToggleRepository msgToggleRepository, PlaceholderRegistry registry) {
this.msgToggleRepository = msgToggleRepository;

this.watcher = registry.createWatcher(MsgPlaceholderSetup.MSG_STATE);
}


@Override
public CompletableFuture<MsgState> getState(UUID playerUniqueId) {
if (this.cachedToggleStates.containsKey(playerUniqueId)) {
return CompletableFuture.completedFuture(this.cachedToggleStates.get(playerUniqueId));
}

return this.msgToggleRepository.getPrivateChatState(playerUniqueId);
public CompletableFuture<MsgState> getState(UUID player) {
return watcher.track(player, msgToggleRepository.getPrivateChatState(player));
}

@Override
public CompletableFuture<Void> setState(UUID playerUniqueId, MsgState state) {
this.cachedToggleStates.put(playerUniqueId, state);

return this.msgToggleRepository.setPrivateChatState(playerUniqueId, state)
.exceptionally(throwable -> {
this.cachedToggleStates.remove(playerUniqueId);
return null;
});
public CompletableFuture<Void> setState(UUID player, MsgState state) {
return watcher.track(player, msgToggleRepository.setPrivateChatState(player, state))
.thenApply(unused -> null);
}
Comment on lines 23 to 32
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The previous implementation of this service used a ConcurrentHashMap (cachedToggleStates) to cache the toggle states of players, avoiding repeated database lookups. This refactoring has removed the cache, meaning that every call to getState() will now result in a database query via msgToggleRepository.getPrivateChatState(). This is a significant performance regression, especially if getState() is called frequently (e.g., by toggleState).

The new PlaceholderWatcher system updates placeholder caches, but it does not provide a caching layer for the service itself. It's recommended to reintroduce a caching mechanism in this service, similar to the previous implementation. The PlaceholderWatcher can then be used to update or invalidate this service-level cache as well.


@Override
public CompletableFuture<MsgState> toggleState(UUID playerUniqueId) {
return this.getState(playerUniqueId).thenCompose(state -> {
MsgState newState = state.invert();
return this.setState(playerUniqueId, newState)
.thenApply(aVoid -> newState);
.thenApply(unused -> newState);
});
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.eternalcode.core.placeholder;

import org.bukkit.entity.Player;

public interface NamedPlaceholder extends Placeholder {

@Override
default String apply(String text, Player targetPlayer) {
return text.replace("{" + this.getName() + "}", this.provideValue(targetPlayer));
}

String getName();

String provideValue(Player targetPlayer);

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package com.eternalcode.core.placeholder;

import com.eternalcode.core.placeholder.watcher.PlaceholderWatcherKey;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import org.bukkit.entity.Player;

import java.util.function.Function;
Expand All @@ -9,27 +12,36 @@ public interface Placeholder {
String apply(String text, Player targetPlayer);

static Placeholder of(String target, String replacement) {
return new PlaceholderRaw(target, player -> replacement);
return new StaticValuePlaceholder(target, player -> replacement);
}

static Placeholder of(String target, Function<Player, String> replacement) {
return new PlaceholderRaw(target, replacement);
return new StaticValuePlaceholder(target, replacement);
}

static Placeholder ofInt(String target, Function<Player, Integer> replacement) {
return new PlaceholderRaw(target, player -> String.valueOf(replacement.apply(player)));
return new StaticValuePlaceholder(target, player -> String.valueOf(replacement.apply(player)));
}

static Placeholder ofBoolean(String target, Function<Player, Boolean> replacement) {
return new PlaceholderRaw(target, player -> String.valueOf(replacement.apply(player)));
return new StaticValuePlaceholder(target, player -> String.valueOf(replacement.apply(player)));
}

static Placeholder ofLong(String target, Function<Player, Long> replacement) {
return new PlaceholderRaw(target, player -> String.valueOf(replacement.apply(player)));
return new StaticValuePlaceholder(target, player -> String.valueOf(replacement.apply(player)));
}

static Placeholder of(String target, Placeholder placeholder) {
return new PlaceholderRaw(target, player -> placeholder.apply(target, player));
return new StaticValuePlaceholder(target, player -> placeholder.apply(target, player));
}

static <T> Placeholder async(
String target,
Copy link
Member

Choose a reason for hiding this comment

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

I would rename target to sth like "placeholderName"

PlaceholderWatcherKey<T> key,
Function<UUID, CompletableFuture<T>> loading,
Function<T, String> mapper
) {
return new PlaceholderAsync<>(target, key, loading, mapper);
}

}
Loading