Skip to content

GH-930 Msg placeholders & Async placeholder for future #1203

Open
P1otrulla wants to merge 16 commits intomasterfrom
2.0/msg-placeholders
Open

GH-930 Msg placeholders & Async placeholder for future #1203
P1otrulla wants to merge 16 commits intomasterfrom
2.0/msg-placeholders

Conversation

@P1otrulla
Copy link
Member

@P1otrulla P1otrulla commented Oct 12, 2025 •
edited
Loading

@P1otrulla
GH-1142 Add message placeholders for msg_toggle and socialspy_status
b3764bd 
* Register new placeholders for `msg_toggle` and `socialspy_status` in `MsgPlaceholderSetup`.
* Extend `MsgMessages` with a `Placeholders` interface and implement language-specific placeholder formats in `ENMsgMessages` and `PLMsgMessages`.
@P1otrulla
GH-1143 Enhance MsgPlaceholderSetup with caching and new placeholders
af593f1 
* Add caching logic for `msg_toggle` state in `MsgPlaceholderSetup`.
* Introduce new formatted placeholders for `msg_toggle` and `socialspy_status`.
* Extend `MsgMessages.Placeholders` to support a loading state with translations.
@P1otrulla
GH-1144 Update MsgPlaceholderSetup caching and integrate `Placehold…
1f98392 
…erAPI`

* Adjust cache duration in `MsgPlaceholderSetup` from 200ms to 5s.
* Rename `msg_toggle` placeholders to `msg_status` for consistency.
* Extend `ENPlaceholders` and `PLPlaceholders` with `OkaeriConfig`.
* Add `PlaceholderAPI` dependency in `runServer` task configuration.
@P1otrulla
Refactor MsgPlaceholderSetup with new caching system
cf7151e 
* Replace existing cache in `MsgPlaceholderSetup` with `AsyncPlaceholderCached`.
* Introduce `AsyncPlaceholderCacheRegistry` for managing placeholder caches.
* Add `AsyncPlaceholderCacheController` to handle cache invalidation on player quit.
* Simplify placeholder logic and remove redundant caching methods.
@P1otrulla
Refactor MsgPlaceholderSetup to enhance caching with `AsyncPlacehol…
1407a28 
…derCacheRegistry`

* Replace individual `stateCache` with dynamically registered cache in the `AsyncPlaceholderCacheRegistry`.
* Simplify placeholder setup logic and adjust `MsgToggleService` to utilize the registry for cache management.
@P1otrulla
Enhance AsyncPlaceholderCacheController with event-based cache inva…
a8c6659 
…lidation

* Add `Subscribe` annotations to invalidate cache on `EternalReloadEvent` and `EternalShutdownEvent`.
@P1otrulla
Refactor AsyncPlaceholderCacheController event methods
2633ef6 
* Rename `onEternalDisable` to `onDisable` and `onReload`.
@P1otrulla
the bomb was defused xd
dc6f963
@P1otrulla P1otrulla requested a review from a team as a code owner October 12, 2025 11:27
@P1otrulla P1otrulla linked an issue Oct 12, 2025 that may be closed by this pull request
Add a placeholder for the /msgtoggle command. i.e: %eternalcore_msgtoggle% #930
Open
gemini-code-assist[bot]
gemini-code-assist bot reviewed Oct 12, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new asynchronous caching mechanism for placeholders and integrates it with the messaging feature. The changes include a new AsyncPlaceholderCacheRegistry and AsyncPlaceholderCached for managing async data loading, new placeholders for message and social spy status, and refactoring MsgToggleServiceImpl to use the new caching system. The overall implementation is good, but I've found a few areas for improvement. There's a race condition in AsyncPlaceholderCached that could lead to multiple concurrent loads for the same data, some code duplication in MsgPlaceholderSetup when defining similar placeholders, and a minor inconsistency in a comment in one of the message files. I've left specific comments with suggestions on how to address these points.

@P1otrulla P1otrulla changed the title Msg placeholders & Async placeholder for future GH-930 Msg placeholders & Async placeholder for future Oct 12, 2025
@P1otrulla P1otrulla requested a review from noyzys October 12, 2025 12:12
noyzys
noyzys reviewed Oct 12, 2025
View reviewed changes
Copy link
Member

