-
Notifications
You must be signed in to change notification settings - Fork 399
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
TooltipDataCallback to allow modders add their own tooltips to existing items #3403
base: 1.20.2
Are you sure you want to change the base?
Conversation
...ring-v1/src/client/java/net/fabricmc/fabric/api/client/rendering/v1/TooltipDataCallback.java
Show resolved
Hide resolved
...ring-v1/src/client/java/net/fabricmc/fabric/api/client/rendering/v1/TooltipDataCallback.java
Outdated
Show resolved
Hide resolved
...src/client/java/net/fabricmc/fabric/impl/client/rendering/tooltip/MultiTooltipComponent.java
Outdated
Show resolved
Hide resolved
...src/client/java/net/fabricmc/fabric/impl/client/rendering/tooltip/MultiTooltipComponent.java
Show resolved
Hide resolved
...src/client/java/net/fabricmc/fabric/impl/client/rendering/tooltip/MultiTooltipComponent.java
Outdated
Show resolved
Hide resolved
...dering-v1/src/client/java/net/fabricmc/fabric/mixin/client/rendering/HandledScreenMixin.java
Outdated
Show resolved
Hide resolved
...g-v1/src/client/java/net/fabricmc/fabric/impl/client/rendering/tooltip/MultiTooltipData.java
Outdated
Show resolved
Hide resolved
...v1/src/client/java/net/fabricmc/fabric/api/client/rendering/v1/TooltipComponentCallback.java
Outdated
Show resolved
Hide resolved
...ring-v1/src/client/java/net/fabricmc/fabric/api/client/rendering/v1/TooltipDataCallback.java
Outdated
Show resolved
Hide resolved
...dering-v1/src/client/java/net/fabricmc/fabric/mixin/client/rendering/HandledScreenMixin.java
Show resolved
Hide resolved
…ient/rendering/v1/TooltipComponentCallback.java Co-authored-by: Juuz <6596629+Juuxel@users.noreply.github.com>
…ient/rendering/v1/TooltipDataCallback.java Co-authored-by: Juuz <6596629+Juuxel@users.noreply.github.com>
It happened when only one tooltip data is added by event handlers I for some reason assumed that there is always original data in item
...dering-v1/src/client/java/net/fabricmc/fabric/mixin/client/rendering/HandledScreenMixin.java
Outdated
Show resolved
Hide resolved
…client/rendering/HandledScreenMixin.java Co-authored-by: Juuz <6596629+Juuxel@users.noreply.github.com>
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.
Looks good, could possibly use Mixin extras in the Mixin, but its not a major issue.
|
||
@Mixin(HandledScreen.class) | ||
class HandledScreenMixin { | ||
@Redirect(method = "drawMouseoverTooltip", at = @At(value = "INVOKE", target = "Lnet/minecraft/item/ItemStack;getTooltipData()Ljava/util/Optional;")) |
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.
A WrapOperation would be better.
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.
Mixin extras avaliable only on loader 0.15+, this version is 0.14 compatible
Changing it for one small feature doesn't make much sense (for me)
Here is ME version for 1.20.4 #3486
if (multiData.tooltipData().isEmpty()) { | ||
return original; | ||
} |
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.
Do we expect people to be allowed to remove the original?
@Redirect(method = "drawMouseoverTooltip", at = @At(value = "INVOKE", target = "Lnet/minecraft/item/ItemStack;getTooltipData()Ljava/util/Optional;")) | ||
Optional<TooltipData> addMultiData(ItemStack stack) { | ||
Optional<TooltipData> original = stack.getTooltipData(); | ||
var multiData = new MultiTooltipData(new ArrayList<>()); |
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.
Nit: No need to create MultiTooltipData here, just start with the list, and then create it later if the list > 1
import net.fabricmc.api.ClientModInitializer; | ||
import net.fabricmc.fabric.api.client.rendering.v1.TooltipComponentCallback; | ||
|
||
public class MultiTooltipComponentRegister implements ClientModInitializer { |
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.
MultiTooltipComponentInitializer
maybe, this class name is a little weird.
Remvoed from last call, there are breaking changes between the two PRs, it might just be best to have one and make sure that all the behaviour is tested/defined. |
I recently was developing feature that added "sell" and "purchase" tooltips for blocks with that functionality. The problem is that these toolips were added by different mods, one for selling, one for purchasing. And to make these mods compatible i had to develop small separate library, and package it to both mods. And i haven't even tested compatibility with other mods that may change tooltips.
The only tooltip related events I found are for adding text lines to tooltip and registering tooltip component.
So I propose new event the Fabric API - TooltipDataCallback(ItemStack, List).
Also i added three example mods to showcase what this can add (Fabric discord message link)