Integrate RoyalDeath as a component of RoyalCommands #33
Conversation
Config options in config.yml, death messages in deathmessage.yml
Config has been adjusted, and the /kill command now support death cause
There was a problem hiding this comment.
Thanks so much for doing this. The code overall looks correct (though I haven't tested it). I've added comments mostly around how we want to name the configuration fields, and some notes about variables.
To elaborate about why this PR was created:
The RoyalDeath plugin is relatively small, and we felt it could be contained within the larger RoyalCommands codebase to ensure it's more likely to be updated for new versions, and to ensure continued compatibility. Plus, added bonus of less duplicate code & the ability to extend it with additional functionality (such as PlaceholderAPI down the line).
| final String pullFrom; | ||
| switch (this.getLastDamageCause()) { | ||
| case BLOCK_EXPLOSION: | ||
| pullFrom = "blo"; |
There was a problem hiding this comment.
I know this particular code path is basically identical from RoyalDeath's implementation, but do we want to stick with strings as identifiers here? Or is that something we can change later without any real damage?
There was a problem hiding this comment.
I did think over this when first doing it, and change each of the types to match the DamageCause.
Where I got stuck is cases like FIRE and FIRE_TICK where we use fir, or PROJECTILE where it can either be pvp or mob
Nothing stopping us from expanding the strings to be the full name (or description)
I am open to suggestions of better ways to tackle this
| # {player} <- name of player who died | ||
| # {dispplayer} <- display name of player who died (nicknames, etc) | ||
| # {killer} <- what killed it (attacker name - PvP only) | ||
| # {dispkiller} <- what killed it (attacker display name - PvP only) | ||
| # {world} <- the world in which the player died | ||
| # {mob} <- what killed it (if it's a monster) | ||
| # {hand} <- what was in the killer's hand (PvP only, will use item name if available) |
There was a problem hiding this comment.
This is the perfect time to align some of these with the existing variables with the larger RCmds. At a bare minimum, kicks.messages.ingame_format uses {dispname} rather than {dispplayer}. Though this might be difficult as most RCmds messages are single-player, whereas death has a player who died and a player who caused it.
There was a problem hiding this comment.
dispname makes sense for the killed player display name
Thoughts on using kdispname for the killer display name?
Rename the death config section to deathmessages
disabled_worlds is now ignored_worlds, vanilla messages will be used
Added config options to
config.yml, custom death messages can be disabledAdded new file
deathmessages.ymlto contain custom messages for each causeAdded support for new causes of death
/killnow supports a cause of death