-
Notifications
You must be signed in to change notification settings - Fork 424
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
Fix FluidStorageUtil#moveWithSound
Play Sound
#3431
Fix FluidStorageUtil#moveWithSound
Play Sound
#3431
Conversation
@@ -111,7 +111,7 @@ private static boolean moveWithSound(Storage<FluidVariant> from, Storage<FluidVa | |||
if (!fill && handItem == Items.POTION) sound = SoundEvents.ITEM_BOTTLE_EMPTY; | |||
} | |||
|
|||
player.playSound(sound, SoundCategory.BLOCKS, 1, 1); | |||
player.getWorld().playSound(player, player.getX(), player.getEyeY(), player.getZ(), sound, SoundCategory.PLAYERS, 1, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has this been tested, I believe you need to pass null as the first param otherwise this is going to play for everyone BUT the player.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method should be called both on client and server. ClientWorld.playSound
will play sound to the except
, while ServerWorld.playSound
will play sound to all players but the except
. This usage is according to vanilla usage, such as FenceGateBlock#onUse
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Humm, I tested with the chute block in the FAPI test mods, they only call this on the server. I wonder if its worth wrappning this is a !world.isClient
? Maybe @Technici4n would have some input on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well it depends on which side modders are expected to call it. It's probably worth documenting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you were right, it makes sense to call this on both sides. I have gone ahead and updated the testmod, this now works as expected. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we indicate it needs to be called on both sides in the doc?
...ransfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/fluid/FluidStorageUtil.java
Show resolved
Hide resolved
…ransfer/v1/fluid/FluidStorageUtil.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just fixed and tested this myself, Thanks.
* fix play sound * Update fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/fluid/FluidStorageUtil.java * Fix chute testmod --------- Co-authored-by: modmuss <[email protected]> (cherry picked from commit 4944b5a)
It plays sound to all players only once now.