gameplay: blighted-creatures#11
Conversation
- set spawn probability to 4% - adjusted loot table and spawn condition for each creature
WalkthroughRefactors core entity type from BlightedEntity to AbstractBlightedEntity, replaces spawn condition utilities with SpawnConditionFactory/SpawnProfile, adds a BlightedCreature base plus many blighted variants and Goldor boss, enhances loot (looting multipliers), introduces BlightedForge and related recipes, and several utility/UX improvements. Changes
Sequence Diagram(s)sequenceDiagram
participant Goldor
participant Sword as OrbitingSword
participant Utilities
participant Player
Goldor->>Utilities: getNearestPlayer(range)
Utilities-->>Goldor: nearest Player (or null)
alt Player found
Goldor->>Sword: detach and launch
Sword->>Player: SwordProjectile travels (ticks)
Player-->>Sword: collision/hit
Sword->>Player: apply damage/effects + explosion
Sword-->>Goldor: cleanup/notify
else No player
Goldor->>Goldor: skip throw, maybe regen swords
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
MrInspection
added
area: content
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/fr/moussax/blightedMC/game/recipes/MaterialRecipes.java (1)
66-73: Bug: encoder7 reused for encoder8's recipe.Lines 67-69 bind keys on
encoder7but the recipe is intended formagmaBucketRecipewhich usesencoder8. This causes the wrong shape/keys to be encoded. The bindings should useencoder8instead.🔎 Fix encoder variable
ShapeEncoder encoder8 = new ShapeEncoder("aaa", "bcb", " b "); - encoder7.bindKey('a', Material.MAGMA_BLOCK, 64); - encoder7.bindKey('b', ItemDirectory.getItem("ENCHANTED_IRON_INGOT"), 4); - encoder7.bindKey('c', ItemDirectory.getItem("ENCHANTED_LAVA_BUCKET"), 1); + encoder8.bindKey('a', Material.MAGMA_BLOCK, 64); + encoder8.bindKey('b', ItemDirectory.getItem("ENCHANTED_IRON_INGOT"), 4); + encoder8.bindKey('c', ItemDirectory.getItem("ENCHANTED_LAVA_BUCKET"), 1); BlightedShapedRecipe magmaBucketRecipe = new BlightedShapedRecipe( ItemDirectory.getItem("MAGMA_BUCKET"), 1 ); - magmaBucketRecipe.setRecipe(encoder7.encodeCraftingRecipe()); + magmaBucketRecipe.setRecipe(encoder8.encodeCraftingRecipe());src/main/java/fr/moussax/blightedMC/game/entities/bosses/TheAncientKnight.java (1)
1-225: Address the deprecation warning by updatingAbstractBlightedEntity.spawn()to use modern Bukkit API.The deprecation warning originates from
AbstractBlightedEntity.spawn()at line 142, which uses the deprecatedWorld.spawnEntity(Location, EntityType)method. Replace it with the genericWorld.spawn(Location, Class<T>)method. This requires refactoring to store the entity'sClass<T>instead ofEntityType, or using a helper method to convertEntityTypeto the corresponding entity class at runtime to maintain compatibility.src/main/java/fr/moussax/blightedMC/core/entities/AbstractBlightedEntity.java (1)
729-731: Minor: Exception message references old class name.The error message still says "BlightedEntity" but should say "AbstractBlightedEntity" for consistency.
🔎 Suggested fix
} catch (CloneNotSupportedException e) { - throw new RuntimeException("Failed to clone BlightedEntity", e); + throw new RuntimeException("Failed to clone AbstractBlightedEntity", e); }
🧹 Nitpick comments (11)
src/main/java/fr/moussax/blightedMC/utils/Utilities.java (1)
127-129: Remove redundant world check.The check
!player.getWorld().equals(source.getWorld())on Line 129 is redundant sincesource.getWorld().getPlayers()already returns only players in the same world.🔎 Proposed refactor
for (Player player : source.getWorld().getPlayers()) { if (player.getGameMode() == org.bukkit.GameMode.SPECTATOR || player.getGameMode() == org.bukkit.GameMode.CREATIVE) continue; - if (!player.getWorld().equals(source.getWorld())) continue; double distanceSquared = player.getLocation().distanceSquared(source.getLocation());src/main/java/fr/moussax/blightedMC/game/blocks/BlightedForge.java (1)
23-23: Use Action enum instead of string comparison.Using
.contains("RIGHT_CLICK")is fragile and relies on string representation. Consider using direct Action enum comparison for type safety and clarity.🔎 Proposed refactor
+ Action action = event.getAction(); - if (event.getAction().toString().contains("RIGHT_CLICK")) { + if (action == Action.RIGHT_CLICK_BLOCK || action == Action.RIGHT_CLICK_AIR) { event.setCancelled(true);Don't forget to add the import:
+import org.bukkit.event.block.Action;src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedCreature.java (2)
28-29: Consider parameterizing hardcoded health and spawn probability.The constructor hardcodes
maxHealth=30andspawnProbability=0.04. If subclasses need different values, consider adding constructor parameters or making these protected fields that subclasses can override.
75-82: Public triggerEnrage exposes manual enrage bypass.Making
triggerEnrage()public allows external code to bypass the health-based enrage condition. If enrage should only occur when health drops below the threshold, consider making this methodprotectedorprivate.src/main/java/fr/moussax/blightedMC/core/entities/loot/LootTable.java (1)
140-156: Looting implementation looks good, but consider API consistency.The looting enchantment detection and adjusted drop chance logic is correctly implemented. However, note that
generateLoot()(lines 111-129) doesn't account for looting level, whiledropLoot()does. IfgenerateLoot()is intended for external use where looting should apply, consider adding agenerateLoot(int lootingLevel)overload for consistency.src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedZombie.java (1)
38-90: Consider extracting common biome sets to reduce duplication.This extensive biome list likely overlaps with other Blighted creature spawn conditions. Extracting common biome groupings (e.g.,
OVERWORLD_SURFACE_BIOMES) to a shared constant inSpawnConditionFactoryor a dedicated utility would improve maintainability.src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedParched.java (1)
44-53: Spawn conditions duplicateBlightedHusk.The spawn condition chain here is identical to
BlightedHusk.defineSpawnConditions(). Consider extracting this to a shared constant (e.g.,SpawnConditionFactory.desertNightSurface()) to reduce duplication and ease future maintenance.src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedPiglin.java (1)
12-12: Unused import.The
Structureimport is not used in this file and should be removed.🔎 Proposed fix
-import org.bukkit.generator.structure.Structure;src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnProfile.java (1)
49-51: Consider adding null-check for condition parameter.Adding a null condition would cause an NPE when
canSpawniterates the list. A defensive check could prevent subtle bugs.🔎 Suggested fix
public void addSpawnCondition(SpawnCondition condition) { + if (condition == null) { + throw new IllegalArgumentException("Condition cannot be null"); + } conditions.add(condition); }src/main/java/fr/moussax/blightedMC/core/entities/listeners/BlightedEntitiesListener.java (2)
265-271: Consider adding error logging for entity instantiation failures.The
createInstancemethod attempts to create an instance via reflection, falling back toclone(). If both fail, the exception will propagate to the caller (line 258) and cause the rehydration to silently skip that entity.While this fallback approach is reasonable, adding error logging would help diagnose issues with entity rehydration during chunk loading.
🔎 Suggested enhancement with error logging
private static AbstractBlightedEntity createInstance(AbstractBlightedEntity prototype) { try { return prototype.getClass().getDeclaredConstructor().newInstance(); } catch (Exception ex) { - return prototype.clone(); + try { + return prototype.clone(); + } catch (Exception cloneEx) { + BlightedMC.getInstance().getLogger().severe( + "Failed to create instance of " + prototype.getClass().getSimpleName() + + ": " + cloneEx.getMessage() + ); + throw new RuntimeException("Entity instantiation failed", cloneEx); + } } }
32-32: Consider thread-safety implications of the static HashMap.The
BLIGHTED_ENTITIESmap is aHashMapaccessed via public static methods. While Bukkit events are typically single-threaded per world, these public methods could be called from async tasks or other threads, potentially causing race conditions.If concurrent access is possible, consider using
ConcurrentHashMapinstead.Note: This is a pre-existing design, not introduced by this PR. The presence of
ThreadLocalat line 33 suggests threading concerns are already considered.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
src/main/java/fr/moussax/blightedMC/commands/admin/SpawnCustomMobCommand.javasrc/main/java/fr/moussax/blightedMC/core/entities/AbstractBlightedEntity.javasrc/main/java/fr/moussax/blightedMC/core/entities/EntityAttachment.javasrc/main/java/fr/moussax/blightedMC/core/entities/EntityImmunities.javasrc/main/java/fr/moussax/blightedMC/core/entities/LifecycleTaskManager.javasrc/main/java/fr/moussax/blightedMC/core/entities/listeners/BlightedEntitiesListener.javasrc/main/java/fr/moussax/blightedMC/core/entities/listeners/SpawnableEntitiesListener.javasrc/main/java/fr/moussax/blightedMC/core/entities/loot/LootDropRarity.javasrc/main/java/fr/moussax/blightedMC/core/entities/loot/LootTable.javasrc/main/java/fr/moussax/blightedMC/core/entities/registry/EntitiesRegistry.javasrc/main/java/fr/moussax/blightedMC/core/entities/rituals/AncientCreature.javasrc/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnCondition.javasrc/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnConditionFactory.javasrc/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnConditions.javasrc/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnProfile.javasrc/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnableEntity.javasrc/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnableEntityProfile.javasrc/main/java/fr/moussax/blightedMC/core/fishing/loot/LootEntry.javasrc/main/java/fr/moussax/blightedMC/core/items/blocks/BlocksRegistry.javasrc/main/java/fr/moussax/blightedMC/core/items/crafting/registry/RecipesDirectory.javasrc/main/java/fr/moussax/blightedMC/core/player/BlightedPlayerListener.javasrc/main/java/fr/moussax/blightedMC/game/blocks/BlightedForge.javasrc/main/java/fr/moussax/blightedMC/game/blocks/BlightedWorkbench.javasrc/main/java/fr/moussax/blightedMC/game/blocks/BlocksDirectory.javasrc/main/java/fr/moussax/blightedMC/game/entities/Dummy.javasrc/main/java/fr/moussax/blightedMC/game/entities/bosses/RevenantHorror.javasrc/main/java/fr/moussax/blightedMC/game/entities/bosses/TheAncientKnight.javasrc/main/java/fr/moussax/blightedMC/game/entities/spawnable/BlightedZombie.javasrc/main/java/fr/moussax/blightedMC/game/entities/spawnable/LaserEngineer.javasrc/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedBogged.javasrc/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedCreature.javasrc/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedDrowned.javasrc/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedHusk.javasrc/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedParched.javasrc/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedPiglin.javasrc/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedSkeleton.javasrc/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedStray.javasrc/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedWitherSkeleton.javasrc/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedZombie.javasrc/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedZombifiedPiglin.javasrc/main/java/fr/moussax/blightedMC/game/recipes/MaterialRecipes.javasrc/main/java/fr/moussax/blightedMC/utils/ItemBuilder.javasrc/main/java/fr/moussax/blightedMC/utils/Utilities.javasrc/main/java/fr/moussax/blightedMC/utils/commands/CommandTabSuggestionBuilder.java
💤 Files with no reviewable changes (4)
- src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnableEntityProfile.java
- src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnConditions.java
- src/main/java/fr/moussax/blightedMC/game/blocks/BlightedWorkbench.java
- src/main/java/fr/moussax/blightedMC/game/entities/spawnable/BlightedZombie.java
🧰 Additional context used
🧬 Code graph analysis (20)
src/main/java/fr/moussax/blightedMC/game/entities/Dummy.java (1)
src/main/java/fr/moussax/blightedMC/core/entities/AbstractBlightedEntity.java (1)
AbstractBlightedEntity(55-758)
src/main/java/fr/moussax/blightedMC/core/entities/rituals/AncientCreature.java (1)
src/main/java/fr/moussax/blightedMC/core/entities/AbstractBlightedEntity.java (1)
AbstractBlightedEntity(55-758)
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedParched.java (3)
src/main/java/fr/moussax/blightedMC/core/entities/loot/LootTable.java (1)
LootTable(21-236)src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnConditionFactory.java (1)
SpawnConditionFactory(27-139)src/main/java/fr/moussax/blightedMC/utils/ItemBuilder.java (1)
ItemBuilder(48-778)
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/LaserEngineer.java (1)
src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnConditionFactory.java (1)
SpawnConditionFactory(27-139)
src/main/java/fr/moussax/blightedMC/game/entities/bosses/RevenantHorror.java (1)
src/main/java/fr/moussax/blightedMC/core/entities/AbstractBlightedEntity.java (1)
AbstractBlightedEntity(55-758)
src/main/java/fr/moussax/blightedMC/core/player/BlightedPlayerListener.java (2)
src/main/java/fr/moussax/blightedMC/core/entities/AbstractBlightedEntity.java (1)
AbstractBlightedEntity(55-758)src/main/java/fr/moussax/blightedMC/core/entities/listeners/BlightedEntitiesListener.java (1)
BlightedEntitiesListener(31-310)
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedDrowned.java (3)
src/main/java/fr/moussax/blightedMC/core/entities/loot/LootTable.java (1)
LootTable(21-236)src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnConditionFactory.java (1)
SpawnConditionFactory(27-139)src/main/java/fr/moussax/blightedMC/utils/ItemBuilder.java (1)
ItemBuilder(48-778)
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedSkeleton.java (3)
src/main/java/fr/moussax/blightedMC/core/entities/loot/LootTable.java (1)
LootTable(21-236)src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnConditionFactory.java (1)
SpawnConditionFactory(27-139)src/main/java/fr/moussax/blightedMC/utils/ItemBuilder.java (1)
ItemBuilder(48-778)
src/main/java/fr/moussax/blightedMC/commands/admin/SpawnCustomMobCommand.java (2)
src/main/java/fr/moussax/blightedMC/core/entities/AbstractBlightedEntity.java (1)
AbstractBlightedEntity(55-758)src/main/java/fr/moussax/blightedMC/core/entities/registry/EntitiesRegistry.java (1)
EntitiesRegistry(17-71)
src/main/java/fr/moussax/blightedMC/game/entities/bosses/TheAncientKnight.java (1)
src/main/java/fr/moussax/blightedMC/core/entities/AbstractBlightedEntity.java (1)
AbstractBlightedEntity(55-758)
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedZombifiedPiglin.java (3)
src/main/java/fr/moussax/blightedMC/core/entities/loot/LootTable.java (1)
LootTable(21-236)src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnConditionFactory.java (1)
SpawnConditionFactory(27-139)src/main/java/fr/moussax/blightedMC/utils/ItemBuilder.java (1)
ItemBuilder(48-778)
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedBogged.java (3)
src/main/java/fr/moussax/blightedMC/core/entities/loot/LootTable.java (1)
LootTable(21-236)src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnConditionFactory.java (1)
SpawnConditionFactory(27-139)src/main/java/fr/moussax/blightedMC/utils/ItemBuilder.java (1)
ItemBuilder(48-778)
src/main/java/fr/moussax/blightedMC/core/items/blocks/BlocksRegistry.java (1)
src/main/java/fr/moussax/blightedMC/game/blocks/BlightedForge.java (1)
BlightedForge(12-28)
src/main/java/fr/moussax/blightedMC/utils/commands/CommandTabSuggestionBuilder.java (2)
src/main/java/fr/moussax/blightedMC/core/entities/AbstractBlightedEntity.java (1)
AbstractBlightedEntity(55-758)src/main/java/fr/moussax/blightedMC/core/entities/registry/EntitiesRegistry.java (1)
EntitiesRegistry(17-71)
src/main/java/fr/moussax/blightedMC/game/recipes/MaterialRecipes.java (1)
src/main/java/fr/moussax/blightedMC/core/items/registry/ItemDirectory.java (1)
ItemDirectory(26-97)
src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnableEntity.java (1)
src/main/java/fr/moussax/blightedMC/core/entities/AbstractBlightedEntity.java (1)
AbstractBlightedEntity(55-758)
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedStray.java (3)
src/main/java/fr/moussax/blightedMC/core/entities/loot/LootTable.java (1)
LootTable(21-236)src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnConditionFactory.java (1)
SpawnConditionFactory(27-139)src/main/java/fr/moussax/blightedMC/utils/ItemBuilder.java (1)
ItemBuilder(48-778)
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedCreature.java (2)
src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnableEntity.java (1)
SpawnableEntity(20-121)src/main/java/fr/moussax/blightedMC/utils/ItemBuilder.java (1)
ItemBuilder(48-778)
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedHusk.java (3)
src/main/java/fr/moussax/blightedMC/core/entities/loot/LootTable.java (1)
LootTable(21-236)src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnConditionFactory.java (1)
SpawnConditionFactory(27-139)src/main/java/fr/moussax/blightedMC/utils/ItemBuilder.java (1)
ItemBuilder(48-778)
src/main/java/fr/moussax/blightedMC/core/fishing/loot/LootEntry.java (1)
src/main/java/fr/moussax/blightedMC/core/entities/AbstractBlightedEntity.java (1)
AbstractBlightedEntity(55-758)
🪛 GitHub Actions: Maven Build
src/main/java/fr/moussax/blightedMC/game/entities/bosses/TheAncientKnight.java
[warning] 1-1: TheAncientKnight.java uses or overrides a deprecated API. Recompile with -Xlint:deprecation for details.
src/main/java/fr/moussax/blightedMC/core/entities/registry/EntitiesRegistry.java
[error] 6-6: Cannot find symbol: class Goldor. (Located in package fr.moussax.blightedMC.game.entities.bosses)
[error] 63-63: Cannot find symbol: class Goldor. (Located in package fr.moussax.blightedMC.game.entities.bosses)
🔇 Additional comments (58)
src/main/java/fr/moussax/blightedMC/core/entities/LifecycleTaskManager.java (1)
11-11: Javadoc reference updated to reflect entity hierarchy refactoring.The Javadoc link has been correctly updated from
BlightedEntitytoAbstractBlightedEntityto align with the PR's entity type refactoring. TheAbstractBlightedEntityclass is properly defined and accessible in the codebase.src/main/java/fr/moussax/blightedMC/core/entities/EntityImmunities.java (1)
9-9: LGTM! Documentation updated correctly.The JavaDoc references have been properly updated to reflect the refactoring from
BlightedEntitytoAbstractBlightedEntity.Also applies to: 24-24
src/main/java/fr/moussax/blightedMC/utils/ItemBuilder.java (1)
179-189: LGTM! Useful enhancement for ItemMeta manipulation.The new
setItemMeta(Consumer<ItemMeta>)method provides a flexible way to mutate ItemMeta directly, complementing the existingsetItemMeta(ItemMeta)overload. This pattern aligns well with other consumer-based methods in the class.src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedStray.java (1)
20-74: LGTM! Well-implemented Blighted entity.The
BlightedStrayimplementation is clean and follows the established patterns:
- Proper use of the new
setItemMetaconsumer pattern for creating the slowness tipped arrow- Appropriate enrage mechanics with infinite-duration buffs
- Comprehensive spawn conditions restricting to snowy/frozen biomes with proper light and environmental constraints
- Standard loot table configuration with appropriate rarities
src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnCondition.java (1)
7-53: LGTM! Clear and consistent API improvement.The rename from
canSpawntotestCanSpawnAtimproves clarity and follows better naming conventions for predicate methods. All combinators (and,or,not) have been updated consistently.src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnableEntity.java (1)
3-3: LGTM! Clean refactoring to AbstractBlightedEntity and SpawnProfile.The refactoring improves naming clarity and consolidates spawn condition management:
- Extends
AbstractBlightedEntityfor better inheritance structure- Renamed fields and methods for consistency (
spawnProbability,defineSpawnConditions,addCondition)- Uses
SpawnProfilefor spawn condition management- Updated
clone()to usecopy()methodAlso applies to: 20-40, 46-55, 71-73, 118-118
src/main/java/fr/moussax/blightedMC/core/entities/listeners/SpawnableEntitiesListener.java (1)
41-41: LGTM! Method references updated correctly.The calls to
getSpawnProbability()correctly reflect the refactored API inSpawnableEntity. The spawn probability calculation logic remains unchanged.Also applies to: 51-51
src/main/java/fr/moussax/blightedMC/game/blocks/BlightedForge.java (1)
25-25: ForgeMenu is designed to handle null recipe parameter intentionally.The
ForgeMenuclass has an overloaded constructor that acceptsForgeRecipe recipeas a nullable parameter. The code explicitly checks for null in multiple locations (lines 49, 56, 67, 77, 201, 221, 246) and gracefully handles the null case by displaying UI messages like "Recipe Required - Select a recipe from the recipe book to start forging." This is an intentional design pattern allowing the menu to function in both recipe-selected and recipe-unselected states.src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedCreature.java (1)
39-61: Verify tick-rate performance for per-tick task execution.The repeating task runs every tick (
1L, 1L), which means 20 executions per second per entity. For multiple spawned creatures, this could impact server performance. Verify whether checking health and spawning particles needs such high frequency—consider increasing the period to 5-10 ticks if feasible.src/main/java/fr/moussax/blightedMC/game/entities/spawnable/LaserEngineer.java (1)
247-256: LGTM! Spawn condition refactor aligns with new framework.The migration from
setupSpawnConditionstodefineSpawnConditionsand usage ofSpawnConditionFactory.biome()is consistent with the spawn system refactor introduced in this PR. The biome restrictions remain functionally identical.src/main/java/fr/moussax/blightedMC/core/items/blocks/BlocksRegistry.java (1)
14-17: LGTM! BlightedForge registration is consistent.The addition of
BlightedForgeto the block registration list aligns with the new block introduced in this PR. TheList.of()usage correctly registers both blocks.src/main/java/fr/moussax/blightedMC/core/entities/rituals/AncientCreature.java (1)
3-3: LGTM! Type migration to AbstractBlightedEntity is consistent.The change from
BlightedEntitytoAbstractBlightedEntityaligns with the broader refactor across the codebase. No behavioral changes introduced.Also applies to: 10-10
src/main/java/fr/moussax/blightedMC/commands/admin/SpawnCustomMobCommand.java (1)
3-3: LGTM! Type migration to AbstractBlightedEntity is consistent.The change from
BlightedEntitytoAbstractBlightedEntityaligns with the broader refactor across the codebase. No behavioral changes introduced.Also applies to: 28-28
src/main/java/fr/moussax/blightedMC/game/blocks/BlocksDirectory.java (1)
34-49: LGTM! BlightedForge item definition is well-structured.The new BlightedForge item is properly configured with appropriate rarity (RARE), material (BLAST_FURNACE), thematic lore, and enchantment glint. The definition is consistent with the block implementation and registration elsewhere in the PR.
src/main/java/fr/moussax/blightedMC/game/recipes/MaterialRecipes.java (1)
85-94: LGTM! BlightedForge recipe is well-designed.The crafting recipe for the Blighted Forge uses appropriate materials and requires the Blighted Workbench as a prerequisite, creating a logical progression. The pattern and material costs are reasonable.
src/main/java/fr/moussax/blightedMC/game/entities/Dummy.java (1)
3-3: LGTM! Type migration to AbstractBlightedEntity is consistent.The change from
BlightedEntitytoAbstractBlightedEntityaligns with the broader refactor across the codebase. No behavioral changes introduced.Also applies to: 16-16
src/main/java/fr/moussax/blightedMC/core/items/crafting/registry/RecipesDirectory.java (1)
39-43: LGTM!The new varargs overload for batch registration is a useful addition that complements the existing single-recipe API. The implementation is straightforward and consistent with the existing pattern.
src/main/java/fr/moussax/blightedMC/game/entities/bosses/TheAncientKnight.java (1)
4-4: Superclass migration looks correct.The import and class declaration changes align with the project-wide refactor from
BlightedEntitytoAbstractBlightedEntity.Also applies to: 18-18
src/main/java/fr/moussax/blightedMC/core/entities/EntityAttachment.java (1)
5-15: LGTM!The record type update and Javadoc changes correctly reflect the migration to
AbstractBlightedEntity. The documentation clearly describes the attachment lifecycle semantics.src/main/java/fr/moussax/blightedMC/game/entities/bosses/RevenantHorror.java (1)
4-4: LGTM!The superclass migration from
BlightedEntitytoAbstractBlightedEntityis consistent with the project-wide refactor. The entity behavior remains unchanged.Also applies to: 20-20
src/main/java/fr/moussax/blightedMC/core/entities/loot/LootTable.java (2)
59-73: LGTM!The new
addLoot(ItemStack, ...)overload is a useful addition that follows the same pattern as the existing overloads, enabling direct ItemStack registration.
235-235: No issues found. The type declaration in theLootEntryrecord is correct.GemsLootAdapterextendsItemLoot, so it is a valid subtype that can be stored in fields of typeItemLoot. There is no type mismatch, misleading declaration, or compile error.src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedWitherSkeleton.java (2)
18-25: LGTM!The constructor properly initializes the entity with appropriate stats, equipment, and loot table. The setup follows the established pattern for Blighted creatures.
44-50: Verify the intended spawn light level constraint.
maxBlockLight(0)restricts spawning to complete darkness (zero block light). This is stricter than vanilla wither skeleton spawning rules. Confirm this is the intended behavior, as it may significantly limit spawn opportunities even within fortresses.src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedZombifiedPiglin.java (3)
19-26: LGTM!The constructor properly initializes the Blighted Zombified Piglin with thematic stats, weapon, and loot appropriate for a Nether mob variant.
37-44: LGTM!The enrage behavior correctly enhances the entity with speed, strength, and a fire aspect weapon upgrade. Good use of defensive
Objects.requireNonNullfor equipment access.
46-54: LGTM!The spawn conditions are well-structured, allowing spawns in Nether biomes or fortress structures, with appropriate light and liquid constraints. The
maxBlockLight(11)threshold is more permissive than the Wither Skeleton variant, which seems intentional for this mob type.src/main/java/fr/moussax/blightedMC/core/player/BlightedPlayerListener.java (1)
4-4: LGTM!The import and variable type updates from
BlightedEntitytoAbstractBlightedEntitycorrectly align with the codebase-wide refactor. The null-check and name resolution logic remain unchanged.Also applies to: 134-134
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedZombie.java (2)
15-20: LGTM!The constructor properly initializes the entity with appropriate identity, combat stats, and loot configuration.
32-36: LGTM!The enrage effects (Speed II and Strength I with infinite duration) provide a meaningful threat increase.
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedHusk.java (3)
17-23: LGTM!Constructor properly initializes the Blighted Husk entity with appropriate stats and loot configuration.
34-41: Verify bow behavior on Husk entity.Equipping a bow on a Husk (zombie variant) won't grant ranged attack capability in vanilla Minecraft—only Skeletons and similar mobs use bows for ranged attacks. If this is intended as a visual indicator or there's custom AI handling ranged behavior, this is fine. Otherwise, the bow will go unused.
43-52: LGTM!The spawn conditions appropriately restrict Blighted Husks to desert biomes at night with sky exposure, consistent with vanilla Husk spawning behavior.
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedParched.java (2)
27-34: Same bow-on-melee concern asBlightedHusk.The "Parched" entity is a skeleton variant that should properly use the bow for ranged attacks, unlike the Husk. If Parched inherits skeleton AI, this implementation is correct.
18-25:EntityType.PARCHEDis valid for Minecraft 1.21+ servers.This entity type is available in Bukkit API for Minecraft 1.21 and later versions. If the plugin targets pre-1.21 servers, this will cause a compilation error. Ensure the project's target Minecraft version is 1.21 or higher.
src/main/java/fr/moussax/blightedMC/core/entities/loot/LootDropRarity.java (1)
17-28: Drop chances exceeding 1.0 are correctly handled as guaranteed drops.The formula
baseChance * (1 + lootingLevel * multiplier)can produce values greater than 1.0, but this is handled correctly inLootTable.javaline 153. The adjusted chance is compared directly withrandomizer.nextDouble()(which returns [0.0, 1.0)), so any value exceeding 1.0 naturally becomes a guaranteed drop. No explicit clamping is necessary.src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedSkeleton.java (3)
1-25: LGTM! Well-structured BlightedSkeleton implementation.The constructor correctly initializes the entity with appropriate combat stats, equipment, and loot table. The use of
ItemBuilderfor equipment and fluentLootTableconstruction follows the established patterns.
27-34: LGTM!The enrage behavior correctly applies potion effects and upgrades the weapon with enchantments. The null-safety check on equipment is appropriate.
36-97: LGTM!The loot table and spawn conditions are well-configured. The extensive biome list covers surface and cave biomes appropriately for a skeleton variant.
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedBogged.java (3)
20-27: LGTM!Constructor correctly initializes the Bogged variant with appropriate stats and equipment for this swamp-dwelling skeleton type.
29-45: LGTM!The poison-tipped arrow creation using
ItemBuilder.setItemMetawithPotionMetais a clean approach. The loot table fits the Bogged theme well with themed poison arrows.
47-67: LGTM!The enrage behavior and spawn conditions are appropriate. Restricting spawns to swamp biomes aligns with vanilla Bogged behavior.
src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedDrowned.java (3)
19-28: LGTM!Good use of
Attribute.WATER_MOVEMENT_EFFICIENCYfor the aquatic entity. Setting main hand toAIRinitially is appropriate since drowned don't always spawn with weapons.
30-46: LGTM!The loot table appropriately includes drowned-themed drops with the rare trident. The enrage behavior granting Impaling II trident is thematically fitting for aquatic combat.
48-69: Spawn condition logic is correct for aquatic mob.The use of
.not()onnotInLiquid()effectively requires the spawn location to be in liquid, which is appropriate for drowned mobs. The biome selection covers all aquatic environments including dripstone caves (which can have water pools).src/main/java/fr/moussax/blightedMC/game/entities/spawnable/blighted/BlightedPiglin.java (1)
19-53: LGTM!The Nether-themed implementation is well-designed with appropriate gold-based loot, fire aspect enchantment, and Nether biome restrictions. The higher damage (8) and XP (16) correctly reflect increased Nether difficulty. The relaxed light level constraint (
maxBlockLight(11)) is suitable for the generally darker Nether environment.src/main/java/fr/moussax/blightedMC/core/entities/registry/EntitiesRegistry.java (2)
17-46: Registry refactoring looks good.The transition from
BlightedEntitytoAbstractBlightedEntityis consistently applied throughout the registry. The clone pattern ingetEntityandgetAllEntitiesensures prototype safety.
6-6: Build failure: MissingGoldorclass.The pipeline is failing because the
Goldorclass cannot be found. Either the class file is missing from this PR, or the import path is incorrect.src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnProfile.java (1)
76-85: LGTM!The
copy()implementation correctly creates an independent list while sharing the statelessSpawnConditionlambdas, which is the right approach for immutable predicates.src/main/java/fr/moussax/blightedMC/utils/commands/CommandTabSuggestionBuilder.java (1)
56-84: LGTM!The refactored tab completion logic properly consolidates candidate collection, filtering via
StringUtil.copyPartialMatches, and sorting into a unified flow. The@NonNullannotations strengthen the API contract.src/main/java/fr/moussax/blightedMC/core/entities/AbstractBlightedEntity.java (2)
98-129: LGTM!Constructor renames from
BlightedEntitytoAbstractBlightedEntityare consistent and maintain the correct delegation chain.
713-728: LGTM!The
clone()method correctly casts toAbstractBlightedEntityand properly resets runtime state while deep-copying mutable collections and items.src/main/java/fr/moussax/blightedMC/core/entities/spawnable/SpawnConditionFactory.java (2)
37-54: LGTM!The
biome()method efficiently captures an immutableSetfor O(1) lookups. The Y-level conditions are straightforward and correct.
122-138: LGTM!The
insideStructure()implementation correctly queries chunk-based structures and checks if the location falls within any structure piece's bounding box. The nested iteration is appropriate for accurate structure detection.src/main/java/fr/moussax/blightedMC/core/fishing/loot/LootEntry.java (3)
25-34: LGTM!The field and constructor parameter renames from
blightedEntitytoabstractBlightedEntityare consistent with the broader refactor to useAbstractBlightedEntityas the base type.
144-147: LGTM!The spawning logic correctly uses
abstractBlightedEntity.clone().spawn()to ensure each spawned entity has independent state, preserving the original template.
176-178: LGTM!The
isEntity()check andgetBlightedEntity()getter correctly reference the renamed field.Also applies to: 204-206
src/main/java/fr/moussax/blightedMC/core/entities/listeners/BlightedEntitiesListener.java (1)
4-4: LGTM! Refactoring from BlightedEntity to AbstractBlightedEntity is consistent.The type refactoring has been applied consistently throughout this file, including:
- Import statement
- Static map declaration
- All method signatures (public and private)
- Local variable declarations
- Static method calls
The logic remains unchanged, making this a clean type hierarchy update.
MrInspection
changed the title
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
src/main/java/fr/moussax/blightedMC/core/items/forging/menu/ForgeRecipesMenu.java (1)
55-59: LGTM! Consider extracting the comparator for reuse.The null-safe sorting logic is correct. However, this exact pattern (null-safe string comparison by display name) is duplicated across
ForgeRecipesMenu,RecipeBookMenu, andItemDirectoryMenu. Consider extracting a reusable comparator utility.🔎 Example utility extraction
// In a utility class public static <T> Comparator<T> byDisplayName(Function<T, String> nameExtractor) { return (a, b) -> { String name1 = nameExtractor.apply(a); String name2 = nameExtractor.apply(b); return (name1 != null ? name1 : "").compareTo(name2 != null ? name2 : ""); }; }Usage:
this.cachedRecipes.sort(byDisplayName(r -> r.getForgedItem().getDisplayName()));src/main/java/fr/moussax/blightedMC/core/items/registry/ItemDirectoryMenu.java (1)
127-129: Unused constructor parameter.The
categoryparameter is not used within this constructor. If it exists only for API consistency or future use, consider documenting the intent. Otherwise, this overload could be simplified.src/main/java/fr/moussax/blightedMC/game/entities/bosses/Goldor.java (1)
208-238: Code duplication: Extract ability runnable setup.The
BukkitRunnablelogic (lines 208-238) is nearly identical to the constructor's version (lines 46-78). Consider extracting a helper method to avoid duplication and maintenance burden.🔎 Proposed refactor
private void setupAbilityTask(Goldor instance) { instance.addRepeatingTask(() -> { instance.abilityRunnable = new BukkitRunnable() { private int tickCounter = 0; @Override public void run() { if (instance.entity == null || instance.entity.isDead()) { instance.stopAbility(); cancel(); return; } if (instance.orbitingSwords.isEmpty() && tickCounter == 0) { instance.spawnSwords(); } instance.performOrbit(); if (tickCounter > 0 && tickCounter % 80 == 0) { instance.performThrowAbility(); } if (tickCounter % 20 == 0) { instance.regenerateSwords(); } tickCounter++; } }; return instance.abilityRunnable; }, 0L, 1L); }Then use in constructor and clone:
// In constructor: setupAbilityTask(this); // In clone(): clone.setupAbilityTask(clone);src/main/java/fr/moussax/blightedMC/core/entities/rituals/menu/RitualsDirectoryMenu.java (1)
44-48: Consider case-insensitive sorting for better alphabetical ordering.The current sorting is case-sensitive, which may result in unexpected grouping when ritual names have mixed cases (e.g., "Zombie" would appear after "ancient Spirit" due to 'Z' < 'a' in ASCII).
🔎 Proposed fix for case-insensitive sorting
this.cachedRituals.sort((r1, r2) -> { String name1 = r1.getSummonedCreature() != null ? r1.getSummonedCreature().getName() : "Unknown Ritual"; String name2 = r2.getSummonedCreature() != null ? r2.getSummonedCreature().getName() : "Unknown Ritual"; - return name1.compareTo(name2); + return name1.compareToIgnoreCase(name2); });src/main/java/fr/moussax/blightedMC/game/rituals/RitualsRegistry.java (1)
13-13: Consider deferring task scheduling in Goldor until entity spawning. Creating and scheduling BukkitRunnable tasks in the constructor wastes resources since the entity doesn't exist until the ritual is performed. The tasks checkif (entity == null)immediately and return, but it's inefficient to create these scheduled objects upfront. Consider moving task creation to a spawn/initialization method or using a factory pattern for entity instantiation instead of a directnew Goldor()call.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
pom.xmlsrc/main/java/fr/moussax/blightedMC/GlobalRegistry.javasrc/main/java/fr/moussax/blightedMC/core/entities/rituals/menu/RitualsDirectoryMenu.javasrc/main/java/fr/moussax/blightedMC/core/items/crafting/menu/RecipeBookMenu.javasrc/main/java/fr/moussax/blightedMC/core/items/forging/menu/ForgeRecipesMenu.javasrc/main/java/fr/moussax/blightedMC/core/items/registry/ItemDirectoryMenu.javasrc/main/java/fr/moussax/blightedMC/core/items/rules/ItemRuleListener.javasrc/main/java/fr/moussax/blightedMC/core/items/rules/PreventFluidPlacementRule.javasrc/main/java/fr/moussax/blightedMC/game/entities/bosses/Goldor.javasrc/main/java/fr/moussax/blightedMC/game/items/ThermalFuels.javasrc/main/java/fr/moussax/blightedMC/game/rituals/RitualsRegistry.java
✅ Files skipped from review due to trivial changes (1)
- pom.xml
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/fr/moussax/blightedMC/GlobalRegistry.java (1)
src/main/java/fr/moussax/blightedMC/game/rituals/RitualsRegistry.java (1)
RitualsRegistry(10-21)
🔇 Additional comments (7)
src/main/java/fr/moussax/blightedMC/core/items/rules/ItemRuleListener.java (1)
34-42: LGTM!The new
onBucketEmptyhandler follows the established pattern used by other event handlers in this class. The implementation correctly retrieves theItemTemplatemanager, checks the rule viacanUse(), and cancels the event when appropriate.src/main/java/fr/moussax/blightedMC/game/items/ThermalFuels.java (1)
32-32: LGTM!The change to
PreventFluidPlacementRuleis semantically appropriate for lava bucket items and is consistent with the other bucket items (magmaBucket,plasmaBucket) in this file.src/main/java/fr/moussax/blightedMC/core/items/rules/PreventFluidPlacementRule.java (1)
7-11: LGTM!The implementation correctly returns
trueforPlayerBucketEmptyEvent, which will trigger event cancellation inItemRuleListener.onBucketEmpty(). This effectively prevents placing fluids from special bucket items.src/main/java/fr/moussax/blightedMC/core/items/crafting/menu/RecipeBookMenu.java (1)
51-55: LGTM!The sorting implementation is consistent with the pattern used in
ForgeRecipesMenu. The null-safe comparison ensures stable ordering.src/main/java/fr/moussax/blightedMC/core/items/registry/ItemDirectoryMenu.java (1)
120-124: LGTM!The sorting implementation is consistent with the pattern used across the other menu files.
src/main/java/fr/moussax/blightedMC/game/entities/bosses/Goldor.java (2)
36-80: LGTM on ability lifecycle setup.The repeating task pattern with proper null/dead entity checks and the integration of orbit, throw, and regen abilities is well-structured. The tick-based coordination is clear.
88-97: Proper cleanup on death.Both
kill()andonDeath()correctly invokestopAbility()before delegating to the superclass, ensuring orbiting swords and projectiles are cleaned up.
1 participant
Summary by CodeRabbit
New Features
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.