Fabric API

Fabric API

108M Downloads

ItemStack.getAttributeModifiers can't be easily influenced for foreign items

Flame115 opened this issue ยท 21 comments

commented

didnt create a crashlog but this is the log file i do have.

https://pastebin.com/vK5EfqvU

commented

I'd recommend removing Not Enough Crashes as it is conflicting with better loading screen.

Also, Cull Leaves mod is conflicting with Sodium but not crashing as far as I see.

commented

Looks like Combat Tweaks has conflicting redirect mixin with Fabric Item API.

Combat Tweaks' redirect
Fabric API's redirect

Not sure how to fix this though ๐Ÿ˜ž
@Technici4n @CammiePone

commented

I suggest that CombatTweaks should fix it.

commented

@CammiePone said she'd fix it on her side, so closing.

commented

Yea, I'll get to fixing it eventually. For now, if you wanna use CCT, just use an older version of FAPI. I plan to use MixinExtras to remove all of my redirects in my mods eventually.

commented

That doesn't help if fabric API doesn't use it though. Something to consider I guess.

commented

Oh I assumed if my mixin applied first, Fabric API's would just redirect the call in the parameters of my inject

commented

Wait, what is even the purpose of this redirect? It does the exact same thing as vanilla does it not?

image
image

commented

My mixin does data-driven attribute modifiers, hence why I redirect on that method. I'm not sure what the Fabric API redirect is even used for.

I get that it's for stack-aware attributes, but also you can literally get the stack with the entity and the slot, and odds are if you have access to the slot, you can get access to the entity somewhere down the line.

commented

Wasn't linking attributes to your entity locked to mods when the fabric entity attribute api was made

sorry saw attributes thought it was entities, this is item stuff.

commented

Looking at it, it's just a hook extension for stack-aware attributes as you said. I don't think removing it is ideal since having these hooks inside an Item class simplifies logic a lot and saves the modder time in implementing it. Otherwise, "somewhere down the line" really just becomes having to shove down logic wherever you can find a space for that.

commented

B sounds interesting. What use cases would this event have? (other than your use case)

commented

Only ways I can see this working is if:
a) we both use MixinExtras
b) an event is added to FAPI so I can move my stuff to that, or
c) I attempt to implement the stack-aware attributes on my end, set mine to a ModifyVariable to completely negate FAPI's mixin, and pray that works

Option A would be great for other modders since then they get access to MixinExtras, but seems the least likely to happen. Option B is probably the best middle-ground since then we both get what we want simultaneously. Option C is what's probably going to happen because what recourse do I have against the API everyone uses suddenly bricking my mixin?

commented

Modders adding attributes to other items in-game, whether they be vanilla or modded, especially their own attributes (i.e. someone makes a magic resistance attribute they want to apply to gold armour)

commented

IMO having an event would be reasonable, changing Mixin in any form is currently no priority. Minecraft relies on item attributes staying consistent for the same stack, so such an event would look exactly the same as the Item extension and run after the item method invocation.

commented

The event is a good idea, but in the meanwhile it looks like it should work fine with a ModifyVariable.

commented

The event would give a chance to modify the existing map. How is it different from a ModifyVariable placed after the method calls?

commented

It would, assuming I can capture it and manage to apply after your redirect. ModifyVariable likes to not work half the time it should for me. Sadly, only thing I can reliably target is when the variable is stored, and I dont remember how much redirects change the code, or if they add additional lines anywhere.

commented

INVOKE_ASSIGN didnt let me target the correct variable last time I tried it with a Modify Variable in FluidRenderer, so I think it targets in the bytecode before the variable is actually stored.

In this case it might work since the variable already exists, but I'll have to check tomorrow.

commented

I'm pretty sure they just replace the method call. It should be safe to target the vanilla method with INVOKE_ASSIGN, in a mixin that applies before fabric's. Then fabric's mixin will replace the method call and your modify variable hook will still be there. You can add -Dmixin.debug.export=true to check.

commented

Closed by #2175.