search

emilyploszaj / trinkets

A data-driven accessory mod and API for Minecraft using Fabric.
https://www.curseforge.com/minecraft/mc-mods/trinkets
MIT License
165 stars 72 forks source link

Update to 1.20.6 #310

Closed Patbox closed 3 months ago

Patbox commented 4 months ago

This pr updates trinkets to version 1.20.6, with minimal api changes to make it match with how MC handles things (mostly moving from EntityAttribute to RegistryEntry<>, as MC requires them everywhere) and to update the dependencies.

Additionally it adds basic support for datafixers updating ItemStacks stored in trinket's inventory (this is unlikely to require any changes in the future).

Solves #307

Patbox commented 3 months ago

Done

HumanGamer commented 3 months ago

I just tried this 1.20.6 version and noticed an issue. With keep inventory enabled, on death, trinkets get deleted. I tried to debug this but couldn't figure out why. Not sure if this is an issue outside of this PR but I've only tried on 1.20.6 with this.

Patbox commented 3 months ago

Should be fixed now

Patbox commented 3 months ago

Fixed all the style issues.

I'm keeping datafixer, but commented on what every part of it does

Lakuna commented 3 months ago

Hello @Patbox,

I updated one of my mods to 1.20.6 using this fork and found that Trinkets is now required on the client side. This appears to be related to Ladysnake/Cardinal-Components-API#174, which is addressed in the migration guide here. It would be nice if this could be fixed before this fork is merged (it appears to work fine otherwise).

Thanks!

Patbox commented 3 months ago

@Lakuna Could you tell me which case you would need that? Trinkets (at least the original version) never really worked with vanilla/non-trinkets environments, as it's not possible to interact with these slots at all. It can also be confusing, as trinkets changes shift clicking behaviour to put items within it's slot to add support for it. Additionally it wouldn't work anyway, as TrinketsAttributeModifiersComponent, which was added to replicate attribute support in nbt, but also can be used for defining the attributes in Item.Settings is required to be on both sides

Lakuna commented 3 months ago

@Lakuna Could you tell me which case you would need that? Trinkets (at least the original version) never really worked with vanilla/non-trinkets environments, as it's not possible to interact with these slots at all. It can also be confusing, as trinkets changes shift clicking behaviour to put items within it's slot to add support for it. Additionally it wouldn't work anyway, as TrinketsAttributeModifiersComponent, which was added to replicate attribute support in nbt, but also can be used for defining the attributes in Item.Settings is required to be on both sides

My server was previously able to be connected to by vanilla clients who didn't wish to use trinkets or by clients with the mod who did want to use trinkets. The only mod that I use that "adds" a trinket is one that allows Elytra to be used in the cape slot. Since this update, players that belong to the former group are no longer able to connect. It would be strange to me if this update would impose a restriction that didn't exist beforehand.

Patbox commented 3 months ago

Going to leave that decision for @emilyploszaj . For myself I will update the polymer port for server side usage

rakantoh commented 3 months ago

sorry if i a noob, but where i can download the commits/forks made by @Patbox? Thanks.

Patbox commented 3 months ago

Not sure how I missed that one again, but now it's changed