[1.17 Fabric] Tooltip woes, AppleSkin compatibility
squeek502 opened this issue ยท 11 comments
Currently, AppleSkin is doing some crazy stuff to get its tooltip rendering to work. I want to fix that, but am running into some issues.
Currently, AppleSkin inserts a custom Text
class into tooltip List<Text>
's and then injects callers of TooltipComponent::of
to replace the TooltipComponent in the generated list with AppleSkin's custom TooltipComponent (here's the link to the ScreenMixin):
// The idea here is to manually swap out the auto-created OrderedTextTooltipComponent
// for our more specific one at each place TooltipComponent::of is called
//
// Ideally, we'd just inject TooltipComponent::of and be done with it,
// but Mixin doesn't support injecting static interface methods:
// https://github.com/SpongePowered/Mixin/issues/318
//
// Note that we are assuming a 1:1 relationship between Text and TooltipComponent,
// and that each Text and corresponding TooltipComponent use the same index
// in their lists. If either of those assumptions are untrue, we'll get some
// funky results.
AppleSkin does the same for REI, injecting REI's ScreenOverlayImplImpl.renderTooltipInner
. This is not ideal and is probably leading to crashes like squeek502/AppleSkin#145.
Now that Fabric's Mixin can handle static interface methods, though, I'd like to update AppleSkin to inject TooltipComponent::of
instead, avoiding the need for any other injections/craziness. I've got this part working, but the problem is that REI coverts AppleSkin's FoodOverlayTextComponent
into StyledString
when it calls MinecraftClient.getInstance().textRenderer.getTextHandler().wrapLines
on it here, meaning that by the time TooltipComponent::of
is called, AppleSkin's custom Text
implementation is lost.
There is also TooltipData
, but that's also a minefield, in that ItemStack.getTooltipData
only accepts a single TooltipData
return value, and because of the conversion to TooltipComponent
, I don't think it's all that feasible to create a cross-mod TooltipData
container that accepts multiple TooltipData
's (Inspecio uses a multi-TooltipData-container, but it only has to handle it's own TooltipData, so it can get away with it)
I noticed that REI has a separate components
list in its Tooltip
s, but was unable to see an easy way to insert AppleSkin's component into that.
Basically, I'm at a loss as to how to get this to work without really hacky solutions and I'm open to any suggestions.
I just saw your implementation of the API, just wanted to say that the tooltip passed to you may be null
Thanks for the heads up; fixed
Inspecio uses TooltipData
which is separate from the List<Text>
that normal tooltips use. I could in theory do the same thing Inspecio is doing, but I don't think it's feasible without breaking compatibility with Inspecio (i.e. I don't think Inspecio's approach can be used by more than 1 mod at a time without a shared compatibility layer).
Because of this, AppleSkin does not use TooltipData
and instead appends a custom Text
implementation to the List<Text>
that normal tooltips use, so REI's isText()
returns true.
Is it possible that you would use the same mixin as Inspecio does: https://github.com/Queerbric/Inspecio/blob/1.17/src/main/java/io/github/queerbric/inspecio/mixin/ScreenMixin.java
REI's conversion does the TooltipComponents separately, the code you were looking at is for Texts only, it calls the lambda in the Screen class, which the mixin by Inspecio injects into.
Note that there are two things that get converted to a TooltipComponent
before rendering:
OrderedText
, which is typically for strings. This is theList<Text>
part of tooltips. This is what AppleSkin currently uses, with a custom conversion to a customTooltipComponent
implementationTooltipData
, which is currently only used forBundleTooltipData
by Minecraft. An ItemStack can only return one of these. This is what Inspecio uses, but because of the restriction to only one-per-ItemStack, it's difficult to get multiple working across different mods
The conversion is what is causing the problem, though. If there's a way to bypass the conversion and insert TooltipComponent
's directly into REI's Tooltip
then that'd work fine.
I just changed my comment since I was confused for a second, would what I meant actually make sense now?
So what I am understanding here is that if Inspecio and AppleSkin both wanting to add TooltipComponents to items, it would only show one of then. If so, I can provide an API to inject multiple tooltips components into REI.
I think different mappings might also be confusing the terms being used here. I'm using the Yarn mappings. It looks like what I'm calling TooltipComponent
is called ClientTooltipComponent
in the REI source, and what I'm calling TooltipData
is called TooltipComponent
in the REI source. ๐
I believe this will need to be re-opened for 1.19, as the change mentioned here seems to regress my use-case outlined here (that is, TooltipComponent
(Mojmap) / TooltipData
(Yarn) won't work for me unless I'm missing something)
EDIT: Nevermind, ignore this, I found a way to update my code to work when adding a TooltipData
. The fix can be seen in squeek502/AppleSkin@17c2ae9
(note that I'm unable to get REI running in my 1.19 dev environment currently, as I'm running into java.lang.ClassNotFoundException: net.fabricmc.fabric.api.command.v1.CommandRegistrationCallback
from architectury when using modImplementation "me.shedaniel:RoughlyEnoughItems-fabric:9.0.466"
, and if I include fabric-api-deprecated
then I get injection failures in ClientPlayNetworkHandler
)