Skip to content

GH-1221 Reformat placeholder definitions #1221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 26, 2025
Merged

GH-1221 Reformat placeholder definitions #1221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3f4d99
Reformat placeholder definitions
Rollczi Oct 21, 2025
5a01379
CR
Rollczi Oct 26, 2025
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
2 changes: 1 addition & 1 deletion eternalcore-core/src/main/java/com/eternalcode/core/bridge/BridgeManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class BridgeManager {

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

Expand Down
4 changes: 2 additions & 2 deletions ...core/src/main/java/com/eternalcode/core/bridge/placeholderapi/PlaceholderApiReplacer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package com.eternalcode.core.bridge.placeholderapi;


import com.eternalcode.core.placeholder.PlaceholderReplacer;
import com.eternalcode.core.placeholder.Placeholder;
import me.clip.placeholderapi.PlaceholderAPI;
import org.bukkit.entity.Player;

public class PlaceholderApiReplacer implements PlaceholderReplacer {
public class PlaceholderApiReplacer implements Placeholder {

@Override
public String apply(String text, Player targetPlayer) {
Expand Down
143 changes: 57 additions & 86 deletions ...-core/src/main/java/com/eternalcode/core/feature/afk/placeholder/AfkPlaceholderSetup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package com.eternalcode.core.feature.afk.placeholder;

import com.eternalcode.annotations.scan.placeholder.PlaceholderDocs;
import com.eternalcode.annotations.scan.placeholder.PlaceholdersDocs;
import com.eternalcode.annotations.scan.placeholder.PlaceholdersDocs.Entry;
import static com.eternalcode.annotations.scan.placeholder.PlaceholdersDocs.Entry.*;
Copy link
Member

Choose a reason for hiding this comment

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

Static imports should typically go to the bottom

import com.eternalcode.core.feature.afk.Afk;
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.PlaceholderReplacer;
import com.eternalcode.core.placeholder.Placeholder;
import com.eternalcode.core.publish.Subscribe;
import com.eternalcode.core.publish.event.EternalInitializeEvent;
import com.eternalcode.core.translation.Translation;
Expand All @@ -17,6 +19,35 @@
import java.util.Optional;
import org.bukkit.Server;

@PlaceholdersDocs(
category = "AFK",
placeholders = {
@Entry(
name = "afk",
description = "Returns `true` if the player is AFK, `false` otherwise",
returnType = Type.BOOLEAN,
requiresPlayer = true
),
@Entry(
name = "afk_formatted",
description = "Returns a formatted AFK status message based on player's language settings e.g. `[AFK]` or ``",
returnType = Type.STRING,
requiresPlayer = true
),
@Entry(
name = "afk_time",
description = "Returns the duration the player has been AFK in a formatted string e.g. `5m 30s`",
returnType = Type.STRING,
requiresPlayer = true
),
@Entry(
name = "afk_playercount",
description = "Returns the total number of AFK players on the server",
returnType = Type.INT,
requiresPlayer = false
)
}
)
@Controller
class AfkPlaceholderSetup {

Expand All @@ -30,91 +61,31 @@ class AfkPlaceholderSetup {
}

@Subscribe(EternalInitializeEvent.class)
void setUpPlaceholders(PlaceholderRegistry placeholderRegistry, AfkService afkService) {
placeholderRegistry.registerPlaceholder(this.createAfkPlaceholder(afkService));
placeholderRegistry.registerPlaceholder(this.createAfkFormattedPlaceholder(afkService));
placeholderRegistry.registerPlaceholder(this.createAfkTimePlaceholder(afkService));
placeholderRegistry.registerPlaceholder(this.createAfkPlayerCountPlaceholder(afkService));
}

@PlaceholderDocs(
name = "afk",
description = "Returns true if the player is AFK, false otherwise",
example = "true",
returnType = "boolean",
category = "AFK",
requiresPlayer = true
)
private PlaceholderReplacer createAfkPlaceholder(AfkService afkService) {
return PlaceholderReplacer.of(
"afk",
player -> String.valueOf(afkService.isAfk(player.getUniqueId()))
);
}

@PlaceholderDocs(
name = "afk_formatted",
description = "Returns a formatted AFK status message based on player's language settings",
example = "[AFK]",
returnType = "String",
category = "AFK",
requiresPlayer = true
)
private PlaceholderReplacer createAfkFormattedPlaceholder(AfkService afkService) {
return PlaceholderReplacer.of(
"afk_formatted",
player -> {
Translation messages = this.translationManager.getMessages(player.getUniqueId());
return afkService.isAfk(player.getUniqueId()) ?
messages.afk().afkEnabledPlaceholder() : messages.afk().afkDisabledPlaceholder();
void setUpPlaceholders(PlaceholderRegistry placeholders, AfkService afkService) {
placeholders.register(Placeholder.ofBoolean("afk", player -> afkService.isAfk(player.getUniqueId())));
placeholders.register(Placeholder.of("afk_formatted", player -> {
Translation messages = this.translationManager.getMessages();
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 change from this.translationManager.getMessages(player.getUniqueId()) to this.translationManager.getMessages() appears to be a bug. The placeholder description for afk_formatted states it should be "based on player's language settings". Using getMessages() without a player context will likely result in using a default language for all players, instead of their specific language. This should be reverted to use the player-specific method call.

Suggested change
Translation messages = this.translationManager.getMessages();
Translation messages = this.translationManager.getMessages(player.getUniqueId());

return afkService.isAfk(player.getUniqueId()) ?
messages.afk().afkEnabledPlaceholder() : messages.afk().afkDisabledPlaceholder();
}));
placeholders.register(Placeholder.of("afk_time", player -> {
Optional<Afk> afkOptional = afkService.getAfk(player.getUniqueId());
if (afkOptional.isEmpty()) {
return "";
}
);
}

@PlaceholderDocs(
name = "afk_time",
description = "Returns the duration the player has been AFK in a formatted string",
example = "5m 30s",
returnType = "String",
category = "AFK",
requiresPlayer = true
)
private PlaceholderReplacer createAfkTimePlaceholder(AfkService afkService) {
return PlaceholderReplacer.of(
"afk_time",
player -> {
Optional<Afk> afkOptional = afkService.getAfk(player.getUniqueId());
if (afkOptional.isEmpty()) {
return "";
}

Afk afk = afkOptional.get();
Instant start = afk.getStart();
Instant now = Instant.now();
Duration afkDuration = Duration.between(start, now);
return DurationUtil.format(afkDuration, true);
}
);
}

@PlaceholderDocs(
name = "afk_playercount",
description = "Returns the total number of AFK players on the server",
example = "3",
returnType = "int",
category = "AFK",
requiresPlayer = false
)
private PlaceholderReplacer createAfkPlayerCountPlaceholder(AfkService afkService) {
return PlaceholderReplacer.of(
"afk_playercount",
ignoredPlayer -> {
long afkPlayerCount = this.server.getOnlinePlayers()
.stream()
.filter(onlinePlayer -> afkService.isAfk(onlinePlayer.getUniqueId()))
.count();
return String.valueOf(afkPlayerCount);
}
);
Afk afk = afkOptional.get();
Instant start = afk.getStart();
Instant now = Instant.now();
Duration afkDuration = Duration.between(start, now);
return DurationUtil.format(afkDuration, true);
}));
placeholders.register(Placeholder.ofLong("afk_playercount", player -> {
long afkPlayerCount = this.server.getOnlinePlayers()
.stream()
.filter(onlinePlayer -> afkService.isAfk(onlinePlayer.getUniqueId()))
.count();
return afkPlayerCount;
}));
}
}
140 changes: 52 additions & 88 deletions eternalcore-core/src/main/java/com/eternalcode/core/feature/home/HomePlaceholderSetup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,47 @@
package com.eternalcode.core.feature.home;

import com.eternalcode.annotations.scan.placeholder.PlaceholderDocs;
import com.eternalcode.annotations.scan.placeholder.PlaceholdersDocs;
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.PlaceholderReplacer;
import com.eternalcode.core.placeholder.Placeholder;
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.Collection;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.bukkit.entity.Player;

@PlaceholdersDocs(
category = "Home",
placeholders = {
@PlaceholdersDocs.Entry(
name = "homes_owned",
description = "Returns a comma-separated list of all homes owned by the player. If the player has no homes, returns a localized message. e.g. `home1, home2, domek`",
returnType = Type.STRING,
requiresPlayer = true
),
@PlaceholdersDocs.Entry(
name = "homes_count",
description = "Returns the number of homes the player currently owns.",
returnType = Type.INT,
requiresPlayer = true
),
@PlaceholdersDocs.Entry(
name = "homes_limit",
description = "Returns the maximum number of homes the player can have.",
returnType = Type.INT,
requiresPlayer = true
),
@PlaceholdersDocs.Entry(
name = "homes_left",
description = "Returns how many more homes the player can create before reaching the limit.",
returnType = Type.INT,
requiresPlayer = true
)
}
)
@Controller
class HomePlaceholderSetup {

Expand All @@ -27,94 +55,30 @@ class HomePlaceholderSetup {
}

@Subscribe(EternalInitializeEvent.class)
void setUp(PlaceholderRegistry placeholderRegistry) {
Stream.of(
this.createHomesOwnedPlaceholder(),
this.createHomesCountPlaceholder(),
this.createHomesLimitPlaceholder(),
this.createHomesLeftPlaceholder()
).forEach(placeholderRegistry::registerPlaceholder);
}

@PlaceholderDocs(
name = "homes_owned",
description = "Returns a comma-separated list of all homes owned by the player. If the player has no homes, returns a localized message.",
example = "home1, home2, domek",
returnType = "String",
category = "Home",
requiresPlayer = true
)
private PlaceholderReplacer createHomesOwnedPlaceholder() {
return PlaceholderReplacer.of(
"homes_owned",
(text, targetPlayer) -> {
Collection<Home> homes = this.homeService.getHomes(targetPlayer.getUniqueId());
Translation translation = this.translationManager.getMessages(targetPlayer.getUniqueId());
void setUp(PlaceholderRegistry placeholders) {
placeholders.register(Placeholder.of("homes_owned", target -> {
Collection<Home> homes = this.homeService.getHomes(target.getUniqueId());
Translation translation = this.translationManager.getMessages(target.getUniqueId());

if (homes.isEmpty()) {
return translation.home().noHomesOwnedPlaceholder();
}

return homes.stream()
.map(Home::getName)
.collect(Collectors.joining(", "));
if (homes.isEmpty()) {
return translation.home().noHomesOwnedPlaceholder();
}
);
}

@PlaceholderDocs(
name = "homes_count",
description = "Returns the number of homes the player currently owns.",
example = "3",
returnType = "int",
category = "Home",
requiresPlayer = true
)
private PlaceholderReplacer createHomesCountPlaceholder() {
return PlaceholderReplacer.of(
"homes_count",
(text, targetPlayer) ->
String.valueOf(this.homeService.getAmountOfHomes(targetPlayer.getUniqueId()))
);
}
return homes.stream()
.map(Home::getName)
.collect(Collectors.joining(", "));
}));
placeholders.register(Placeholder.ofInt("homes_count", target -> this.homeService.getAmountOfHomes(target.getUniqueId())));
placeholders.register(Placeholder.ofInt("homes_limit", target -> this.homeService.getHomeLimit(target)));
placeholders.register(Placeholder.ofInt("homes_left", target -> {
int homesLimit = this.homeService.getHomeLimit(target);
int amountOfHomes = this.homeService.getAmountOfHomes(target.getUniqueId());

@PlaceholderDocs(
name = "homes_limit",
description = "Returns the maximum number of homes the player can have.",
example = "5",
returnType = "int",
category = "Home",
requiresPlayer = true
)
private PlaceholderReplacer createHomesLimitPlaceholder() {
return PlaceholderReplacer.of(
"homes_limit",
(text, targetPlayer) ->
String.valueOf(this.homeService.getHomeLimit(targetPlayer))
);
}

@PlaceholderDocs(
name = "homes_left",
description = "Returns how many more homes the player can create before reaching the limit.",
example = "2",
returnType = "int",
category = "Home",
requiresPlayer = true
)
private PlaceholderReplacer createHomesLeftPlaceholder() {
return PlaceholderReplacer.of(
"homes_left",
(text, targetPlayer) -> {
int homesLimit = this.homeService.getHomeLimit(targetPlayer);
int amountOfHomes = this.homeService.getAmountOfHomes(targetPlayer.getUniqueId());

if (homesLimit < -1 || amountOfHomes > homesLimit) {
return "0";
}

return String.valueOf(homesLimit - amountOfHomes);
if (homesLimit < -1 || amountOfHomes > homesLimit) {
return 0;
}
);

return homesLimit - amountOfHomes;
}));
Comment on lines +73 to +82
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 logic for the homes_left placeholder seems incorrect when a player has an unlimited number of homes (usually represented by homesLimit == -1). The current implementation will return 0, which is misleading. For an unlimited limit, it should indicate that, for example by returning -1 or another special value since this is an integer placeholder.

The proposed suggestion handles the unlimited case and also simplifies the logic for calculating remaining homes.

        placeholders.register(Placeholder.ofInt("homes_left", target -> {
            int homesLimit = this.homeService.getHomeLimit(target);
            if (homesLimit == -1) {
                return -1; // Use -1 to indicate unlimited
            }

            int amountOfHomes = this.homeService.getAmountOfHomes(target.getUniqueId());
            return Math.max(0, homesLimit - amountOfHomes);
        }));

}
}
35 changes: 35 additions & 0 deletions eternalcore-core/src/main/java/com/eternalcode/core/placeholder/Placeholder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.eternalcode.core.placeholder;

import org.bukkit.entity.Player;

import java.util.function.Function;

public interface Placeholder {

String apply(String text, Player targetPlayer);

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

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

static Placeholder ofInt(String target, Function<Player, Integer> replacement) {
return new PlaceholderRaw(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)));
}

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

static Placeholder of(String target, Placeholder placeholder) {
return new PlaceholderRaw(target, player -> placeholder.apply(target, player));
}
Comment on lines +31 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The implementation of of(String target, Placeholder placeholder) seems incorrect or at least very confusing. It calls placeholder.apply(target, player), passing the new target string as the text to be processed. This means it will try to find and replace placeholders within the target string itself, which is likely not the intended behavior for aliasing a placeholder. This method should be reviewed to ensure it behaves as expected.


}
Loading