Roughly Enough Items Fabric/Forge/NeoForge (REI)

Roughly Enough Items Fabric/Forge/NeoForge (REI)

40M Downloads

[1.17 Fabric] Tooltip woes, AppleSkin compatibility

squeek502 opened this issue ยท 11 comments

commented

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 Tooltips, 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.

commented

I just saw your implementation of the API, just wanted to say that the tooltip passed to you may be null

commented

Thanks for the heads up; fixed

commented

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.

commented

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.

commented

Note that there are two things that get converted to a TooltipComponent before rendering:

  • OrderedText, which is typically for strings. This is the List<Text> part of tooltips. This is what AppleSkin currently uses, with a custom conversion to a custom TooltipComponent implementation
  • TooltipData, which is currently only used for BundleTooltipData 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.

commented

I just changed my comment since I was confused for a second, would what I meant actually make sense now?

commented

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.

commented

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. ๐Ÿ˜•

commented

Thanks, I think that'll work if I understand it correctly!

commented

I have added an API to do this, see EntryRendererRegistry

commented

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)