Noaaan / MythicMetals

Fabric based Minecraft mod that adds new materials into the game. Includes new tools, ores, anvils, and sets of armor.
Other
73 stars 26 forks source link

Added support for Trinkets API #238

Closed jsbursik closed 2 months ago

jsbursik commented 2 months ago

All that was needed for Issue #181 was adding the tags for the Celestium Elytra for it to be recognized as elytra by the Trinkets API.

Tested in game and was able to apply the Elytra to a trinket slot (using Elytra Slot mod)

jsbursik commented 2 months ago

Actually - not fixed. It seems that the bonuses aren't working correctly. Looking into it further...

Edit: Other mods like BetterEnd also have this issue where stats don't apply from the trinket slot, doing some more research.

Noaaan commented 2 months ago

This is kinda the main reason I have put off implementing this at the moment. Yes, adding it to the cape slot would be sufficient in order to equip it. However, the main issues is correct rendering and attributes. I am unsure if I want attributes to apply, and to handle that I do believe it would require adding Trinkets API as a dependency. I also plan on adding a custom Elytra renderer as well, and handling this correctly will be important. This is why I commented this on the issue, since it isn't high priority to add this right now:

the point of the Celestium Elytra is that you don't need the attributes while in a separate slot, as it is an armored Elytra.

I assume you have tested this PR: how does it work currently in practice? If it works ok I might consider merging it

jsbursik commented 2 months ago

Yeah, I'm starting to understand how this is working now. The tag only really applies basic Elytra functionality when used in the trinket slot.

I didn't even think to check the rendering, it doesn't show up when in the cape slot. The way I tested originally was just through a single player world, then I copied over the patched jar to a server of mine to test it out there as well.

Maybe go the BetterEnd route and render the chest armor + elytra when equipped to the chest slot? Then there wouldn't be a dependency for Trinkets and the aesthetic need would be fixed.

Or maybe I'll get bored and write a compat mod.

Noaaan commented 2 months ago

I have seen that and it is something I am considering, yeah. Again, most of it hinges on when (eventually) I get/finish commissioning the new textures + models for it.

jsbursik commented 2 months ago

Closing PR as it really shouldn't be merged. Really only a band-aid fix with underlying issues.