@noyzys noyzys left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Jezeli cos niezrozumialego smialo pytac lub dawac wlasne propozycje zmian ;).
Wszystko mozna podkrecic i jezeli jest jakis problem nie sugerowac sie moimi zmianami bo zawsze jest jakies ala w trakcie review

@P1otrulla P1otrulla requested a review from Qbiterv October 12, 2025 22:37
imDMK
imDMK approved these changes Oct 14, 2025
View reviewed changes
@vLuckyyy vLuckyyy requested review from Rollczi and removed request for Qbiterv October 19, 2025 12:38
Rollczi
Rollczi requested changes Oct 21, 2025
View reviewed changes
Copy link
Member

@Rollczi Rollczi left a comment

Choose a reason for hiding this comment

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

1 komentarz na razie bo może być dużo do przerobienia

@Rollczi
Copy link
Member

Rollczi commented Feb 21, 2026

nic się nie dzieje to zrobiłem commita

@Rollczi
CR
81b610e
@Rollczi
CR
450a2d7
@Rollczi
CR
9f0d5eb
@Rollczi
Merge branch 'master' into 2.0/msg-placeholders
3e5a511 
# Conflicts:
#	eternalcore-core/src/main/java/com/eternalcode/core/bridge/BridgeManager.java
#	eternalcore-core/src/main/java/com/eternalcode/core/bridge/placeholderapi/PlaceholderAPIPlaceholder.java
#	eternalcore-core/src/main/java/com/eternalcode/core/placeholder/PlaceholderBukkitRegistryImpl.java
#	eternalcore-core/src/main/java/com/eternalcode/core/placeholder/PlaceholderRaw.java
#	eternalcore-core/src/main/java/com/eternalcode/core/placeholder/PlaceholderRegistry.java
#	eternalcore-core/src/main/java/com/eternalcode/core/placeholder/PlaceholderReplacer.java
#	eternalcore-plugin/build.gradle.kts
@Rollczi
CR
082e063
CitralFlo
CitralFlo requested changes Feb 24, 2026
View reviewed changes
Copy link
Member

@CitralFlo CitralFlo left a comment

Choose a reason for hiding this comment

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

-- Polish below --
For me using watcher is a little counterintuitive.
We are using cache in PlaceholderAsync, so maybe moving it to the PlaceholderSetup.class would be a better option.

My idea is creating a interface or abstract that needs to be implemented in PlaceholderSetup for AsyncPlaceholder. Instead of putting the watcher in Repo and then using Repo in setup class. I would suggest using predefined standards with cache (required) in class method for refresh and method used in Service that would update / call update for specific player. Additional fallback option that would display some text before accessing value in db ex. "Loading..." it would not be a problem if the player will get answer from server in next message - when placeholder is ready.

PL:
Tak jak patrze to i tak opiera sie na cache.
Może jakiś standard do tego cache i wołanie do placeholderów z update z serwisu, dodatkowo jakis fallback jakby nie bylo. Raczej nie problem jak sie wyswietli "Loading..." a potem poprawna wartosc w następnej wiadomości.
Tylko wiesz np jako abstract albo interfejs dla PlaceholderAsync. Że wymaga tego placeholder i wszystkie warunki do trzymania tych danych są spełnione
napisze to w review.

Image

eternalcore-core/src/main/java/com/eternalcode/core/placeholder/Placeholder.java
}

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

@CitralFlo CitralFlo Feb 24, 2026

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"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Rollczi Rollczi Rollczi requested changes

@CitralFlo CitralFlo CitralFlo requested changes

@imDMK imDMK imDMK approved these changes

@vLuckyyy vLuckyyy Awaiting requested review from vLuckyyy

@Jakubk15 Jakubk15 Awaiting requested review from Jakubk15

@igoyek igoyek Awaiting requested review from igoyek

@noyzys noyzys Awaiting requested review from noyzys

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add a placeholder for the /msgtoggle command. i.e: %eternalcore_msgtoggle%

8 participants

@P1otrulla @Rollczi @noyzys @vLuckyyy @igoyek @Jakubk15 @CitralFlo @imDMK