Overhaul: Maven migration, variant support, bug fixes, and updated docs by RhythmicSys · Pull Request #10 · Simplexity-Development/SimpleBucketMobs

RhythmicSys · 2026-04-19T05:04:16Z

Overhaul: Maven migration, variant support, bug fixes, and updated docs

Summary

Migrated the build from Gradle to Maven with the Paper shade plugin and Mojang mappings support
Replaced the monolithic BucketMob listener with a split architecture (BucketHandler, EntityHandler, InteractListeners, BucketRule)
Added per-mob variant texture support (cat breeds, fox types, horse colour/marking combinations, wolf variants, frog variants, etc.) via texture.yml using NamespacedKey item models instead of the old integer custom model data system
Bumped api-version from 1.20 to 1.21.6 and updated Paper API dependency to match
Fixed a collection of bugs that had been silently breaking core functionality
Rewrote the README to accurately reflect the current configuration format and variant system

Bug fixes

Config keys were completely wrong
ConfigHandler read from bucket-style.* keys that have never existed in config.yml. The correct section is captured-item-style.*. Every style setting (item name, glint, material, item model, resource pack toggle) was silently ignored and fell back to hardcoded Java defaults. All key paths corrected.

config.yml had item-model: default
"default" is not a valid NamespacedKey and produced a logged warning on every reload. Changed to "minecraft:bucket".

Entity release spawned at the capture location
addEntity() always reads the entity's stored NBT position. The previous teleport()-before/after approach broke when the capture chunk was unloaded because teleport() requires the entity to already be in a world and cannot reposition an unspawned entity. Fixed by calling Entity.setPos() via reflection (Paper 1.21+ uses Mojang mappings at runtime, so the method name is stable) before addEntity(), so the mob spawns directly at the release location without touching the old chunk.

Enchantment glint config was hardcoded
BucketHandler always set ENCHANTMENT_GLINT_OVERRIDE to true regardless of the config value. Now reads ConfigHandler.isEnchantmentGlint().

Dead-code loop in setupTypes()
A loop that added disallowed entity types to entityRules ran immediately before entityRules.clear(), making it a no-op. Removed.

Both hands triggered bucketing/unbucketing
PlayerInteractEntityEvent and PlayerInteractEvent both fire once per hand. Neither handler checked EquipmentSlot.HAND, so off-hand interactions caused duplicate processing. Added hand guards to both.

Event cancelled too late in onBucketMob
The entity was removed and the bucket given before setCancelled(true), so other listeners received the event referencing an already-removed entity. Cancellation now happens first.

Debucket command NPE and wrong tag
Calling .getItemMeta() on a null item (empty hand) caused a NullPointerException. The command also only checked the legacy string tag; buckets created by the current plugin version use the byte-array tag and would always return "not a bucket mob". Added null guard and both-tag support.

Texture used e.printStackTrace()
Replaced with logger.error().

Test plan

- add support for variants

- Config should now be more dynamic - default rules for bucketing mobs - Moved style configs under 'bucket-style' - Adjusted handling to use new formats

- getItemModel now returns the actual key - validation of the string happens in the Texture class now - Started work on making capture items and captured items more customizable

…dling bugs ConfigHandler was reading from bucket-style.* keys that never existed in config.yml; the correct section is captured-item-style.*. This silently ignored every style setting (name, glint, material, item model, resource pack toggle) and fell back to Java-side defaults. Fixed all key paths. config.yml had item-model: default, which is not a valid NamespacedKey and logged a warning on every reload. Changed to minecraft:bucket. Entity release now uses NMS Entity.setPos() via reflection before addEntity() is called, so mobs spawn at the release location directly. The previous approach (teleport before/after addEntity) broke when the capture chunk was unloaded, because teleport() requires the entity to already be in a world, and addEntity() always used the stored NBT position. Additional fixes: - Enchantment glint config option was always hardcoded to true; now reads isEnchantmentGlint() from ConfigHandler - Dead-code loop in setupTypes() added disallowed types to entityRules then immediately cleared them; removed the loop - onBucketMob and onUnbucketMob now guard on EquipmentSlot.HAND to avoid processing off-hand duplicate events - Event is now cancelled before world mutations in onBucketMob so other listeners do not see a half-removed entity - Debucket command: added null check for empty main hand, and now handles both the legacy string tag and the new byte-array tag - Texture config load error now goes through the SLF4J logger instead of printStackTrace()

Peashooter101 · 2026-04-19T11:10:01Z

+    // These can already be bucketed, we don't wanna double-bucket lol
+    private final Set<EntityType> generalDisallowedTypes = Set.of(
+            EntityType.TROPICAL_FISH, EntityType.SALMON, EntityType.COD,
+            EntityType.AXOLOTL, EntityType.PUFFERFISH, EntityType.TADPOLE);


TODO: Add Sulfur Cube when 26.2 releases.

Peashooter101 · 2026-04-19T11:12:29Z

+    @SuppressWarnings("SameParameterValue")
+    private Material validateMaterial(String materialName, Material defaultMaterial, String path) {
+        if (materialName == null) {
+            logger.warn("No material found for {}, using default material: {}", path, defaultMaterial);
+            return defaultMaterial;
+        }
+        Material material = Material.getMaterial(materialName);
+        if (material == null) {
+            logger.warn("Invalid material in '{}': {} - using default material: {}", path, materialName, defaultMaterial);
+            return defaultMaterial;
+        }
+        return material;
+    }


Either the caller should determine the defaultMaterial or the defaultMaterial should be known at this point without passing it in as a parameter.

In this case, the defaultMaterial should be a field and should be known without passing it in as a parameter.

Peashooter101 · 2026-04-19T11:15:02Z

Requires additional testing with a new version of the Bucket Mobs Resource Pack.

Peashooter101 · 2026-04-19T11:16:30Z

+    public boolean isAllowed() {
+        return allow;
+    }
+
+    public boolean isSneakRequired() {
+        return sneakRequired;
+    }
+
+    public boolean canPickupWhenAggro() {
+        return pickupWhenAggro;
+    }
+
+    public boolean shouldRequirePermission() {
+        return requiresPermission;
+    }


If the fields are final, they can just be public without getters.

Peashooter101 · 2026-04-19T11:16:59Z

+    public String getItemName() {
+        return itemName;
+    }
+
+    public String getItemLore() {
+        return itemLore;
+    }
+
+    public boolean isEnchantGlint() {
+        return enchantGlint;
+    }
+
+    public NamespacedKey getItemModel() {
+        return itemModel;
+    }
+
+    public Material getItemType() {
+        return itemType;
+    }


If the fields are final, they can just be public without getters.

Peashooter101 · 2026-04-19T11:17:11Z

+    public String getItemName() {
+        return itemName;
+    }
+
+    public boolean isEnchantGlint() {
+        return enchantGlint;
+    }
+
+    public NamespacedKey getItemModel() {
+        return itemModel;
+    }
+
+    public Material getItemType() {
+        return itemType;
+    }
+
+    public Integer getStackSize() {
+        return stackSize;
+    }


If the fields are final, they can just be public without getters.

Peashooter101 · 2026-04-19T11:20:05Z

+capture-item-style:
+  default:
+    name: default
+    enchantment-glint: false
+    item-model: "minecraft:bucket"
+    item-type: BUCKET
+    stack-size: default
+captured-item-style:
+  use-resource-pack: false
+  default:
+    item-type: BUCKET
+    item-model: "minecraft:bucket"
+    name: "<aqua><display_name> in a Bucket"
+    lore:
+      - "<gray>A <type_name_cased> sits in the bucket</gray>"
+    enchantment-glint: true


This seems like a confusing name set: capture and captured are very easy to miss and confuse. Perhaps choose another name?

Peashooter101 · 2026-04-19T11:20:45Z

    default: op
  simplebucketmobs.debucket:
-    description: "Dump saved mob NBT data from Mob Bucket to chat."
+    description: "Dump saved mob NBT data from Mob Bucket to chat."


Does this accurately reflect debucket now? It shows the number of bytes for the new format.

Peashooter101 · 2026-04-19T11:21:06Z

What is this file?

Should this file even be being committed?

