Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several enhancements and new features to the EternalEconomy plugin. It updates dependency versions in Versions.kt, including adding TRIUMPH_GUI and LITE_SKULL_API while removing ADVENTURE_PLATFORM_BUKKIT and ADVENTURE_API. The core plugin now includes TriumphGUI for GUI and LiteSkullAPI, along with an AdminGenerateCommand for generating accounts. A MiniMessageHolder interface was created, and the AccountRepositoryImpl was refactored to use AccountTable and create a composite index for leaderboard queries. The plugin configuration was updated with new settings for leaderboard GUI, and a LeaderboardConfigurer was added to manage leaderboard commands and GUI. The review comments suggest clarifying the removal of Adventure dependencies, commend the detailed comment for createIndexes(), suggest a non-blocking delay mechanism for the account generation command, recommend using page.totalCount() for accurate player count, and propose moving the RESET_ITEM component to MiniMessageHolder for reusability.
| const val JETBRAINS_ANNOTATIONS = "26.0.2-1" | ||
|
|
||
| const val ADVENTURE_PLATFORM_BUKKIT = "4.4.1" | ||
| const val ADVENTURE_API = "4.24.0" |
There was a problem hiding this comment.
The removal of ADVENTURE_PLATFORM_BUKKIT and ADVENTURE_API is noted. Could you please clarify if their functionality is now entirely covered by eternalcode-commons-adventure or if there's another reason for their removal? This helps in understanding the dependency graph and potential impacts.
| try { | ||
| TableUtils.createTableIfNotExists(databaseManager.connectionSource(), AccountWrapper.class); | ||
| TableUtils.createTableIfNotExists(databaseManager.connectionSource(), AccountTable.class); | ||
| this.createIndexes(); |
There was a problem hiding this comment.
The comment for createIndexes() is very detailed and provides excellent context for why this custom index is necessary. This is a great example of clear documentation for a performance-critical change.
...nomy-core/src/main/java/com/eternalcode/economy/command/impl/admin/AdminGenerateCommand.java
Outdated
Show resolved
Hide resolved
| int players = page.maxPages() <= 1 | ||
| ? page.entries().size() | ||
| : (page.maxPages() - 1) * pageSize + page.entries().size(); |
There was a problem hiding this comment.
The calculation for players here might not always accurately reflect the total number of players, especially if the last page is not full or if the totalCount from the snapshot is directly available. It would be more accurate and robust to use page.totalCount() directly, which is already part of the LeaderboardPage record, to represent the total number of players in the leaderboard.
int players = (int) page.totalCount();| @Exclude | ||
| private static final Component RESET_ITEM = Component.text() | ||
| .decoration(TextDecoration.ITALIC, false) | ||
| .build(); |
There was a problem hiding this comment.
The RESET_ITEM component is a useful utility for removing default italics. To avoid duplication and promote reusability, consider moving this static final field to the MiniMessageHolder interface or a dedicated utility class. This would centralize its definition and make it accessible across all classes that implement MiniMessageHolder.
…ing the codebase.
1 participant
No description provided.