-
Notifications
You must be signed in to change notification settings - Fork 433
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
Add BlockBreakEffectsCallback #1023
base: 1.16
Are you sure you want to change the base?
Conversation
...ts-interaction-v0/src/main/java/net/fabricmc/fabric/api/event/BlockBreakEffectsCallback.java
Outdated
Show resolved
Hide resolved
Should probably add a testmod too |
I did add a testmod though? |
...ts-interaction-v0/src/main/java/net/fabricmc/fabric/api/event/BlockBreakEffectsCallback.java
Outdated
Show resolved
Hide resolved
...ts-interaction-v0/src/main/java/net/fabricmc/fabric/api/event/BlockBreakEffectsCallback.java
Outdated
Show resolved
Hide resolved
...ts-interaction-v0/src/main/java/net/fabricmc/fabric/api/event/BlockBreakEffectsCallback.java
Outdated
Show resolved
Hide resolved
...ts-interaction-v0/src/main/java/net/fabricmc/fabric/api/event/BlockBreakEffectsCallback.java
Outdated
Show resolved
Hide resolved
...ts-interaction-v0/src/main/java/net/fabricmc/fabric/api/event/BlockBreakEffectsCallback.java
Outdated
Show resolved
Hide resolved
…/api/event/BlockBreakEffectsCallback.java Co-authored-by: liach <[email protected]>
...ts-interaction-v0/src/main/java/net/fabricmc/fabric/api/event/BlockBreakEffectsCallback.java
Outdated
Show resolved
Hide resolved
Co-authored-by: liach <[email protected]>
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.
Everything looks good code wise, but I guess I just wonder if maybe the two events ought to be separated. Because both methods are using the same event signature it means we're limited to the common information between the two methods and we're missing out on some extra information that could be useful in certain situations. I guess it really comes down to the comment I made at the BlockMixin, I don't specifically know what each of these methods have in terms of parameters, so I don't know what kind of useful information they might be able to relay to the event. You know these methods better than I do, so if you think someone might benefit from having those additional variables passed in then I might recommend the two events get separated for greater control.
...ts-interaction-v0/src/main/java/net/fabricmc/fabric/api/event/BlockBreakEffectsCallback.java
Show resolved
Hide resolved
...nts-interaction-v0/src/main/java/net/fabricmc/fabric/mixin/event/interaction/BlockMixin.java
Show resolved
Hide resolved
What do you mean? There isn't any information that isn't common between the two mixins. They both have the entity that broke the block, the position of the block, the block state, and the world. The |
Is there anything else I should do? |
...nts-interaction-v0/src/main/java/net/fabricmc/fabric/mixin/event/interaction/BlockMixin.java
Show resolved
Hide resolved
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.
Upon further consideration, I believe the block mixin will suffice.
Any update on this? Should I rebaee it on 1.17? |
Keep this on 1.16, we can cherry pick at merge time |
Sometimes you might not want the block break particles and sounds to appear for a block. However, the only good way to do this (if you want access to the breaking entity) is a
@Redirect
inWorld#breakBlock
. This has already caused a conflict between Magna and EnergonRelics.