search

Queerbric / Inspecio

A Minecraft mod which adds more tooltip components to items.
https://modrinth.com/mod/inspecio
GNU Lesser General Public License v3.0
102 stars 25 forks source link

Fix OutOfBoundsException in Potion/TippedArrowItemMixin #38

Closed Fourmisain closed 2 years ago

Fourmisain commented 3 years ago

I ran into this while messing around with tooltips, adding a second/sub name to items. Looking at PotionItemMixin (which is basically the exact same as TippedArrowItemMixin)

https://github.com/Queerbric/Inspecio/blob/bdeaa062c48e8170c2450c16d431103c3fb2c2de/src/main/java/io/github/queerbric/inspecio/mixin/PotionItemMixin.java#L57-L65

If e.g. tooltip.size() is 3 and oldTooltipLength is 2, it'll enter the loop, remove the tooltip at index 2 and continue the loop since tooltip.size() > 1, it'll call get at the same index which fails since index 2 is now out of bounds. Fix is to simply change the 1 to oldTooltipLength.

I rewrote the offending code but didn't quite finish it because I have a question: Why does this removal loop break if a LiteralText.EMPTY is found? Is this something for mod compat? Because this doesn't seem to happen in Vanilla at all and it seems much easier to e.g. redirect PotionUtil.buildTooltip() to do nothing.

LambdAurora commented 3 years ago

I honestly don't remember why the LiteralText.EMPTY, but I think in some cases there could be a linebreak then stuff that should not be removed...

And the reason for not using a Redirect is mod compatibility, multiple mods cannot redirect the same invoke.

Fourmisain commented 3 years ago

I honestly don't remember why the LiteralText.EMPTY, but I think in some cases there could be a linebreak then stuff that should not be removed...

Alright, kept the logic in, just in case!

I rewrote it again, it's slightly less readable but O(n) and about 30% faster in common cases - not that it really matters much.