GH-986 GH-1096 Moved sounds and enhanced death messages #1097
Conversation
📦 Development Build ReadyWarning Do not use this build in production. It is for testing purposes only and may be unstable. |
P1otrulla
unassigned
Rollczi,
mydelkonivea,
vLuckyyy,
igoyek,
Jakubk15,
CitralFlo,
VerniQ and
imDMK
P1otrulla
requested review from
Rollczi,
mydelkonivea,
vLuckyyy,
igoyek,
Jakubk15,
VerniQ and
imDMK
![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 refactors the death message system to be more detailed and moves sound configuration into the translation files, which is a great improvement. The new death message logic is well-structured, handling various scenarios like PvP, projectile, and environmental deaths separately.
My review focuses on a few areas for improvement. I've identified some duplicated logic in DeathMessageController that could be refactored for better maintainability. More importantly, there are a couple of cases where the new death messages defined in the English translation file will not be triggered correctly due to issues in the controller logic, specifically for TNT and Wither Skull deaths. I've also noted some inconsistencies between the English and Polish translations that should be addressed. Overall, these are great enhancements, and with a few fixes, they will be perfect.
...alcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java
Outdated
Show resolved
Hide resolved
...alcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java
Outdated
Show resolved
Hide resolved
...ore-core/src/main/java/com/eternalcode/core/feature/deathmessage/DeathMessageController.java
Outdated
Show resolved
Hide resolved
...alcore-core/src/main/java/com/eternalcode/core/translation/implementation/PLTranslation.java
Outdated
Show resolved
Hide resolved
…athmessage/DeathMessageController.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
While PlayerJoinSoundListener and PlayerQuitSoundListener were indeed moved to appropriate joinmessage/quitmessage packages that utilized Multification notices, the PlayerChatSoundListener was removed entirely, and—as far as I checked—there is no feature in EternalCore that replaced the removed controller. I'm hereby requesting re-adding that controller and moving it to the chat package.
…enhanced-death-messages # Conflicts: # eternalcore-core/src/main/java/com/eternalcode/core/configuration/implementation/PluginConfiguration.java
eternalcore-core/src/main/java/com/eternalcode/core/feature/chat/ChatSoundController.java
Outdated
Show resolved
Hide resolved
…at/ChatSoundController.java Co-authored-by: Jakub Kędziora <[email protected]>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the sound configuration and significantly enhances the death message system. The changes are well-structured, moving configurations to more appropriate domain-specific classes and introducing detailed, scenario-based death messages. I've identified a couple of issues: a potential regression in the quit message handling for vanished players, and a misconfigured death message for TNT. Overall, this is a great improvement to the plugin's features and configuration.
...e-core/src/main/java/com/eternalcode/core/feature/deathmessage/messages/ENDeathMessages.java
Outdated
Show resolved
Hide resolved
…h-messages' into 2.0/move-sound-and-enhanced-death-messages
| EntityDamageEvent.DamageCause.FIRE_TICK, Collections.singletonList( | ||
| BukkitNotice.builder() | ||
| .sound(Sound.ITEM_FIRECHARGE_USE) | ||
| .chat("<white>☠ <dark_red>{PLAYER} <red>burned out in flames...") |
There was a problem hiding this comment.
| .chat("<white>☠ <dark_red>{PLAYER} <red>burned out in flames...") | |
| .chat("<white>☠ <dark_red>{PLAYER} <red>burned to ashes...") |
| "CACTUS", Arrays.asList( | ||
| BukkitNotice.builder() | ||
| .sound(Sound.BLOCK_GRASS_BREAK) | ||
| .chat("<white>☠ <dark_red>{PLAYER} <red>pricked themselves on a cactus!") |
There was a problem hiding this comment.
| .chat("<white>☠ <dark_red>{PLAYER} <red>pricked themselves on a cactus!") | |
| .chat("<white>☠ <dark_red>{PLAYER} <red>was pricked by a <dark_green>cactus!") |
| "# {KILLER} - Killer player", | ||
| "# {WEAPON} - Weapon used" | ||
| }) | ||
| public Map<String, List<Notice>> deathMessageByWeapon = Map.of( |
There was a problem hiding this comment.
Add support to new minecraft weapon mace
| "# {KILLER} - Zabójca", | ||
| "# {WEAPON} - Używana broń" | ||
| }) | ||
| public Map<String, List<Notice>> deathMessageByWeapon = Map.of( |
There was a problem hiding this comment.
Add support to new minecraft weapon mace
|
|
||
| if (this.vanishService.isVanished(player)) { | ||
|
|
||
| if (this.vanishService.isVanished(victim)) { | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
Add option to not use custom death messages
https://medal.tv/pl/games/minecraft/clips/kRaChp2MuznLZNLP4?invite=cr-MSxBOWksNDI2MDIzOTA4&v=29