GH-884 Fix sound serdes #884
Conversation
|
Warning Rate limit exceeded@Jakubk15 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 9 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis pull request introduces a comprehensive refactoring of the dependency injection (DI) framework and several core components in the EternalCore project. The changes primarily focus on modernizing the annotation-based configuration, enhancing bean candidate management, and streamlining the event handling mechanism. Key modifications include replacing Changes
Sequence DiagramsequenceDiagram
participant DependencyScanner
participant BeanFactory
participant BeanCandidateContainer
participant BeanCandidate
participant PriorityProvider
DependencyScanner->>BeanFactory: Scan for candidates
BeanFactory->>BeanCandidateContainer: Add candidates
BeanCandidateContainer->>PriorityProvider: Determine priority
PriorityProvider-->>BeanCandidateContainer: Return priority level
BeanCandidateContainer->>BeanCandidate: Create prioritized candidates
BeanFactory->>BeanCandidate: Instantiate beans
This diagram illustrates the new bean candidate selection process, highlighting the introduction of priority-based candidate management and the interaction between different components in the dependency injection framework. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🔭 Outside diff range comments (3)
eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AftPlaceholderSetup.java (1)
Line range hint
31-57: Refactor to use constructor injection for better DI practicesThe
setUpPlaceholdersmethod receives dependencies through parameters, which could be moved to constructor injection for better maintainability and alignment with DI best practices.Consider this refactoring:
@Controller class AftPlaceholderSetup { private final TranslationManager translationManager; private final ViewerService viewerService; + private final PlaceholderRegistry placeholderRegistry; + private final AfkService afkService; @Inject - AftPlaceholderSetup(TranslationManager translationManager, ViewerService viewerService) { + AftPlaceholderSetup( + TranslationManager translationManager, + ViewerService viewerService, + PlaceholderRegistry placeholderRegistry, + AfkService afkService + ) { this.translationManager = translationManager; this.viewerService = viewerService; + this.placeholderRegistry = placeholderRegistry; + this.afkService = afkService; + this.setUpPlaceholders(); } - @Subscribe(EternalInitializeEvent.class) - void setUpPlaceholders(PlaceholderRegistry placeholderRegistry, AfkService afkService) { + private void setUpPlaceholders() {eternalcore-core/src/main/java/com/eternalcode/core/configuration/implementation/PluginConfiguration.java (1)
Add minimum interval validation to prevent potential spam
The AutoMessage implementation lacks validation for the interval duration. Very short intervals could lead to chat spam and potential performance issues. Consider:
- Adding validation in the
interval()method to enforce a minimum threshold (e.g., 30 seconds)- Or implementing validation in the AutoMessageService before scheduling
🔗 Analysis chain
Line range hint
367-400: Verify AutoMessage interval validation.The AutoMessage implementation should ensure that the interval is not too short to prevent spam.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any existing validations for AutoMessage interval rg -A 5 "interval.*Duration" --type javaLength of output: 939
Script:
#!/bin/bash # Find AutoMessageSettings interface definition ast-grep --pattern 'interface AutoMessageSettings { $$$ }' # Find where interval() is called rg -A 5 '\.interval\(\)' --type javaLength of output: 850
Script:
#!/bin/bash # Check AutoMessageService class implementation ast-grep --pattern 'class AutoMessageService { $$$ }' # Look for any Duration validation patterns rg "Duration.*validate|validate.*Duration" --type javaLength of output: 114
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanCandidateContainer.java (1)
Line range hint
43-54: Avoid modifying the collection during iteration to preventConcurrentModificationExceptionIn
nextCandidate(Class<?> type), removing elements fromthis.candidateswhile iterating over it can cause aConcurrentModificationException. To safely remove elements during iteration, use an iterator and itsremove()method.Apply this diff to use an iterator for safe removal:
BeanCandidate nextCandidate(Class<?> type) { synchronized (this.lock) { + Iterator<BeanCandidate> iterator = this.candidates.iterator(); + while (iterator.hasNext()) { + BeanCandidate candidate = iterator.next(); - for (BeanCandidate candidate : this.candidates) { if (!candidate.isCandidate(type)) { continue; } - this.candidates.remove(candidate); + iterator.remove(); return candidate; } } return null; }
🧹 Nitpick comments (45)
eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AfkCommand.java (2)
37-37: LGTM! Consider adding a comment explaining the dynamic delay behavior.The use of a lambda for dynamic delay retrieval is a good improvement, allowing the delay to reflect configuration changes at runtime.
+ // Use lambda to dynamically retrieve delay from config, allowing runtime updates this.delay = new Delay<>(() -> this.pluginConfiguration.afk.getAfkDelay());
64-64: Consider capturing the delay value to ensure consistency.While using
getAfkDelay()is correct, there's a potential race condition if the configuration changes between thehasDelaycheck andmarkDelaycall.void execute(@Context Player player) { UUID uuid = player.getUniqueId(); + Duration afkDelay = this.pluginConfiguration.afk.getAfkDelay(); - if (this.delay.hasDelay(uuid)) { + if (this.delay.hasDelay(uuid, afkDelay)) { Duration time = this.delay.getDurationToExpire(uuid); // ... existing code ... } // ... existing code ... - this.delay.markDelay(uuid, this.pluginConfiguration.afk.getAfkDelay()); + this.delay.markDelay(uuid, afkDelay); }eternalcore-core/src/main/java/com/eternalcode/core/feature/catboy/CatboyController.java (3)
Line range hint
39-50: Consider renaming the event handler method.The method name
onCatboyEventconflicts with another method in the same class. Consider a more specific name likeonCatboyDismountto better describe its purpose.- void onCatboyEvent(EntityDismountEvent event) { + void onCatboyDismount(EntityDismountEvent event) {
Line range hint
39-50: Add documentation explaining the business logic.Please add a Javadoc comment explaining:
- Why cats are removed when dismounted by catboys
- Whether this is intended game behavior
+ /** + * Handles the dismounting of players from cats. + * When a catboy dismounts from a cat, the cat is removed from the world + * [Please explain the game design reason for this behavior] + * + * @param event The dismount event + */ @EventHandler void onCatboyEvent(EntityDismountEvent event) {
Line range hint
39-39: Add event priority for consistency.Other event handlers in this class specify priority. Consider adding
priority = EventPriority.MONITORfor consistency.- @EventHandler + @EventHandler(priority = EventPriority.MONITOR)eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/RepairCommand.java (1)
Line range hint
36-169: Consider refactoring to reduce code duplication.The repair methods (
repair,repairAll,repairArmor) share similar patterns for:
- Delay checking
- Notification sending
- Item repair logic
Consider extracting these common patterns into helper methods to improve maintainability and reduce duplication.
Would you like me to propose a refactored version that consolidates these common patterns?
eternalcore-core/src/main/java/com/eternalcode/core/feature/helpop/HelpOpCommand.java (2)
39-39: LGTM! Consider documenting the dynamic delay behavior.The change to use a supplier lambda for delay initialization is a good improvement, allowing for dynamic updates to the delay value. This makes the code more flexible and maintainable.
Consider adding a comment to document that the delay value can be dynamically updated through config changes:
+ // Uses supplier to allow dynamic updates to delay value from config this.delay = new Delay<>(() -> this.config.helpOp.getHelpOpDelay());
Line range hint
43-86: Consider extracting notice creation logic for better modularity.The
executemethod handles multiple responsibilities including delay checking, notice creation, and broadcasting. Consider extracting the notice creation and broadcasting logic into separate methods for better maintainability.Here's a suggested refactor:
@Execute @DescriptionDocs(description = "Send helpop message to all administrator with eternalcore.helpop.spy permission", arguments = "<message>") void execute(@Context Player player, @Join String message) { UUID uuid = player.getUniqueId(); HelpOpEvent event = new HelpOpEvent(player, message); this.eventCaller.callEvent(event); if (event.isCancelled()) { return; } if (this.delay.hasDelay(uuid)) { - Duration time = this.delay.getDurationToExpire(uuid); - - this.noticeService.create() - .notice(translation -> translation.helpOp().helpOpDelay()) - .placeholder("{TIME}", DurationUtil.format(time)) - .player(uuid) - .send(); + sendDelayNotice(uuid); return; } + broadcastHelpOpMessage(player, message); + sendConfirmationNotice(player); + this.delay.markDelay(uuid, this.config.helpOp.getHelpOpDelay()); + } + + private void sendDelayNotice(UUID uuid) { + Duration time = this.delay.getDurationToExpire(uuid); + this.noticeService.create() + .notice(translation -> translation.helpOp().helpOpDelay()) + .placeholder("{TIME}", DurationUtil.format(time)) + .player(uuid) + .send(); + } + + private void broadcastHelpOpMessage(Player player, String message) { NoticeBroadcast notice = this.noticeService.create() .console() .notice(translation -> translation.helpOp().format()) .placeholder("{PLAYER}", player.getName()) .placeholder("{TEXT}", message); for (Player admin : this.server.getOnlinePlayers()) { if (!admin.hasPermission("eternalcore.helpop.spy")) { continue; } notice = notice.player(admin.getUniqueId()); } notice.send(); + } + private void sendConfirmationNotice(Player player) { this.noticeService .create() .player(player.getUniqueId()) .notice(translation -> translation.helpOp().send()) .send(); - - this.delay.markDelay(uuid, this.config.helpOp.getHelpOpDelay()); }eternalcore-core/src/main/java/com/eternalcode/core/injector/scan/ComponentBeanCandidateImpl.java (1)
29-32: Well-designed addition to the DI framework.The new
getType()method complements the existing type-related functionality inComponentBeanHolderImpl, providing access to static type information at the candidate level while maintaining proper encapsulation. This enhancement aligns well with the DI framework's architecture and type safety requirements.eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/LiteCommandsSetup.java (1)
Line range hint
52-61: Consider handling edge case in shutdown handlerWhile the lifecycle management is good, consider handling the case where shutdown occurs before initialization is complete.
@Subscribe(EternalShutdownEvent.class) public void onShutdown(LiteCommands<CommandSender> liteCommands) { + if (liteCommands == null) { + return; + } liteCommands.unregister(); }eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanCandidate.java (1)
11-12: Add Javadoc for the new getType method.Consider adding documentation to explain the purpose and usage of this method, especially since it's a public API.
+ /** + * Returns the type of the bean candidate. + * + * @return the class type of the bean + */ Class<?> getType();eternalcore-core/src/main/java/com/eternalcode/core/injector/scan/DependencyScanner.java (2)
40-43: Add Javadoc to document the predicate's purpose.Consider adding Javadoc to explain:
- The predicate's role in filtering types
- Example usage
- Whether the predicate is applied with AND or OR logic when multiple predicates exist
+/** + * Adds a predicate to filter types during scanning. + * Multiple predicates are combined with AND logic. + * + * @param predicate the predicate to test classes against + * @return this scanner instance for method chaining + */ public DependencyScanner includeType(Predicate<Class<?>> predicate) {
69-72: Consider caching filter results for repeated scans.The current implementation evaluates all predicates for each class. If the scanner is reused, consider caching the results.
+ private final Map<Class<?>, Boolean> filterCache = new HashMap<>(); + private boolean isIncluded(Class<?> clazz) { + return filterCache.computeIfAbsent(clazz, + c -> this.includedTypes.stream().allMatch(filter -> filter.test(c))); }eternalcore-core/src/main/java/com/eternalcode/core/feature/home/HomePlaceholderSetup.java (2)
Line range hint
52-58: Consider extracting home limit validation logic.The
homesLeftcalculation logic is split between an instance and a static method. Consider:
- Moving the validation logic to
HomeService- Adding input validation for negative values in
amountOfHomes- private String homesLeft(Player targetPlayer) { - int homesLimit = this.homeService.getHomeLimit(targetPlayer); - int amountOfHomes = this.homeService.getAmountOfHomes(targetPlayer.getUniqueId()); - - return homesLeft(homesLimit, amountOfHomes); - } - - static String homesLeft(int homesLimit, int amountOfHomes) { - if (homesLimit < -1 || amountOfHomes > homesLimit) { - return "0"; - } - - int result = homesLimit - amountOfHomes; - - return String.valueOf(result); - } + private String homesLeft(Player targetPlayer) { + return String.valueOf(this.homeService.calculateHomesLeft(targetPlayer)); + }Also applies to: 60-69
Line range hint
71-83: Consider using String.join for better readability.The stream collection to join home names could be simplified:
- return homes.stream().map(Home::getName).collect(Collectors.joining(", ")); + return String.join(", ", homes.stream().map(Home::getName).toList());eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanCandidatePriorityProvider.java (3)
4-4: Consider internalizing the PriorityLevel enum.The class currently depends on
dev.rollczi.litecommands.priority.PriorityLevel. Consider creating an internal priority enum to reduce coupling with external libraries and maintain better control over the priority levels.
7-7: Add class-level documentation.Please add Javadoc to explain:
- The purpose of this provider
- The priority assignment logic
- The relationship with the DI framework
+/** + * Provides priority levels for bean candidates in the dependency injection framework. + * Assigns HIGH priority to subscriber beans and NORMAL priority to regular beans. + */ public class BeanCandidatePriorityProvider implements Function<BeanCandidate, PriorityLevel> {
9-15: Add parameter validation and consider expanding priority levels.The current implementation has several potential improvements:
- Add null checks for the candidate parameter
- Validate candidate.getType() return value
- Consider a more granular priority system for different types of beans
Here's a suggested implementation:
@Override public PriorityLevel apply(BeanCandidate candidate) { + if (candidate == null) { + throw new IllegalArgumentException("BeanCandidate cannot be null"); + } + + Class<?> type = candidate.getType(); + if (type == null) { + throw new IllegalArgumentException("BeanCandidate type cannot be null"); + } + if (SubscriberUtil.isSubscriber(candidate.getType())) { return PriorityLevel.HIGH; } return PriorityLevel.NORMAL; }eternalcore-core/src/main/java/com/eternalcode/core/feature/randomteleport/RandomTeleportSettingsImpl.java (2)
Line range hint
166-196: Add validation in migration code.The migration method could benefit from validation to ensure the migrated values are within acceptable ranges. For example, validating that the cooldown and teleport attempts are positive values.
@Override public boolean migrate() { boolean migrated = false; if (randomTeleportDelay != null) { + if (randomTeleportDelay.isNegative()) { + randomTeleportDelay = Duration.ZERO; + } cooldown = randomTeleportDelay; randomTeleportDelay = null; migrated = true; } // ... other migrations ... if (randomTeleportAttempts != null) { + if (randomTeleportAttempts <= 0) { + randomTeleportAttempts = 10; + } teleportAttempts = randomTeleportAttempts; randomTeleportAttempts = null; migrated = true; } return migrated; }
Line range hint
134-139: Enhance version compatibility documentation.The height range description could be more specific about version compatibility. Consider adding the exact Minecraft versions where these limits apply.
@Description({ "# Height range for random teleportation", - "# - Minimum: -64 (1.18+) or 0 (older versions)", - "# - Maximum: 320 (1.18+) or 256 (older versions)", + "# - Minimum: -64 (Minecraft 1.18+) or 0 (Minecraft 1.17 and older)", + "# - Maximum: 320 (Minecraft 1.18+) or 256 (Minecraft 1.17 and older)", "# - Default range: 60-160 blocks", "# Note: Values are automatically capped to world height limits" })eternalcore-core/src/main/java/com/eternalcode/core/feature/chat/ChatSettings.java (1)
Line range hint
5-12: Consider adding JavaDoc comments to interface methods for clarity.Adding JavaDoc comments to the methods in
ChatSettingswill improve code maintainability and help other developers understand the purpose and usage of each method. This is especially helpful for interfaces, as they define the contract for implementing classes.Example:
public interface ChatSettings { + /** + * Checks if the chat feature is enabled. + * + * @return true if chat is enabled, false otherwise + */ boolean isChatEnabled(); + /** + * Enables or disables the chat feature. + * + * @param chatEnabled true to enable chat, false to disable + */ void setChatEnabled(boolean chatEnabled); // Add similar comments for other methodseternalcore-core/src/main/java/com/eternalcode/core/injector/bean/PrioritizedBeanCandidate.java (1)
3-3: Consider internalizing the priority mechanismThe class currently depends on
dev.rollczi.litecommands.priority.PriorityLevel. Consider creating an internal priority enum to reduce coupling with external libraries, making the DI framework more independent.eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AfkSettings.java (1)
Line range hint
5-13: Consider documenting the delay-related behaviorSince the delay functionality is now decoupled, consider adding Javadoc to clarify the relationship between
getAfkDelay()andgetAfkInactivityTime()methods, as their purposes might not be immediately clear to implementers.Add documentation like this:
public interface AfkSettings { + /** + * Controls the delay before a player is marked as AFK. + * + * @return the duration to wait before marking a player as AFK + */ Duration getAfkDelay(); + /** + * Controls how long a player can be inactive before triggering AFK status. + * + * @return the maximum duration of inactivity allowed + */ Duration getAfkInactivityTime();eternalcore-core/src/main/java/com/eternalcode/core/publish/Publisher.java (1)
4-4: Consider enhancing type safety of the subscribe method.The
subscribemethod accepts anyObject, which isn't type-safe. Consider using a bounded type parameter or a marker interface to ensure only valid event handlers can be registered.Example improvement:
- void subscribe(Object subscriber); + void subscribe(EventHandler subscriber);Where
EventHandlercould be a marker interface or a base class for all event handlers.eternalcore-core/src/main/java/com/eternalcode/core/publish/SubscriberUtil.java (3)
10-10: Fix typo in parameter name.The parameter name "subscriberCanditate" contains a typo and should be "subscriberCandidate".
- public static boolean isSubscriber(Class<?> subscriberCanditate) { + public static boolean isSubscriber(Class<?> subscriberCandidate) {
10-22: Consider performance optimization and add documentation.The current implementation has several areas for improvement:
- Results could be cached to avoid repeated reflection calls
- Parameter validation for null is missing
- Method lacks documentation explaining its purpose and behavior
Consider applying these improvements:
+ /** Cache of subscriber status to avoid repeated reflection calls */ + private static final Map<Class<?>, Boolean> subscriberCache = new ConcurrentHashMap<>(); + + /** + * Checks if a class is a subscriber by looking for methods annotated with @Subscribe. + * + * @param subscriberCandidate the class to check for subscriber status + * @return true if the class contains at least one method with @Subscribe annotation + * @throws NullPointerException if subscriberCandidate is null + */ public static boolean isSubscriber(Class<?> subscriberCandidate) { + Objects.requireNonNull(subscriberCandidate, "subscriberCandidate cannot be null"); + + return subscriberCache.computeIfAbsent(subscriberCandidate, clazz -> { for (Method method : subscriberCandidate.getDeclaredMethods()) { Subscribe subscribe = method.getAnnotation(Subscribe.class); if (subscribe != null) { return true; } } return false; + }); }
11-19: Consider making the early return more explicit.The current implementation returns true immediately upon finding a method with @subscribe annotation. While correct, the logic could be more explicit about its short-circuit nature.
for (Method method : subscriberCandidate.getDeclaredMethods()) { Subscribe subscribe = method.getAnnotation(Subscribe.class); - - if (subscribe == null) { - continue; - } - - return true; + if (subscribe != null) { + // Found a method with @Subscribe annotation, no need to check further + return true; + } }eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanFactory.java (1)
6-6: Consider using an internal priority enum instead of external dependency.The
PriorityLevelis imported fromlitecommands, which creates a coupling between the DI framework and this external library. Consider creating an internal priority enum to maintain better separation of concerns.eternalcore-core/src/main/java/com/eternalcode/core/configuration/composer/OldEnumComposer.java (1)
21-23: Consider using ConcurrentHashMap and implementing cache size limitsThe static
VALUE_OF_METHODScache could benefit from:
- Using
ConcurrentHashMapfor thread safety- Implementing a size limit to prevent potential memory leaks in case of many different enum types
- private static final Map<Class<?>, Method> VALUE_OF_METHODS = new HashMap<>(); + private static final Map<Class<?>, Method> VALUE_OF_METHODS = new ConcurrentHashMap<>(16);eternalcore-core/src/main/java/com/eternalcode/core/EternalCore.java (1)
3-3: Consider InjectingCompatibilityServicefor Better ModularityCreating an instance of
CompatibilityServicedirectly within the constructor may reduce testability and flexibility. Consider injectingCompatibilityServicevia theBeanFactoryor constructor injection to enhance modularity and ease of testing.eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/Bean.java (1)
9-9: Update Documentation for@BeanAnnotation UsageThe
@Beanannotation now targets both methods and fields. Ensure that the documentation and usage guidelines are updated to reflect this change so that developers understand how to correctly use@Beanon fields.eternalcore-core/src/main/java/com/eternalcode/core/compatibility/Compatibility.java (1)
8-15: LGTM! Consider adding Javadoc.The annotation interface is well-structured with appropriate retention and target. Consider adding Javadoc to document:
- The purpose of the annotation
- The meaning of
fromandtoversion ranges- Examples of usage
eternalcore-core/src/main/java/com/eternalcode/core/configuration/compatibility/ConfigurationCompatibilityV21_2.java (1)
14-18: Add parameter validation and consider documenting the compatibility logic.The event handler should validate the event parameter and document the purpose of adding the OldEnumComposer.
@Subscribe void onConfigSettingsSetup(ConfigurationSettingsSetupEvent event) { + if (event == null) { + throw new IllegalArgumentException("event cannot be null"); + } event.getSettings() .withDynamicComposer(OldEnumComposer.IS_OLD_ENUM, new OldEnumComposer()); }eternalcore-core/src/main/java/com/eternalcode/core/compatibility/CompatibilityService.java (2)
16-17: Consider caching PaperLib version calls.The calls to
PaperLib.getMinecraftVersion()andPaperLib.getMinecraftPatchVersion()could be expensive. Consider caching these values since they won't change during runtime.+private final int minorVersion; +private final int patchVersion; + +public CompatibilityService() { + this.minorVersion = PaperLib.getMinecraftVersion(); + this.patchVersion = PaperLib.getMinecraftPatchVersion(); +} public boolean isCompatible(Class<?> type) { // ... use this.minorVersion and this.patchVersion instead }
22-28: Simplify version comparison logic.The version comparison methods could be simplified and made more readable.
-private boolean isCompatibleTo(Version to, int minor, int patch) { - return minor < to.minor() || minor == to.minor() && patch <= to.patch(); +private boolean isCompatibleTo(Version to, int minor, int patch) { + if (minor < to.minor()) return true; + if (minor > to.minor()) return false; + return patch <= to.patch(); } -private boolean isCompatibleFrom(Version from, int minor, int patch) { - return minor > from.minor() || minor == from.minor() && patch >= from.patch(); +private boolean isCompatibleFrom(Version from, int minor, int patch) { + if (minor > from.minor()) return true; + if (minor < from.minor()) return false; + return patch >= from.patch(); }eternalcore-core/src/main/java/com/eternalcode/core/configuration/ConfigurationManager.java (3)
48-62: Add documentation for the createCdn methodConsider adding Javadoc to document:
- Purpose of the method
- Parameter requirements
- Expected behavior of ConfigurationSettingsSetupEvent
- Return value specifications
Example:
/** * Creates a new CDN instance with custom composers and event publishing. * * @param publisher The publisher for configuration setup events * @param resolverRegistry The registry for notice resolution * @return Configured CDN instance */ private static Cdn createCdn(Publisher publisher, NoticeResolverRegistry resolverRegistry)
57-57: Consider documenting the package-private visibility choiceThe use of
PACKAGE_PRIVATEvisibility for member resolution might have security or architectural implications. Consider adding a comment explaining this design decision.// Package-private visibility is used to [explain reason] .withMemberResolver(Visibility.PACKAGE_PRIVATE);
59-62: Add error handling for event publishingThe event publishing and building process should include error handling to gracefully handle potential failures.
Consider wrapping the event publishing in a try-catch block:
- ConfigurationSettingsSetupEvent event = publisher.publish(new ConfigurationSettingsSetupEvent(cdnSettings)); - - return event.getSettings() - .build(); + try { + ConfigurationSettingsSetupEvent event = publisher.publish(new ConfigurationSettingsSetupEvent(cdnSettings)); + return event.getSettings().build(); + } catch (Exception e) { + throw new ConfigurationException("Failed to setup CDN configuration", e); + }eternalcore-core/src/main/java/com/eternalcode/core/configuration/implementation/PluginConfiguration.java (1)
Line range hint
180-227: Consider adding validation for chat delay.While the Chat implementation is correct, consider adding validation to ensure chatDelay is not set to a negative duration.
@Override @Exclude public void setChatDelay(Duration chatDelay) { + if (chatDelay.isNegative()) { + throw new IllegalArgumentException("Chat delay cannot be negative"); + } this.chatDelay = chatDelay; }eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyBeanCandidate.java (2)
Line range hint
45-48: Include name field in toString() method.The toString() implementation should include the new name field for better debugging and logging.
@Override public String toString() { return "LazyBeanCandidate{" + - "instance=" + this.instance + + "name='" + this.name + '\'' + + ", instance=" + this.instance + '}'; }
23-26: Consider caching the type for better performance.The type is currently computed on every call by invoking getInstance(). Since the instance type won't change, consider caching it after first computation.
public class LazyBeanCandidate implements BeanCandidate { private final String name; private final Supplier<Object> instanceSupplier; private Object instance; + private Class<?> cachedType; @Override public Class<?> getType() { - return this.getInstance().getClass(); + if (this.cachedType == null) { + this.cachedType = this.getInstance().getClass(); + } + return this.cachedType; }eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyFieldBeanCandidate.java (1)
Line range hint
39-43: UpdatetoString()method to display the correct instance typeIn the
toString()method,this.instance.getClass()returns the class of theSupplier, not the class of the actual instance. To display the correct instance type, you should usethis.instanceTypeor retrieve the class fromthis.instance.get().Apply this diff to fix the issue:
@Override public String toString() { return "LazyFieldBeanCandidate{" + - "instanceType=" + this.instance.getClass() + + "instanceType=" + this.instanceType + ", field=" + this.field + '}'; }Alternatively, if you want to reflect the actual runtime class of the instance:
@Override public String toString() { return "LazyFieldBeanCandidate{" + - "instanceType=" + this.instance.getClass() + + "instanceType=" + this.instance.get().getClass() + ", field=" + this.field + '}'; }eternalcore-core/src/main/java/com/eternalcode/core/configuration/migration/MigrationController.java (1)
Line range hint
1-1: Architectural Review: Event System RefactoringThe removal of the
Subscriberinterface while retaining@Subscribeannotations suggests a shift towards a more annotation-driven event system. This architectural change appears throughout multiple components (PlaceholdersSetup,MigrationController). While the changes are consistent, we should:
- Document this architectural change
- Update any developer guidelines about event handling
- Consider adding integration tests for the new event system
Would you like me to help create documentation for the new event handling system or generate integration tests?
.github/HOW_USE_DI.md (2)
129-131: Fix documentation grammar and clarity.The documentation has a few issues:
- Word repetition: "Setup Setup"
- Grammar: "are cannot be registered"
-#### 8. Setup -Setup is an annotation that allows you to register beans in the bean container. -It is used to register dependencies that are cannot be registered in the bean container using annotations. e.g. MiniMessage, AdventureProvider, HikariDataSource, etc. +#### 8. Setup +The @Setup annotation allows you to register beans in the bean container. +It is used to register dependencies that cannot be registered in the bean container using annotations (e.g., MiniMessage, AdventureProvider, HikariDataSource, etc.).🧰 Tools
🪛 LanguageTool
[duplication] ~129-~129: Possible typo: you repeated a word
Context: ...on initialize } } ``` #### 8. Setup Setup is an annotation that allows you to reg...(ENGLISH_WORD_REPEAT_RULE)
[grammar] ~131-~131: Do not use the modal verb ‘cannot’ after the verb ‘be’. Did you mean “cannot”?
Context: ...t is used to register dependencies that are cannot be registered in the bean container usi...(BE_MD)
134-134: Update example class name for consistency.The example class name should reflect current naming conventions:
-public class UserBeanSetup { +public class UserSetup {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (67)
.github/HOW_USE_DI.md(2 hunks)buildSrc/src/main/kotlin/Versions.kt(2 hunks)buildSrc/src/main/kotlin/eternalcode-java.gradle.kts(1 hunks)eternalcore-api-example/src/main/java/com/eternalcode/example/feature/home/ApiHomeListener.java(1 hunks)eternalcore-api/src/main/java/com/eternalcode/core/delay/Delay.java(0 hunks)eternalcore-api/src/main/java/com/eternalcode/core/delay/DelaySettings.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/EternalCore.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/BridgeManagerInitializer.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/adventure/AdventureSetup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/LiteCommandsSetup.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/configurator/config/CommandConfiguration.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/metrics/BStatsMetricsSetup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/bridge/skullapi/SkullAPISetup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/compatibility/Compatibility.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/compatibility/CompatibilityService.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/compatibility/Version.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/configuration/ConfigurationManager.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/configuration/ConfigurationSettingsSetupEvent.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/configuration/compatibility/ConfigurationCompatibilityV21_2.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/configuration/composer/DurationComposer.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/configuration/composer/MaterialComposer.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/configuration/composer/OldEnumComposer.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/configuration/implementation/PluginConfiguration.java(9 hunks)eternalcore-core/src/main/java/com/eternalcode/core/configuration/migration/MigrationController.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/database/DatabaseManagerSetup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AfkCommand.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AfkSettings.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AftPlaceholderSetup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/catboy/CatboyController.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/chat/ChatSettings.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/RepairCommand.java(6 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/helpop/HelpOpCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/helpop/HelpOpSettings.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/home/HomePlaceholderSetup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/randomteleport/RandomTeleportCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/randomteleport/RandomTeleportSettingsImpl.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/Bean.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/Component.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/ConfigurationFile.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/Controller.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/Repository.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/Service.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/Setup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/Task.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanCandidate.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanCandidateContainer.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanCandidatePriorityProvider.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanFactory.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyBeanCandidate.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyFieldBeanCandidate.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/PrioritizedBeanCandidate.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/SimpleBeanCandidate.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/processor/BeanProcessorFactory.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/scan/ComponentBeanCandidateImpl.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/scan/ComponentNameProvider.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/scan/DependencyScanner.java(5 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/scan/DependencyScannerFactory.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/scan/MethodBeanCandidate.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/notice/EternalCoreNoticeBukkitResolver.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/placeholder/PlaceholdersSetup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/publish/LocalPublisher.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/publish/Publisher.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/publish/Subscriber.java(0 hunks)eternalcore-core/src/main/java/com/eternalcode/core/publish/SubscriberUtil.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/scheduler/SchedulerSetup.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManagerSetup.java(1 hunks)eternalcore-plugin/build.gradle.kts(1 hunks)
💤 Files with no reviewable changes (5)
- eternalcore-core/src/main/java/com/eternalcode/core/publish/Subscriber.java
- eternalcore-api/src/main/java/com/eternalcode/core/delay/DelaySettings.java
- eternalcore-api/src/main/java/com/eternalcode/core/delay/Delay.java
- eternalcore-core/src/main/java/com/eternalcode/core/configuration/composer/MaterialComposer.java
- eternalcore-core/src/main/java/com/eternalcode/core/injector/scan/DependencyScannerFactory.java
✅ Files skipped from review due to trivial changes (10)
- eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/Component.java
- eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/ConfigurationFile.java
- eternalcore-core/src/main/java/com/eternalcode/core/compatibility/Version.java
- eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/Service.java
- eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/Setup.java
- eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/Repository.java
- eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/Controller.java
- eternalcore-core/src/main/java/com/eternalcode/core/injector/annotations/component/Task.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/randomteleport/RandomTeleportCommand.java
- eternalcore-core/src/main/java/com/eternalcode/core/bridge/BridgeManagerInitializer.java
🧰 Additional context used
🪛 LanguageTool
.github/HOW_USE_DI.md
[duplication] ~129-~129: Possible typo: you repeated a word
Context: ...on initialize } } ``` #### 8. Setup Setup is an annotation that allows you to reg...
(ENGLISH_WORD_REPEAT_RULE)
[grammar] ~131-~131: Do not use the modal verb ‘cannot’ after the verb ‘be’. Did you mean “cannot”?
Context: ...t is used to register dependencies that are cannot be registered in the bean container usi...
(BE_MD)
🔇 Additional comments (77)
eternalcore-core/src/main/java/com/eternalcode/core/feature/catboy/CatboyController.java (1)
15-15: Good choice using Bukkit's EntityDismountEvent!Using Bukkit's API instead of Spigot-specific APIs improves compatibility and stability.
eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/configurator/config/CommandConfiguration.java (1)
5-5: LGTM! Import addition is appropriate.The Bean annotation import aligns with the PR's objective of modernizing the DI framework.
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/RepairCommand.java (4)
15-15: LGTM!The added import is necessary for the lambda syntax used in the delay initialization.
34-34: Good improvement using a dynamic delay supplier!Using a lambda for delay initialization allows for dynamic updates to the repair delay value at runtime.
78-78: LGTM!The delay marking is consistent with the new delay retrieval method.
168-168: LGTM!The delay marking is consistent with the new delay retrieval method.
eternalcore-core/src/main/java/com/eternalcode/core/scheduler/SchedulerSetup.java (3)
6-6: LGTM! Import changes align with the new DI structure.The import change reflects the reorganization of annotations into a dedicated component package, which improves code organization.
10-10: LGTM! Removal of Subscriber interface improves class focus.The removal of the
Subscriberinterface aligns with the PR's goal of streamlining the event handling mechanism. The class now has a more focused responsibility of providing the scheduler bean.
9-9: LGTM! Annotation update is consistent with the new DI framework.The change from
@BeanSetupto@Setupaligns with the modernization of the annotation-based configuration described in the PR objectives.Let's verify the consistency of this change across the codebase:
✅ Verification successful
✅ Annotation migration verified: complete and consistent
The migration from
@BeanSetupto@Setuphas been fully completed with consistent usage across all setup classes in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that @BeanSetup has been completely replaced with @Setup # Check for any remaining @BeanSetup annotations echo "Checking for any remaining @BeanSetup annotations:" rg "@BeanSetup" # Check for consistent usage of @Setup echo "Verifying @Setup usage pattern:" rg "@Setup" -A 2Length of output: 2694
eternalcore-core/src/main/java/com/eternalcode/core/feature/helpop/HelpOpSettings.java (2)
5-5: Good job simplifying the interface!Removing the inheritance from
DelaySettingsmakes the interface more focused and follows the Interface Segregation Principle (ISP). This change aligns well with the broader restructuring of delay settings management across the codebase.
5-9: Verify the impact on implementing classesThe interface looks clean and focused. However, since this is part of a broader restructuring, let's verify that all implementing classes have been updated accordingly.
✅ Verification successful
All implementations have been properly updated ✅
The implementation in
PluginConfigurationand all usages inHelpOpCommandare correctly using the newgetHelpOpDelay()method. No inconsistencies were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all implementations of HelpOpSettings to verify they've been updated # Search for class implementations ast-grep --pattern 'class $_ implements $$$HelpOpSettings$$$' # Search for any remaining references to the old delay() method rg -A 3 'delay\(\).*Duration' --type javaLength of output: 107
Script:
#!/bin/bash # Try multiple approaches to find implementations and usages # Search for the interface name in general rg "HelpOpSettings" --type java -l # Search for implementations with different patterns ast-grep --pattern 'class $_ implements HelpOpSettings' ast-grep --pattern 'record $_ implements HelpOpSettings' ast-grep --pattern 'class $_ implements $_HelpOpSettings' # Search for both old and new method names rg "getHelpOpDelay|delay\(\)" --type java -A 2Length of output: 4647
eternalcore-plugin/build.gradle.kts (2)
41-41: Verify version compatibility across configurations.The Minecraft version has been updated to 1.21.4 for the development server. Please ensure this aligns with:
- The shadowJar filename indicating "MC 1.17.x-1.21.x" compatibility
- The pluginYml apiVersion set to "1.13"
Run this script to check version declarations across the project:
✅ Verification successful
Version compatibility verified across configurations
The Minecraft version update to 1.21.4 is compatible with:
- Plugin API version 1.13 used across modules
- Spigot/Paper API dependencies (1.21.3)
- Declared compatibility range (1.17.x-1.21.x)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for version declarations and potential inconsistencies # Search for version declarations echo "Searching for version declarations..." rg -g '*.{kt,kts,yml,yaml,xml}' '1\.2[0-9]\.[0-9]|apiVersion|minecraftVersion' # Check plugin.yml if it exists echo "Checking plugin.yml..." fd -g 'plugin.yml' --exec catLength of output: 623
41-41: Verify PR objective alignment.This change updates the Minecraft version, but the PR title suggests it's meant to fix sound serialization. Please clarify if this version update is necessary for the sound serialization fixes.
Let's verify if there are any sound-related changes that depend on this version update:
eternalcore-core/src/main/java/com/eternalcode/core/injector/scan/ComponentBeanCandidateImpl.java (2)
29-32: LGTM! Clean implementation of thegetType()method.The implementation correctly exposes the component type and follows the interface contract. The method is properly annotated with
@Overrideand maintains consistency with Java's type system.
29-32: Verify consistent implementation across bean candidates.Let's ensure this implementation aligns with other
BeanCandidateimplementations in the codebase.✅ Verification successful
Implementation aligns with other bean candidates
The
getType()implementation follows the same straightforward pattern used across all otherBeanCandidateimplementations in the codebase, consistently returning the stored class type.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all implementations of BeanCandidate interface echo "Searching for BeanCandidate implementations..." ast-grep --pattern 'class $_ implements BeanCandidate { $$$ }' # Find all getType method implementations echo "Searching for getType implementations..." ast-grep --pattern 'public Class<?> getType() { $$$ }'Length of output: 15061
eternalcore-core/src/main/java/com/eternalcode/core/bridge/metrics/BStatsMetricsSetup.java (1)
10-10: Verify event handling after Subscriber interface removalThe removal of the
Subscriberinterface aligns with the codebase-wide changes to the event system. However, let's verify that the event handling still works correctly with just the@Subscribeannotation.✅ Verification successful
Event handling implementation verified
The removal of the
Subscriberinterface is consistent with the codebase-wide event handling implementation. Multiple classes are successfully using@Subscribeannotation without the interface, and the event dispatch system inLocalPublisherproperly validates and handles these annotations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new event handling implementation # Check if there are other classes using @Subscribe without Subscriber interface # and verify the event dispatcher implementation # Look for other @Subscribe usages without Subscriber interface ast-grep --pattern 'class $_ { $$$ @Subscribe($_) $$$ }' # Check the event dispatcher implementation rg -A 10 "void dispatch.*Event"Length of output: 98
Script:
#!/bin/bash # First, find event-related files fd -e java -E test "Event|event" # Look for Subscribe annotation usage with ripgrep rg "@Subscribe" -A 3 # Look for event dispatcher/bus implementation rg -l "class.*EventBus|class.*EventDispatcher|class.*EventHandler"Length of output: 10272
eternalcore-core/src/main/java/com/eternalcode/core/bridge/litecommand/LiteCommandsSetup.java (2)
Line range hint
25-45: Well-structured bean configuration with proper error handling!The LiteCommands builder configuration is well-implemented with:
- Proper dependency injection
- Localized error messages
- Integration with Adventure platform
4-4: Verify the migration from@BeanSetupto@SetupThe annotation change and removal of
Subscriberinterface align with the framework's modernization. However, we should verify that all setup classes have been consistently updated.Also applies to: 22-23
✅ Verification successful
Migration to
@Setupverified successfullyThe codebase shows consistent usage of
@Setupannotation across all setup classes, with complete removal of@BeanSetupannotations and Subscriber implementations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining @BeanSetup annotations or Subscriber implementations echo "Checking for remaining @BeanSetup annotations..." rg "@BeanSetup" echo "Checking for remaining Subscriber implementations..." rg "implements.*Subscriber" echo "Verifying consistent @Setup usage..." rg "^@Setup"Length of output: 1138
buildSrc/src/main/kotlin/eternalcode-java.gradle.kts (1)
38-38: Verify Java 21 compatibility with the Minecraft ecosystemThe upgrade to Java 21 is a significant change that requires careful consideration:
- Ensure that your target Minecraft server versions support Java 21
- Verify that all dependencies are compatible with Java 21
- Update development and CI/CD environments accordingly
Let's verify the project's dependencies and Minecraft version compatibility:
Consider documenting the minimum Java version requirement in the project's README or installation guide to help users and contributors set up their environments correctly.
✅ Verification successful
Java 21 upgrade is safe, but documentation needs updating
The upgrade to Java 21 is compatible with the project's ecosystem:
- Project already supports Minecraft 1.21.x
- All dependencies are modern and Java 21 compatible
- Current documentation already specifies "Java 17 or later"
Consider updating the README.md to explicitly mention Java 21 support to help users prepare their environments appropriately.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for Minecraft version references in build files echo "Checking Minecraft version dependencies..." rg -l "spigot|paper|minecraft" --type gradle # Check for explicit Java version requirements in documentation echo "Checking for Java version documentation..." fd "README.md|CONTRIBUTING.md|INSTALL.md"Length of output: 439
Script:
#!/bin/bash # First, let's read the README echo "=== README Content ===" cat README.md # Find all gradle files echo -e "\n=== Gradle Files ===" fd -e gradle -e kts # Check build files for specific dependency patterns echo -e "\n=== Dependency Patterns ===" rg -A 5 "implementation|api.*paper|spigot" --type kotlin --type gradleLength of output: 14512
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanCandidate.java (3)
3-7: LGTM! Good use of priority handling.The interface now extends
Prioritized, which is a clean way to integrate priority-based bean management into the DI framework.
15-18: LGTM! Sensible default priority.The default implementation returning
NORMALpriority is a good choice that follows the principle of least surprise.
28-30: Verify PrioritizedBeanCandidate implementation.The factory method looks good, but let's verify the existence and proper implementation of the
PrioritizedBeanCandidateclass.✅ Verification successful
PrioritizedBeanCandidate implementation verified successfully
The implementation properly follows the decorator pattern and correctly implements all required functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for PrioritizedBeanCandidate class definition rg -l "class PrioritizedBeanCandidate" # Check if it properly implements BeanCandidate ast-grep --pattern 'class PrioritizedBeanCandidate implements BeanCandidate { $$$ }'Length of output: 3952
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/SimpleBeanCandidate.java (1)
22-25: LGTM! Clean implementation.The implementation is straightforward and correctly returns the stored type field.
eternalcore-core/src/main/java/com/eternalcode/core/bridge/skullapi/SkullAPISetup.java (4)
5-5: LGTM! Import changes align with the new annotation.
Line range hint
14-24: LGTM! Core functionality remains intact.The bean initialization and event handling logic maintain good separation of concerns and remain unchanged.
11-11: Verify consistent annotation usage across the codebase.The change from
@BeanSetupto@Setupappears to be part of a broader DI framework refactoring.Let's verify the consistent usage of these annotations:
✅ Verification successful
Migration to @setup annotation is complete and consistent
All occurrences of
@BeanSetuphave been replaced with@Setup, and the new annotation is used consistently across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining @BeanSetup annotations that might have been missed # and confirm consistent @Setup usage echo "Checking for any remaining @BeanSetup annotations:" rg "@BeanSetup" echo -e "\nVerifying consistent @Setup usage:" rg "@Setup" -A 2Length of output: 2706
12-12: Verify event handling mechanism changes.The removal of the
Subscriberinterface while retaining@Subscribeannotation suggests an architectural change in the event handling mechanism.Let's verify the consistency of this change:
✅ Verification successful
Event handling changes are consistent with codebase architecture
The removal of the
Subscriberinterface is part of a broader architectural pattern where event handling is managed through@Subscribeannotations andLocalPublisher. This is consistently implemented across multiple setup classes includingSkullAPISetup.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining Subscriber implementations # and verify the new event handling pattern echo "Checking for any remaining Subscriber implementations:" rg "implements.*Subscriber" echo -e "\nVerifying @Subscribe usage pattern:" ast-grep --pattern 'class $_ { $$$ @Subscribe($_) $$$ }'Length of output: 300
Script:
#!/bin/bash # Search for Subscriber interface definition echo "Checking for Subscriber interface definition:" rg "interface.*Subscriber" # Search for @Subscribe annotations with more specific pattern echo -e "\nChecking for @Subscribe annotations:" rg "@Subscribe" # Look for event registration methods echo -e "\nChecking for event registration patterns:" rg -i "register.*event|subscribe.*event"Length of output: 4215
eternalcore-core/src/main/java/com/eternalcode/core/database/DatabaseManagerSetup.java (5)
6-6: LGTM! Import change aligns with DI framework refactoring.The import change reflects the reorganization of annotation classes into a more structured package hierarchy.
Line range hint
16-28: LGTM! Well-structured bean setup with proper error handling.The implementation follows good practices with proper dependency injection and exception handling.
Line range hint
30-33: LGTM! Clean event subscription with proper resource cleanup.The event handler is well-implemented with explicit event type specification and proper database cleanup on shutdown.
13-13: Verify consistent annotation usage across the codebase.The change from
@BeanSetupto@Setuplooks good, but let's ensure this change is applied consistently.✅ Verification successful
Annotation migration is complete and consistent
The change from
@BeanSetupto@Setuphas been consistently applied across the entire codebase. No remaining instances of@BeanSetupwere found, and@Setupis appropriately used in all setup and initialization classes.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining @BeanSetup annotations that might have been missed # and verify consistent usage of @Setup echo "Checking for any remaining @BeanSetup annotations:" rg "@BeanSetup" echo -e "\nVerifying @Setup annotation usage:" rg "@Setup" --type javaLength of output: 1008
14-14: Verify the new event handling pattern.The removal of
Subscriberinterface while retaining@Subscribeannotation suggests a shift in the event handling pattern. Please confirm if this is the intended design where event subscription is now handled purely through annotations.✅ Verification successful
Event handling pattern change is consistent
The removal of the
Subscriberinterface is part of a consistent architectural change where event handling is now managed purely through@Subscribeannotations. This pattern is uniformly implemented across all setup classes in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the consistency of the new event handling pattern echo "Checking for remaining Subscriber interface implementations:" ast-grep --pattern 'class $_ implements $$$Subscriber$$$' echo -e "\nChecking @Subscribe annotation usage without Subscriber interface:" rg "@Subscribe" --type java -C 2Length of output: 8131
eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AftPlaceholderSetup.java (1)
18-18: Verify event handling mechanism after Subscriber removalThe class no longer implements the
Subscriberinterface, but still uses the@Subscribeannotation. This might lead to the event handler not being registered properly.Let's verify the event handling implementation:
eternalcore-core/src/main/java/com/eternalcode/core/injector/scan/DependencyScanner.java (2)
25-25: LGTM! Well-designed field for type filtering.The field is properly encapsulated and thread-safe.
32-38: LGTM! Method renames improve clarity.The new method names better reflect their purpose in the builder pattern.
Also applies to: 50-53
eternalcore-core/src/main/java/com/eternalcode/core/injector/scan/MethodBeanCandidate.java (1)
31-34: LGTM! Clean implementation ofgetType().The implementation correctly returns the method's return type, maintaining consistency with the bean candidate system.
Let's verify the interface contract implementation across the codebase:
✅ Verification successful
Implementation verified and consistent with BeanCandidate contract
The
getType()implementation inMethodBeanCandidatecorrectly returns the method's return type and follows the same pattern as other implementations in the codebase. All BeanCandidate implementations maintain a consistent contract while appropriately handling their specific contexts.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent implementation of getType() across BeanCandidate implementations # Expected: All BeanCandidate implementations should have getType() method # Find all implementations of BeanCandidate ast-grep --pattern 'class $_ implements $$$BeanCandidate$$$ { $$$ }' # Search for getType method implementations ast-grep --pattern 'public Class<?> getType() { $$$ }'Length of output: 441
Script:
#!/bin/bash # Find BeanCandidate interface definition ast-grep --pattern 'interface BeanCandidate { $$$ }' # Find implementations with more flexible pattern ast-grep --pattern 'class $_ implements $_BeanCandidate' # Find class definitions containing BeanCandidate rg "class.*BeanCandidate" -lLength of output: 900
Script:
#!/bin/bash # Show content of potential implementation files to verify for file in LazyBeanCandidate.java LazyFieldBeanCandidate.java PrioritizedBeanCandidate.java SimpleBeanCandidate.java ComponentBeanCandidateImpl.java MethodBeanCandidate.java; do echo "=== $file ===" rg "class.*implements.*BeanCandidate" -A 5 $(fd "$file") echo "--- getType() implementation ---" rg "getType\(\)" -A 3 $(fd "$file") echo -e "\n" doneLength of output: 6075
eternalcore-core/src/main/java/com/eternalcode/core/feature/home/HomePlaceholderSetup.java (2)
Line range hint
41-50: LGTM! Well-structured placeholder registration.The placeholder setup is well-organized using Stream API for registering multiple placeholders. The code is clean and maintainable.
20-20: Verify event handling after removing Subscriber interface.The removal of
implements Subscriberinterface might affect the event handling mechanism. Please ensure that:
- The event system can still detect and process
@Subscribeannotations without the interface- The
setUpmethod is still being called during initializationLet's verify the event handling setup:
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanCandidatePriorityProvider.java (1)
10-10: Verify the implementation of SubscriberUtil.isSubscriber.Let's ensure the subscriber detection logic is robust and handles all cases correctly.
✅ Verification successful
Implementation of SubscriberUtil.isSubscriber is correct and robust
The utility properly detects subscriber classes by checking for @subscribe annotations on their methods, with appropriate null checks and consistent usage across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the implementation of SubscriberUtil.isSubscriber # Search for the implementation ast-grep --pattern 'class SubscriberUtil { $$$ isSubscriber($_) { $$$ } $$$ }' # Look for any tests covering this utility rg -l "SubscriberUtil" --glob "**/*Test.java"Length of output: 138
Script:
#!/bin/bash # Try different approaches to locate SubscriberUtil # Find any file containing SubscriberUtil rg -l "SubscriberUtil" # Look for isSubscriber method implementation rg "isSubscriber.*\(" -A 5 # Find import statements referencing SubscriberUtil rg "import.*SubscriberUtil"Length of output: 3095
eternalcore-core/src/main/java/com/eternalcode/core/feature/randomteleport/RandomTeleportSettingsImpl.java (1)
79-79: Verify the removal ofGRASSfrom safe blocks.The removal of
Material.GRASSfromairBlocksmeans that players can no longer be teleported onto grass blocks. This could significantly reduce the number of valid teleport locations, especially in overworld biomes where grass blocks are common safe landing spots.Please confirm if this change is intentional. If it is, consider the following implications:
- Players might have fewer safe spots to teleport to
- Teleportation in grass-heavy biomes might require more attempts
- The
teleportAttemptsvalue (currently 10) might need to be increased to compensateLet's verify the impact of this change:
eternalcore-core/src/main/java/com/eternalcode/core/feature/chat/ChatSettings.java (1)
5-5: Verify the impact of removingextends DelaySettings.The
ChatSettingsinterface no longer extendsDelaySettings, and the default methoddelay()has been removed. Ensure that any implementations or usages ofChatSettingsthat relied on the inheriteddelay()method are updated accordingly. This change could affect classes that expectChatSettingsto provide adelay()method, potentially leading to compilation errors or runtime issues.Run the following script to identify any usages of
delay()associated withChatSettings:This script searches for instances where
ChatSettingsis used and checks if thedelay()method is called on those instances.✅ Verification successful
No issues found with removing
extends DelaySettingsThe removal of
extends DelaySettingsfrom theChatSettingsinterface appears to be safe. No usages of thedelay()method were found in relation toChatSettingsinstances, and the only implementation (PluginConfiguration.Chat) is contained within the same codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find references to the `delay()` method on `ChatSettings` instances. # Search for method calls to `delay()` on objects of type `ChatSettings`. rg --java '$[a-zA-Z0-9_]+ = .*ChatSettings.*;' -A 5 | rg '\.delay\('Length of output: 479
Script:
#!/bin/bash # Find implementations of ChatSettings interface ast-grep --pattern 'class $_ implements $$$ChatSettings$$$' # Find any references to delay() method in relation to ChatSettings rg -l "ChatSettings" | xargs rg "delay\(\)" # Find type references to ChatSettings rg "ChatSettings" -A 2 -B 2Length of output: 6149
eternalcore-core/src/main/java/com/eternalcode/core/configuration/composer/DurationComposer.java (4)
14-14: Great improvement in duration parsing!The switch to
DurationParser.TIME_UNITS.parseprovides a more robust and standardized way of parsing durations. This change maintains proper error handling while potentially supporting a wider range of duration formats.
19-19: Excellent symmetry with deserialization!The use of
DurationParser.TIME_UNITS.formatprovides a clean and consistent approach that aligns well with the deserialization logic. This change eliminates manual string manipulation, making the code more maintainable.
3-3: Verify litecommands dependency declarationThe new import suggests a dependency on the litecommands library. Let's verify its declaration in the project configuration.
✅ Verification successful
Dependency verification successful
The litecommands dependency is properly declared in the core module's build configuration, supporting the new import.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if litecommands dependency is properly declared # Expected: Find dependency declaration in build files # Search for litecommands in gradle or maven files fd -e gradle -e kts -e xml | xargs rg -l "litecommands"Length of output: 269
Script:
#!/bin/bash # Check the specific dependency declaration in core module cat eternalcore-core/build.gradle.kts | rg "litecommands"Length of output: 251
14-19: Verify duration format compatibilityThe switch to
DurationParser.TIME_UNITSchanges how durations are parsed and formatted. Let's verify existing duration strings in the codebase are compatible with the new format.✅ Verification successful
Duration format compatibility verified
The switch to
DurationParser.TIME_UNITSmaintains consistent duration parsing and formatting across the codebase. All duration strings follow the standard format (e.g., "5s", "1m", "1h", "1d") and existing tests verify this behavior.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find and analyze duration string formats in the codebase # Expected: Identify potential compatibility issues # Search for duration string patterns rg -A 2 '"(\d+[smhd]|\d+\s*(second|minute|hour|day))"' # Look for existing Duration.parse calls that might need updates ast-grep --pattern 'Duration.parse($_)'Length of output: 367
Script:
#!/bin/bash # Search for DurationParser implementation and usage ast-grep --pattern 'class DurationParser' # Look for duration strings in configuration files rg -t yaml -t properties -t xml '".*(\d+[smhd]|\d+\s*(second|minute|hour|day)).*"' # Search for duration-related test files fd -e java -E target --exec grep -l -i "duration" {} # Check for other duration string patterns rg "Duration" -A 2 -g "*.java"Length of output: 68771
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/PrioritizedBeanCandidate.java (3)
5-9: Well-structured implementation of the decorator pattern!The class properly encapsulates the decorated
BeanCandidateand uses immutable fields, following best practices for the decorator pattern.
15-33: Clean and correct implementation of interface methods!The methods properly delegate to the encapsulated candidate while maintaining the interface contract. The implementation is concise and follows the single responsibility principle.
5-34: Verify integration with BeanCandidateContainerLet's ensure this implementation integrates correctly with the container and priority handling system.
✅ Verification successful
✓ Priority handling implementation verified
The
PrioritizedBeanCandidateis properly integrated with the DI framework's priority system, working in conjunction withBeanCandidateContainerandBeanCandidatePriorityProviderto provide a robust and thread-safe priority handling mechanism.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration with BeanCandidateContainer and usage patterns # Check how PrioritizedBeanCandidate is used rg -A 5 "PrioritizedBeanCandidate" # Verify BeanCandidateContainer integration ast-grep --pattern 'class BeanCandidateContainer { $$$ add($$$) { $$$ } $$$ }'Length of output: 1942
Script:
#!/bin/bash # Search for BeanCandidateContainer with less strict pattern rg -l "BeanCandidateContainer" # Look for PriorityLevel usage to understand priority handling rg -A 5 "PriorityLevel" # Find any sorting/priority-related code in container rg -A 10 "sort|priority|compareTo" --type javaLength of output: 43903
eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AfkSettings.java (2)
Line range hint
5-13: Inconsistency with PR objectivesThe changes to this file seem unrelated to the PR's stated objective of "Fix sound serdes". This might indicate that either:
- The PR title needs updating to reflect the broader scope of changes
- These changes should be in a separate PR focused on DI framework refactoring
Let's verify if there are any sound-related changes in this PR:
5-5: Verify the removal of DelaySettings interfaceThe removal of
extends DelaySettingsis part of a larger refactoring to decouple delay-related functionality. However, this change might impact existing code that relies on thedelay()method inherited fromDelaySettings.Let's verify the impact on consumers:
✅ Verification successful
Removal of DelaySettings interface is safe
The removal of
extends DelaySettingsis a valid refactoring as the delay functionality is properly handled within theAfkimplementation class using the dedicatedafkCommandDelayfield. No code relies on the inheriteddelay()method.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all references to AfkSettings.delay() to ensure they're updated # Search for any remaining calls to delay() method on AfkSettings instances ast-grep --pattern 'AfkSettings $var = $$$; $$$; $var.delay($$$)' # Find implementations to ensure they're updated ast-grep --pattern 'class $_ implements AfkSettings'Length of output: 122
Script:
#!/bin/bash # Find all references to AfkSettings and DelaySettings rg "DelaySettings|AfkSettings" -l # Find any delay-related method calls in AFK context fd -e java -e kt --exec grep -l "delay" {} # Look for the DelaySettings interface definition fd -e java -e kt --exec grep -l "interface.*DelaySettings" {}Length of output: 1995
Script:
#!/bin/bash # Check the implementation details in AFK-related files echo "=== AfkTask.java ===" rg -A 5 -B 5 "class AfkTask" eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AfkTask.java echo -e "\n=== AfkServiceImpl.java ===" rg -A 5 -B 5 "class AfkServiceImpl" eternalcore-core/src/main/java/com/eternalcode/core/feature/afk/AfkServiceImpl.java echo -e "\n=== PluginConfiguration.java AfkSettings section ===" rg -A 10 "AfkSettings" eternalcore-core/src/main/java/com/eternalcode/core/configuration/implementation/PluginConfiguration.javaLength of output: 2381
buildSrc/src/main/kotlin/Versions.kt (3)
3-4: Verify compatibility with Minecraft API version upgradeThe upgrade from 1.19.4 to 1.21.3 is a significant version jump that might introduce breaking changes. Please ensure:
- All plugin features are compatible with the new API
- Server administrators are notified about the minimum required Minecraft version
Let's check for potential breaking changes:
✅ Verification successful
API version upgrade appears safe
The codebase primarily uses stable Bukkit API features (Location, Server, World) that maintain backward compatibility. No usage of version-specific or deprecated features detected. Basic testing recommended but no major compatibility concerns identified.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Spigot/Paper API usage that might be affected rg -l "org\.bukkit|org\.spigot" | xargs ast-grep --pattern 'import org.bukkit.$_' rg -l "io\.papermc" | xargs ast-grep --pattern 'import io.papermc.$_'Length of output: 18575
Line range hint
1-48: Verify removal of PIXEL_WIDTH dependencyThe AI summary mentions that PIXEL_WIDTH was removed. Please ensure that:
- All references to this dependency have been properly removed/replaced
- Any functionality depending on it has been migrated
Let's verify the removal:
✅ Verification successful
✓ PIXEL_WIDTH dependency has been completely removed
No references to PIXEL_WIDTH were found in the codebase, including build files and source code, confirming its successful removal.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to PIXEL_WIDTH rg -i "pixel.?width"Length of output: 22
Script:
#!/bin/bash # Search for potential dependency declarations and imports rg -i "pixel" --type gradle --type kotlin --type javaLength of output: 53
7-7: Verify changelog for library updatesThe following libraries have been updated:
- MULTIFICATION: 1.1.4 → 1.2.1
- LITE_COMMANDS: 3.8.0 → 3.9.7
Please ensure:
- Review changelogs for any breaking changes
- Test the new features/fixes that might affect the codebase
Let's check the usage of these libraries:
Also applies to: 26-26
eternalcore-core/src/main/java/com/eternalcode/core/publish/Publisher.java (2)
7-7: LGTM! Enhanced type safety with generic return type.The change to return the event object enables method chaining while preserving type information, which is a good improvement.
Line range hint
1-9: Verify impact of breaking changes.The signature change from
void publish(PublishEvent)to<E extends PublishEvent> E publish(E)is a breaking change that will affect all implementations of this interface.Let's verify the impact:
eternalcore-core/src/main/java/com/eternalcode/core/publish/SubscriberUtil.java (2)
1-9: LGTM! Well-structured utility class.The class follows best practices for utility classes with appropriate final modifier and private constructor.
1-24: Verify migration from Subscriber interface.Let's ensure all previous Subscriber implementations are properly detected and no references to the old interface remain.
✅ Verification successful
Migration to annotation-based subscriber detection is complete and correct
The new SubscriberUtil is properly integrated and used consistently throughout the codebase, with no remnants of the old Subscriber interface.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify complete migration from Subscriber interface # Check for any remaining references to the old Subscriber interface echo "Checking for remaining Subscriber interface references..." rg -l "implements.*Subscriber" || echo "No direct implementations found" rg -l "import.*Subscriber" || echo "No imports found" # Find potential subscriber classes (classes with @Subscribe methods) echo -e "\nFinding potential subscriber classes..." ast-grep --pattern 'class $_ { $$$ @Subscribe $$$ }'Length of output: 618
Script:
#!/bin/bash # Check the implementation details of classes using @Subscribe echo "=== BeanCandidatePriorityProvider.java ===" rg -A 5 "@Subscribe" eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanCandidatePriorityProvider.java echo -e "\n=== BeanProcessorFactory.java ===" rg -A 5 "@Subscribe" eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/processor/BeanProcessorFactory.javaLength of output: 422
Script:
#!/bin/bash echo "=== BeanCandidatePriorityProvider.java ===" cat eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanCandidatePriorityProvider.java echo -e "\n=== BeanProcessorFactory.java ===" cat eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/processor/BeanProcessorFactory.javaLength of output: 6329
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanFactory.java (1)
31-34: Verify the impact on existing bean registrations.Since this introduces a new priority system for beans, we should verify its integration with existing code.
Let's check for potential impacts:
✅ Verification successful
Priority system implementation is properly integrated
The new priority system is correctly implemented with proper handling of bean priorities and is already being used in the codebase with BeanCandidatePriorityProvider.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for existing bean registrations and potential priority requirements # Look for bean registrations echo "Checking for bean registrations..." rg -A 2 "addCandidate|@Setup|@BeanSetup" --type java # Look for potential priority-sensitive scenarios (multiple beans of same type) echo "Checking for multiple bean implementations..." ast-grep --pattern 'class $_ implements $_' --type javaLength of output: 6861
Script:
#!/bin/bash # Check priority provider implementation and look for multiple implementations echo "Checking priority provider implementation..." rg -p "class BeanCandidatePriorityProvider" -A 10 --type java echo "Looking for multiple implementations of interfaces..." rg "implements \w+" --type java echo "Checking for priority-related annotations or configurations..." rg "Priority|@Priority|priority" --type javaLength of output: 39890
eternalcore-core/src/main/java/com/eternalcode/core/publish/LocalPublisher.java (3)
63-63: LGTM! Great improvement to type safety.The generic type parameter
<E extends PublishEvent>preserves the exact event type and enables method chaining. This change aligns well with modern event publishing patterns.
67-67: LGTM! Consistent handling of unsubscribed events.Returning the event when no subscribers exist maintains consistency and enables event state inspection even when unhandled.
76-77: Consider thread safety implications.The return statement correctly completes the event publishing chain. However, since the event object can be modified by multiple subscribers, consider making
PublishEventimplementations immutable to prevent potential race conditions in concurrent scenarios.Let's verify if
PublishEventimplementations are mutable:eternalcore-core/src/main/java/com/eternalcode/core/configuration/composer/OldEnumComposer.java (2)
1-19: LGTM! Well-structured class with appropriate interfacesThe class is properly organized with clear responsibilities for handling
OldEnumserialization and deserialization.
1-54: Verify integration with the configuration systemLet's verify how this composer is registered and used within the configuration system.
✅ Verification successful
Implementation verified and properly integrated
The OldEnumComposer is correctly integrated into the configuration system through ConfigurationCompatibilityV21_2, with proper version compatibility handling (>= 21.2) and safe reflection usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for composer registration and usage echo "Searching for composer registration..." rg -l "OldEnumComposer" --type java echo "Searching for configuration setup..." ast-grep --pattern 'class $_ implements ConfigurationSetup { $$$ }'Length of output: 490
Script:
#!/bin/bash # Check how OldEnumComposer is used in the compatibility class echo "Checking compatibility class implementation..." cat eternalcore-core/src/main/java/com/eternalcode/core/configuration/compatibility/ConfigurationCompatibilityV21_2.java echo -e "\nSearching for OldEnum usage..." rg "OldEnum" -A 3 --type javaLength of output: 5829
eternalcore-core/src/main/java/com/eternalcode/core/injector/scan/ComponentNameProvider.java (1)
6-6: Confirm the Necessity of MakingComponentNameProviderPublicChanging
ComponentNameProviderto public increases its visibility beyond its original package. Ensure that this change is intentional and does not expose internal APIs unnecessarily, which could lead to tight coupling or security concerns.eternalcore-core/src/main/java/com/eternalcode/core/configuration/ConfigurationSettingsSetupEvent.java (1)
18-20: Consider thread-safety implications.The mutable state could lead to thread-safety issues if the event is handled by multiple subscribers concurrently. Consider:
- Making the field final and removing the setter
- Or documenting the thread-safety guarantees
eternalcore-core/src/main/java/com/eternalcode/core/configuration/ConfigurationManager.java (1)
37-46: LGTM! Constructor changes are well-structuredThe addition of the
Publisherparameter is well-integrated, maintaining proper dependency injection patterns and following the single responsibility principle.eternalcore-core/src/main/java/com/eternalcode/core/configuration/implementation/PluginConfiguration.java (3)
12-12: LGTM!The addition of the Bean annotation import aligns with the dependency injection framework changes.
Line range hint
82-104: LGTM! Well-structured TeleportAsk implementation.The implementation correctly provides configuration for teleport request expiration and timer settings.
249-256: Verify usage of renamed repair delay method.The Repair class has undergone a breaking change:
- Removed
DelaySettingsinterface- Renamed
delay()torepairDelay()Please ensure all callers are updated.
✅ Verification successful
Breaking change appears safe to proceed
The renaming of
delay()torepairDelay()in the Repair class appears safe as:
- No direct calls to the old
delay()method were found in the codebase- RepairCommand uses a separate
Delay<UUID>system for cooldown management- The removal of
DelaySettingsinterface has no impact as no implementations were found🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential callers of the old delay method rg -A 3 "repair.*delay\(\)" --type java # Search for implementations or references to DelaySettings ast-grep --pattern 'class $_ implements DelaySettings { $$$ }'Length of output: 107
Script:
#!/bin/bash # Search for Repair class references rg -B 2 -A 2 "Repair" --type java # Search for repair-related delay calls with more variations rg -B 2 -A 2 "\.delay\(\)" --type java # Search for configuration repair settings rg -B 2 -A 2 "repair.*config" --type javaLength of output: 12451
Script:
#!/bin/bash # Search for delay field and initialization in RepairCommand rg -B 5 -A 5 "private.*delay" eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/RepairCommand.java # Search for any repair delay initialization ast-grep --pattern 'delay = $_.repairDelay()'Length of output: 488
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyBeanCandidate.java (2)
7-7: LGTM! Field and constructor changes look good.The name field is properly encapsulated and initialized through the constructor.
Also applies to: 11-12
34-34: LGTM! BeanHolder creation properly includes name parameter.The modification correctly integrates the name field while maintaining type safety.
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanCandidateContainer.java (1)
17-18:⚠️ Potential issuePrevent duplicate candidates by ensuring uniqueness in the
candidateslistChanging
candidatesfrom aSetto aMutablePrioritizedListmay introduce duplicateBeanCandidateentries, which could lead to unexpected behavior during candidate retrieval. To maintain the uniqueness of candidates, consider checking for duplicates before adding a new candidate.Apply this diff to prevent duplicates:
void addCandidate(BeanCandidate candidate) { PriorityLevel priorityLevel = this.priorityProvider.apply(candidate); if (!priorityLevel.equals(candidate.getPriority())) { candidate = BeanCandidate.prioritized(candidate, priorityLevel); } synchronized (this.lock) { + if (!this.candidates.contains(candidate)) { this.candidates.add(candidate); + } } }Likely invalid or redundant comment.
eternalcore-core/src/main/java/com/eternalcode/core/notice/EternalCoreNoticeBukkitResolver.java (1)
4-4: LGTM! Annotation update aligns with DI framework changes.The transition from
@BeanSetupto@Setupis consistent with the broader DI framework refactoring.Let's verify the removed imports are not used elsewhere in this class:
Also applies to: 9-9
✅ Verification successful
Import changes verified - old resolver classes safely removed
The removed resolver imports (ActionbarResolver, ChatResolver, SoundAdventureResolver) are not referenced anywhere in the codebase, and the file now correctly uses the new resolver implementations from the multification framework.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any usage of removed resolver imports rg -l "ActionbarResolver|ChatResolver|SoundAdventureResolver" "eternalcore-core/src/main/java/com/eternalcode/core/notice/"Length of output: 123
Script:
#!/bin/bash # Search for resolver classes in the entire codebase rg -l "class (ActionbarResolver|ChatResolver|SoundAdventureResolver)" # Search for any remaining resolver references rg "(Actionbar|Chat|SoundAdventure)Resolver" -A 2 -B 2 # Look for resolver-related patterns using ast-grep ast-grep --pattern 'class $_ implements Resolver { $$$ }'Length of output: 190
Script:
#!/bin/bash # Search for resolver-related imports rg "^import.*Resolver" # Search for resolver interface definitions rg "interface.*Resolver" # Search for any class implementing anything with Resolver in the name rg "class.*implements.*Resolver"Length of output: 3209
eternalcore-core/src/main/java/com/eternalcode/core/placeholder/PlaceholdersSetup.java (1)
10-10: Verify event handling after Subscriber interface removal.While removing the
Subscriberinterface aligns with the DI refactoring, we should ensure that the@Subscribeannotations onsetUpandsetUpPlaceholdersmethods still function correctly without the interface.Let's check the event handling implementation:
✅ Verification successful
Event handling remains functional after Subscriber interface removal
The @subscribe annotation is part of a custom event system that works independently of the Subscriber interface. This is evidenced by:
- Consistent usage across multiple setup classes in the codebase
- Proper validation in LocalPublisher
- Maintained functionality through annotation-based event handling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if @Subscribe is still functional without Subscriber interface ast-grep --pattern 'class $_ { $$$ @Subscribe($_) $_ $_($$$) { $$$ } }'Length of output: 85
Script:
#!/bin/bash # Search for @Subscribe annotations rg "@Subscribe" -A 2 # Look for the specific class implementation rg "class PlaceholdersSetup" -A 10 # Check for event-related imports rg "import.*Subscribe" --type javaLength of output: 8426
eternalcore-core/src/main/java/com/eternalcode/core/bridge/adventure/AdventureSetup.java (1)
7-7: LGTM! Consistent with DI framework updates.The annotation change from
@BeanSetupto@Setupaligns with the framework refactoring while preserving the Adventure configuration functionality.Also applies to: 13-13
eternalcore-core/src/main/java/com/eternalcode/core/configuration/migration/MigrationController.java (1)
12-12: Verify migration initialization after Subscriber interface removal.The removal of the
Subscriberinterface is part of the DI refactoring, but we should ensure that:
- The
@Subscribeannotation ononMigrationremains functional- The migration process is still triggered during initialization
Let's verify the migration event handling:
✅ Verification successful
Migration initialization remains functional after Subscriber interface removal
The event handling mechanism has been successfully modernized to use annotation-based discovery:
- The
@Subscribeannotation is properly maintained on theonMigrationmethod- The method signature and implementation remain unchanged, ensuring the migration process works as before
- The removal of the
Subscriberinterface aligns with the DI refactoring, relying on annotation-based discovery instead of explicit interface implementation🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for EternalInitializeEvent handlers ast-grep --pattern '@Subscribe $_ $_($_ event) { $$$ }' # Check if there are other classes still implementing Subscriber rg "implements.*Subscriber" "eternalcore-core/src/main/"Length of output: 2504
Script:
#!/bin/bash # Check for event bus registration patterns rg -A 5 "eventBus|EventBus" "eternalcore-core/src/main/" # Check for any class that might register event handlers ast-grep --pattern 'register($$$)'Length of output: 93
eternalcore-core/src/main/java/com/eternalcode/core/translation/TranslationManagerSetup.java (1)
5-5: LGTM! Annotation update aligns with DI framework changes.The replacement of
@BeanSetupwith@Setupis consistent with the broader dependency injection framework refactoring.Also applies to: 11-11
...n/java/com/eternalcode/core/bridge/litecommand/configurator/config/CommandConfiguration.java
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/RepairCommand.java
Outdated
Show resolved
Hide resolved
| for (Field field : ReflectUtil.getAllSuperFields(componentClass)) { | ||
| if (!field.isAnnotationPresent(Bean.class)) { | ||
| continue; | ||
| } | ||
|
|
||
| Bean bean = field.getAnnotation(Bean.class); | ||
| BeanCandidate beanCandidate = new LazyFieldBeanCandidate(this.dependencyInjector, componentClass, field, bean); | ||
|
|
||
| beanCandidates.add(beanCandidate); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for field accessibility.
The field scanning should handle potential SecurityException when accessing fields from different protection domains.
for (Field field : ReflectUtil.getAllSuperFields(componentClass)) {
+ try {
if (!field.isAnnotationPresent(Bean.class)) {
continue;
}
Bean bean = field.getAnnotation(Bean.class);
BeanCandidate beanCandidate = new LazyFieldBeanCandidate(this.dependencyInjector, componentClass, field, bean);
beanCandidates.add(beanCandidate);
+ } catch (SecurityException e) {
+ // Log warning or throw a wrapped exception
+ logger.warn("Unable to access field in class {}: {}", componentClass.getName(), e.getMessage());
+ }
}Committable suggestion skipped: line range outside the PR's diff.
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/PrioritizedBeanCandidate.java
Show resolved
Hide resolved
...re-core/src/main/java/com/eternalcode/core/injector/bean/processor/BeanProcessorFactory.java
Outdated
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyBeanCandidate.java
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyFieldBeanCandidate.java
Outdated
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyFieldBeanCandidate.java
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/BeanCandidateContainer.java
Show resolved
Hide resolved
eternalcore-api-example/src/main/java/com/eternalcode/example/feature/home/ApiHomeListener.java
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyFieldBeanCandidate.java (1)
Line range hint
40-45: Fix instance type in toString()The current implementation shows the Supplier class instead of the actual instance type. Consider using the cached
instanceTypefield instead.@Override public String toString() { return "LazyFieldBeanCandidate{" + - "instanceType=" + this.instance.getClass() + + "instanceType=" + this.instanceType + ", field=" + this.field + '}'; }
🧹 Nitpick comments (1)
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyFieldBeanCandidate.java (1)
30-32: Add parameter validationConsider adding null checks for the constructor parameters to fail fast with clear error messages.
public LazyFieldBeanCandidate(DependencyInjector dependencyInjector, Class<?> componentClass, Field field, Bean bean) { + if (dependencyInjector == null) { + throw new IllegalArgumentException("dependencyInjector cannot be null"); + } + if (componentClass == null) { + throw new IllegalArgumentException("componentClass cannot be null"); + } this(() -> dependencyInjector.newInstance(componentClass), field, bean); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyFieldBeanCandidate.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/processor/BeanProcessorFactory.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/processor/BeanProcessorFactory.java
🔇 Additional comments (2)
eternalcore-core/src/main/java/com/eternalcode/core/injector/bean/LazyFieldBeanCandidate.java (2)
3-12: LGTM! Well-structured field declarationsGood use of
Supplier<Object>for lazy initialization and proper encapsulation with private final fields.
14-28: LGTM! Robust constructor implementationThe constructor properly handles field accessibility and error cases. Good improvement on the exception message compared to the previous version.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Refactoring
BeanSetupannotation toSetup.DelaySettingsinterface.