added Brewing Time Options (Hover and Actionbar), toggle chat message - #130
added Brewing Time Options (Hover and Actionbar), toggle chat message#130kami-tsuki wants to merge 3 commits into
Conversation
Thorinwasher
left a comment
Thorinwasher
left a comment
There was a problem hiding this comment.
I will test this as well. Don't worry, I'm not a clean code guy. I mostly care if it's flexible / adaptable to unforeseen changes in required behavior and so on, it seem modular enough, the logic looks solid as well.
Might be an issue on plugin reloads / server restarts; This behavior will not be persistently stored, but I think that's fine, as it's an utility kind of thing.
| private static class IngredientAddition { | ||
| final Ingredient ingredient; | ||
| final long timestamp; | ||
| final ItemStack originalItem; | ||
|
|
||
| IngredientAddition(Ingredient ingredient, long timestamp, ItemStack originalItem) { | ||
| this.ingredient = ingredient; | ||
| this.timestamp = timestamp; | ||
| this.originalItem = originalItem.clone(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Could make this a record :^)
| if (ingredientHistory.isEmpty()) { | ||
| return null; | ||
| } | ||
|
|
||
| // Check if brew has been extracted | ||
| if (brewExtracted) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
| if (ingredientHistory.isEmpty()) { | |
| return null; | |
| } | |
| // Check if brew has been extracted | |
| if (brewExtracted) { | |
| return null; | |
| } | |
| if (ingredientHistory.isEmpty() || brewExtracted) { | |
| return null; | |
| } |
| if (removed) { | ||
| // Recalculate recipe only if there are still ingredients | ||
| if (!isEmpty) { | ||
| this.recipe = brew.closestRecipe(TheBrewingProject.getInstance().getRecipeRegistry()) | ||
| .orElse(null); | ||
| } else { | ||
| this.recipe = null; | ||
| } | ||
|
|
||
| // Play a sound effect | ||
| final boolean finalIsEmpty = isEmpty; | ||
| BukkitAdapter.toLocation(this.location) | ||
| .ifPresent(loc -> { | ||
| World world = loc.getWorld(); | ||
| if (finalIsEmpty) { | ||
| // Play water splash sound when reverting to normal cauldron | ||
| world.playSound(loc, org.bukkit.Sound.ENTITY_PLAYER_SPLASH, 0.7f, 1.0f); | ||
| world.spawnParticle(Particle.SPLASH, loc.add(0.5, 0.7, 0.5), 20, 0.2, 0.1, 0.2, 0.5); | ||
| } else { | ||
| // Play item pickup sound for normal removal | ||
| world.playSound(loc, org.bukkit.Sound.ENTITY_ITEM_PICKUP, 0.5f, 1.2f); | ||
| } | ||
| }); | ||
|
|
||
| return lastAddition.originalItem; | ||
| } |
There was a problem hiding this comment.
Probably make this a method instead, I don't see why not
| boolean removed = false; | ||
| boolean isEmpty = false; | ||
|
|
||
| if (brew.lastStep() instanceof BrewingStep.Cook cook) { |
There was a problem hiding this comment.
Try to unwrap these if possible. Use return statements, or maybe make a separate method for the cook and mix case. I think you already know this, but if you don't here's an example:
if(statment1) {
if(statement2) {
// Code 1
} else {
// Code 2
}
} else {
// Code 3
}Becomes this:
if(!statement1) {
// Code 3
return;
}
if(!statement2) {
// Code 2
return;
}
// Code 1Up to you, though. I have also written similar code in this project, for what I could guess at least. Makes life somewhat easier for me to review it though.
There was a problem hiding this comment.
Also the mix and cook part here are extremely similar to each other, it should be possible to remove some code duplication.
| Ingredient ingredient = BukkitIngredientManager.INSTANCE.getIngredient(item); | ||
|
|
||
| // Track this ingredient addition for potential removal | ||
| ingredientHistory.add(new IngredientAddition(ingredient, time, item)); |
There was a problem hiding this comment.
The item added here has to have 1 in amount, I got a dupe otherwise by adding items when holding an item stack with more than 1 items in it.
| boolean removed = false; | ||
| boolean isEmpty = false; | ||
|
|
||
| if (brew.lastStep() instanceof BrewingStep.Cook cook) { |
There was a problem hiding this comment.
Also the mix and cook part here are extremely similar to each other, it should be possible to remove some code duplication.
|
@kami-tsuki Any updates on this? |
Sry I'll try to get back into that topic by some time. Just got a ton of other things going on currently... |
|
As this PR hasn't been updated in a while, I went ahead and reimplemented the clock part of it in #210 |
2 participants
Brew Time can optionally be shown on Action Bar (default disabled)
Brew Time can optionally be shown in Chat (default enabled)
Brew time can update on Look at the cauldron when using a clock in main or offhand (default disabled)
Implemented Removal of Latest added Items
Configurable Amount (0 to disable)
Configurable Limit Time (0 to disable)
When removed all Ingredients Cauldron Brewing is resetted
Message in Action bar when removing, item drops at Player pos and a small sound effect appears