Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes Explosive pickaxe not properly dropping blocks #4062

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Sfiguz7
Copy link
Member

@Sfiguz7 Sfiguz7 commented Dec 16, 2023

Description

When using an ExplosiveTool to break vanilla blocks in the vicinity of Slimefun Blocks, drops from the vanilla block would disappear. This adds back those drops.

ASKING FOR OTHER PEOPLE TO TEST ANYWAY BUT BELOW IS WHAT I TESTED
Tested in 1.20 the following towers of blocks (listing bottom to top, breaking the middle one)

  • Acontainer with items inside, vanilla block, sensitive blocks on both

  • Acontainer with items inside, vanilla container with items inside, sensitive blocks on both

  • Acontainer with items inside, shulker with items inside, sensitive blocks on both --> NO DUPES

  • Acontainer with items inside, vanilla ore, sensitive blocks on both, fortune on the explosive tool --> Multiple drops from the ore

  • All the above with Acontainer and vanilla swapped to reconfirm nothing broke

As Acontainer I have used an Electric Dust Washer, but they should all work the same.
As vanilla containers I have used chests, furnaces, lecterns, chiseled bookshelves, jukeboxes, all filled.

Proposed changes

When drops are dropped, check if the main block broken was vanilla. If so, drop what it should.

Related Issues (if applicable)

#4051

Checklist

  • I have fully tested the proposed changes and promise that they will not break everything into chaos.
  • I have also tested the proposed changes in combination with various popular addons and can confirm my changes do not break them.
  • I have made sure that the proposed changes do not break compatibility across the supported Minecraft versions (1.16.* - 1.20.*).
  • I followed the existing code standards and didn't mess up the formatting.
  • I did my best to add documentation to any public classes or methods I added.
  • I have added Nonnull and Nullable annotations to my methods to indicate their behaviour for null values
  • I added sufficient Unit Tests to cover my code.

@Sfiguz7 Sfiguz7 requested a review from a team as a code owner December 16, 2023 17:10
@github-actions github-actions bot added the ✨ Fix This Pull Request fixes an issue. label Dec 16, 2023
Copy link
Contributor

Your Pull Request was automatically labelled as: "✨ Fix"
Thank you for contributing to this project! ❤️

Copy link

sonarcloud bot commented Dec 16, 2023

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

5 New issues
0 Security Hotspots
71.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

github-actions bot commented Dec 16, 2023

Slimefun preview build

A Slimefun preview build is available for testing!
Commit: 68b223f5

https://preview-builds.walshy.dev/download/Slimefun/4062/68b223f5

Note: This is not a supported build and is only here for the purposes of testing.
Do not run this on a live server and do not report bugs anywhere but this PR!

@Sfiguz7 Sfiguz7 added the 🎯 Needs testing This Issue needs to be tested by our team to see if it can be reproduced. label Dec 16, 2023
WalshyDev
WalshyDev previously approved these changes Dec 17, 2023
@J3fftw1 J3fftw1 added 🎯 Needs testing This Issue needs to be tested by our team to see if it can be reproduced. Build tested Used to indicate the PR preview build has been tested by one of the team and removed 🎯 Needs testing This Issue needs to be tested by our team to see if it can be reproduced. labels Dec 31, 2023
J3fftw1
J3fftw1 previously approved these changes Dec 31, 2023
@TheBusyBot TheBusyBot added the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Dec 31, 2023
@J3fftw1 J3fftw1 removed the 🎯 Needs testing This Issue needs to be tested by our team to see if it can be reproduced. label Dec 31, 2023
@Sfiguz7 Sfiguz7 dismissed stale reviews from J3fftw1 and WalshyDev via 98b0840 February 25, 2024 17:55
Copy link
Contributor

@J3fftw1 J3fftw1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

besides the unused import this LGTM

@WalshyDev WalshyDev changed the title Fix #4051 Explosive pickaxe not properly dropping blocks Feb 25, 2024
@WalshyDev WalshyDev changed the title Explosive pickaxe not properly dropping blocks Fixes Explosive pickaxe not properly dropping blocks Feb 25, 2024
Copy link

sonarcloud bot commented Feb 25, 2024

@TheBusyBot TheBusyBot removed the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Mar 6, 2024
Comment on lines +254 to +256
// Fixes #4051
if (sfBlock == null) {
block.breakNaturally(item);
Copy link

@mcchampions mcchampions Nov 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call Block#breakNaturally will drop drops, it will cause many bugs(double drops,drops that should disappear fall normally)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build tested Used to indicate the PR preview build has been tested by one of the team ✨ Fix This Pull Request fixes an issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Break a block with an explosive block with a Slimefun block under it wont break the block mined
6 participants