oh this shouldn't have been committed, no, this was just a reference from earlier in my testing to look at the format of a mob's data

Peashooter101

Mostly non-blocking, I think you're good to go after addressing the issues shown.

Peashooter101

Tested on 1.21.11

Functional Problems

Fresh startup generates warnings:

[06:28:12 INFO]: [SimpleBucketMobs] Enabling SimpleBucketMobs v2.0.0-DEV
[06:28:12 WARN]: [SimpleBucketMobs] Invalid locale key: LOGGER_INVALID_LOCALE_KEY
[06:28:12 WARN]: [SimpleBucketMobs] Invalid locale key: LOGGER_INVALID_MOB_TYPE
[06:28:12 WARN]: [SimpleBucketMobs] Invalid locale key: LOGGER_INVALID_LOCALE_KEY
[06:28:12 WARN]: [SimpleBucketMobs] Invalid locale key: LOGGER_INVALID_MOB_TYPE

Bucketing has no default audio from interaction. There also does not appear to be any configuration option to set the sound if you wanted to.
Cow milking audio plays in spite of not milking cow. May be client sided and does not appear to be cancellable? Requires investigation.
Slimes and Spiders when aggro is placed on the player are bucketable with pickup-when-aggro: false. These should not just work on Hostiles but on Neutral mobs as well.

Testing Problems

Cannot confirm /sbm debucket with legacy mob bucket, I do not have access to a legacy mob bucket.
Testing use-resource-pack: true requires updated Simple Bucket Mobs resource pack.

General Issues

Should we allow the user to specify what permission they would like to use when bucketing mobs? For an example use case, allow a player to bucket hostiles at a specific rank, but only bucket Villagers at a later rank?

…-plugin.yml

Peashooter101 · 2026-04-21T05:06:33Z

-        interactEvent.setCancelled(true);
        ItemStack bucketItem = BucketHandler.getMobBucket(livingEntity);
        BucketHandler.addMobBucketToInventory(player, bucketItem);
+        player.playSound(Sound.sound(Key.key("minecraft", "item.bucket.fill_fish"), Sound.Source.PLAYER, 1.0f, 1.0f));


May be a good idea to make this sound configurable.

Peashooter101

No more blocking issues, testing was either done by you or me for confirmed issues.

RhythmicSys and others added 10 commits April 3, 2025 13:40

Fix issue where 'use-as-blacklist' would only work on plugin reload

124b4d7

technically works - also deletes other buckets in the stack

f755645

Old mob buckets can be used

ca603f7

Some cleanup

536ed56

Config options should work again

0a70a69

Move to maven

ad9badc

- add support for variants

Refactored Config

46ac9c6

- Config should now be more dynamic - default rules for bucketing mobs - Moved style configs under 'bucket-style' - Adjusted handling to use new formats

Adjusted item model stuff

e5e105d

- getItemModel now returns the actual key - validation of the string happens in the Texture class now - Started work on making capture items and captured items more customizable

update readme

ec3359b

RhythmicSys requested a review from Peashooter101 April 19, 2026 05:04

Peashooter101 reviewed Apr 19, 2026

View reviewed changes

Peashooter101 approved these changes Apr 19, 2026

View reviewed changes

Peashooter101 requested changes Apr 19, 2026

View reviewed changes

Fix issues brought up in PR review, and swap out plugin.yml for paper…

6fd149c

…-plugin.yml

Peashooter101 reviewed Apr 21, 2026

View reviewed changes

Peashooter101 approved these changes Apr 21, 2026

View reviewed changes

Uh oh!

Conversation

RhythmicSys commented Apr 19, 2026 • edited by Peashooter101 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overhaul: Maven migration, variant support, bug fixes, and updated docs

Summary

Bug fixes

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Peashooter101 Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Peashooter101 left a comment

Choose a reason for hiding this comment

Uh oh!

Peashooter101 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Functional Problems

Testing Problems

General Issues

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Peashooter101 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

RhythmicSys commented Apr 19, 2026 •

edited by Peashooter101

Loading

Peashooter101 Apr 19, 2026 •

edited

Loading

Peashooter101 left a comment •

edited

Loading