AppleSkin

AppleSkin

236M Downloads

Issue #117 still present for the 1.17 branch for Fabric.

Ynaught opened this issue ยท 8 comments

commented

Both in versions 2.0.0 as well as 2.1.0.
Only differences from the #117 report are:

  • "The space for it, however, is there." does not apply here. The space for it does not show either.
  • This is referring to MC 1.17/1.17.1 (not mc 1.16.5).
  • This is referring to Fabric (not Forge).

Examples (Apologies for the resource pack):

Versions used in Screenshots:

  • RoughlyEnoughItems-6.0.264-alpha-fabric (This is the latest "Ready" build currently of writing this.)
  • appleskin-fabric-mc1.17-2.1.0
  • cloth-config-5.0.34-fabric
  • architectury-2.2.21-fabric
  • fabric-api-0.37.0+1.17 (a single version behind at the time of writing this, it was updated 3 days ago as of writing this. Updating it to that newer version of fabric-api-0.37.1+1.17 does not solve the issue nor seem to effect it any differently.)
  • fabric-loader-0.11.6-1.17.1

Side note:
I don't use github that often so I'm not entirely sure what all information I am supposed to provide here to aid this thread further.

commented

In REI 6.x, REF using architectury as the compatible platform. It will be compatible with Fabric/Forge at the same time, but its code is very different from Fabric/Forge.

For example:
in fabric: net.minecraft.ItemStack.getTooltip
in architectury: net.minecraft.world.ItemStack.getTooltipLines

If want to solve this problem, maybe need to provide a special version of architectury.

commented

The problem doesn't seem to be with getTooltip--net.minecraft.world.ItemStack.getTooltipLines is just a different mapped name for the same function. TooltipOverlayHandler.onItemTooltip gets called and AppleSkin adds its FoodOverlayTextComponent successfully, but then REI does its own conversion to TooltipComponents here. This bypasses AppleSkin's injection of its own conversion in 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.
//
// Note also that Screen.renderOrderedTooltip is not being injected,
// even though that's another place that calls TooltipComponent::of.
// As far as I can tell, it's not possible to Mixin Screen.renderOrderedTooltip
// in a way that would allow us to manipulate the TooltipComponent list
// (since it only lives on the stack). This shouldn't be a problem, though,
// since that method is not intended for ItemStack tooltips--it's only used
// internally for text-only tooltips and never for ItemStack tooltips.

The best fix here would be to mixin TooltipComponent::of instead which should be possible as of FabricMC/Mixin#51. I'll try that out (need to figure out what version of Fabric its included in first though).

EDIT: Nevermind, looks like Fabric's Mixin with that PR included hasn't been released/included in any Fabric release yet, so we'll have to go with another fix. The simplest might be to mixin REI's render tooltip method in the same way we mixin Screen.renderTooltip.

commented

So, if I'm reading this right, the issue is that the GUI is being replaced or something like that and that replacement is not something you can manipulate, correct?

Would it not be possible to replace the replacement? Say just have your own version of it pop-up over top of all of it as an above layer? Might look a bit messy and be a bit sloppy of a solution, but is this a possibility?

commented

Found a solution: da1a149

Will try to release a fixed version soon.

commented

Should be fixed in v2.1.1

commented

I'll give it a test within the next 24hrs and report back on how it goes just to make sure everything's square.

commented

Yup, looks good.