Added EffMakePlayerBreakBlock #889

underscore95 · 2026-01-05T10:23:48Z

Describe your changes

This PR adds EffMakePlayerBreakBlock. It forces the player to break blocks using Player#breakBlock

Target Minecraft Versions: any
Requirements: none
Related Issues: #888

Checklist before requesting a review

Tests have been added if necessary
I have read the contributing guidelines

Fusezion

Just a quick once over

src/main/java/com/shanebeestudios/skbee/elements/other/effects/EffMakePlayerBreakBlock.java

src/test/scripts/elements/other/effects/EffMakePlayerBreakBlock.sk

src/main/java/com/shanebeestudios/skbee/elements/other/effects/EffMakePlayerBreakBlock.java

…/EffMakePlayerBreakBlock.java Co-authored-by: Fusezion <fusezionstream@gmail.com>

ShaneBeee

Just some food for thought.

ShaneBeee · 2026-01-05T15:39:28Z

src/main/java/com/shanebeestudios/skbee/elements/other/effects/EffMakePlayerBreakBlock.java

+    protected void execute(Event event) {
+        Player player = this.player.getSingle(event);
+        if (player == null) return;
+        this.blocks.stream(event).forEach(player::breakBlock);


One thing to consider here.
As discussed in the issue, the problem with this effect is that it doesn't play the sound or show the particles.
You could in theory use the Block#breakNaturally(ItemStack, (bool)trigger effects/exp) method to ensure it does.
for the item, just grab the players item in hand (or off hand).

I think you stated the event isn't called for that method tho, so you'd have to call it here as well.

Just something to think about.
Thoughts?!?!

Hmm, if it is just calling the block break event in addition to breakNaturally then I think it should just be merged into the existing effect tbh

since its worded so differently (the effect itself) im ok with it being its own effect.

We could change the wording (optionally add as %player% to the end of the effect), I don't see a point of having two effects which do the exact same thing except one also calls an event

that is true. could do an option like use item or player.
or just have two patterns:
break %blocks% [naturally] with effects [exp:[and] with (experience|exp|xp)] [using %-itemtype%]
and
make %player% break %blocks% [with effects]

I dunno if the pattern should include the event call?!?!?

I think a single pattern makes more sense, so either using %itemtype% or as %player%
I don't think it is necessary to mention the event in the pattern, in the documentation is good enough imo.

thats true, you could do
[(using %-itemtype%|by %-player%]
or something like that.

Okay I've just combined the effects into one, using two patterns since I couldn't find a good way to determine if an item type or player was being used otherwise.

Thoughts?

Personally not too happy about "The triggered on break event can be cancelled, however drops and other variables cannot be modified" but there isn't really any workarounds.

I'm not sure how I feel about one pattern calling the event, and the other not.

I think 1 of 2 things should happen here:

Revert back to using a separate effect

Have an option to call the event in the pattern (eww ... I know right?!?!)

Let's see what fuse has to say.

src/main/java/com/shanebeestudios/skbee/elements/other/effects/EffMakePlayerBreakBlock.java

ShaneBeee · 2026-01-05T21:43:03Z

src/main/java/com/shanebeestudios/skbee/elements/other/effects/EffBreakBlocksWithEffects.java

+                BlockBreakEvent breakEvent = new BlockBreakEvent(block, thePlayer);
+                Bukkit.getPluginManager().callEvent(breakEvent);
+                if (!breakEvent.isCancelled()) {


I THINK this can be shortened

Suggested change

BlockBreakEvent breakEvent = new BlockBreakEvent(block, thePlayer);

Bukkit.getPluginManager().callEvent(breakEvent);

if (!breakEvent.isCancelled()) {

if (new BlockBreakEvent(block, thePlayer).callEvent()) {

ShaneBeee · 2026-01-05T21:45:23Z

src/main/java/com/shanebeestudios/skbee/elements/other/effects/EffMakePlayerBreakBlock.java

+    protected void execute(Event event) {
+        Player player = this.player.getSingle(event);
+        if (player == null) return;
+        this.blocks.stream(event).forEach(player::breakBlock);


I'm not sure how I feel about one pattern calling the event, and the other not.

I think 1 of 2 things should happen here:

Revert back to using a separate effect

Have an option to call the event in the pattern (eww ... I know right?!?!)

Let's see what fuse has to say.

underscore95 · 2026-01-10T10:10:08Z

Hey any update on this regarding reverting back to a separate effect or calling the event in the pattern?

Fusezion · 2026-01-10T10:52:50Z

Honestly whoops, didn't realize you wanted my follow up kind of left it to Shane.
I haven't looked over too much of the conversation so expect inconsistences.

Alright so after reading over the code and it does look like you at least considered what happens if its cancelled 🎊
Tho this is only partially "fixing" bypassing this method we're still not exactly adding to the players statistic and as you said things like drops may not be changeable (tho I'll assume if it drops its calling BlockDropItemEvent)

As for the comment about splitting patterns into unique effects at this point I don't see much reason to do so.
BlockBreakEvent requires a player for even calling the event, and so long as its clearly placed in the description I don't mind it being apart of the same effect.

I get the concern about making one pattern do X while the other does Y but this is also done all over the place even in skript tho sometimes as a means to mitigate collisions & keep everything centralized which I believe is what this is doing anyways.

Strongly against adding an optional to "call" the event especially if it requires a player in the first place.

Fusezion

meant to mark my review as the above

underscore95 · 2026-01-10T14:14:15Z

I'm not sure if it calls BlockDropItemEvent or not tbh. though I don't think it matters

if breakNaturally calls the event, then that is fine
if it doesn't, I don't think specifically when adding the player should call it, and I don't think we should change it so that existing code now acts differently

I agree about the statistic, I'll confirm that block break event isn't updating it and update it myself in that case.

I agree with your take on the pattern, I'll keep that as is unless @ShaneBeee has any more concerns.

Will push something either monday or tuesday

ShaneBeee

I'll agree with Fuse on this.
I'm not too picky. I just want to make sure everything works.

Just one small nitpick:

ShaneBeee · 2026-01-10T14:59:55Z

src/main/java/com/shanebeestudios/skbee/elements/other/effects/EffBreakBlocksWithEffects.java

+        String item = this.itemType != null ? (" using " + this.itemType.toString(e, d)) : "";
+        String player = this.player != null ? (" as " + this.player.toString(e, d)) : "";
+        return "break " + blocks + " naturally with effects" + xp + item + player;


One little nitpick.
The pattern is break block naturally as player with effects and with xp
Yep the toString would come out as
break block naturally with effects and with xp as player

While this isn't really a big deal, I still think it should match the pattern.

To make things easier on yourself you could use the SyntaxStringBuilder class from Skript.

underscore95 added 2 commits January 5, 2026 10:10

Added EffMakePlayerBreakBlock

9b50c62

Added test for EffMakePlayerBreakBlock

2feb61a

Fusezion requested changes Jan 5, 2026

View reviewed changes

Update src/main/java/com/shanebeestudios/skbee/elements/other/effects…

9631f9b

…/EffMakePlayerBreakBlock.java Co-authored-by: Fusezion <fusezionstream@gmail.com>

Fusezion added the enhancement 👷🏼‍♂️ New feature or request label Jan 5, 2026

underscore95 added 2 commits January 5, 2026 10:36

Use Expression#stream, removed unnecessary annotations

9b4c338

Delete EffMakePlayerBreakBlock.sk

1699dbd

underscore95 requested a review from Fusezion January 5, 2026 13:41

Improved description for EffMakePlayerBreakBlock

32d9c60

ShaneBeee approved these changes Jan 5, 2026

View reviewed changes

Merged EffMakePlayerBreakBlock into EffBreakBlocksWithEffects

a54be6f

ShaneBeee reviewed Jan 5, 2026

View reviewed changes

Fusezion reviewed Jan 10, 2026

View reviewed changes

ShaneBeee requested changes Jan 10, 2026

View reviewed changes

Uh oh!

Added EffMakePlayerBreakBlock #889

Are you sure you want to change the base?

Added EffMakePlayerBreakBlock #889

Uh oh!

Conversation

underscore95 commented Jan 5, 2026

Describe your changes

Checklist before requesting a review

Uh oh!

Fusezion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ShaneBeee left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

underscore95 commented Jan 10, 2026

Uh oh!

Fusezion commented Jan 10, 2026

Uh oh!

Fusezion left a comment

Choose a reason for hiding this comment

Uh oh!

underscore95 commented Jan 10, 2026

Uh oh!

ShaneBeee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants