Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.11.2: Item.shouldCauseReequipAnimation is confusing #3915

Closed
ThexXTURBOXx opened this issue May 22, 2017 · 5 comments
Closed

1.11.2: Item.shouldCauseReequipAnimation is confusing #3915

ThexXTURBOXx opened this issue May 22, 2017 · 5 comments

Comments

@ThexXTURBOXx
Copy link

ThexXTURBOXx commented May 22, 2017

#2550 is back again. In my mod "Reforged" I try to use it, but it is just ignored. Code here: https://github.com/ThexXTURBOXx/Reforged/blob/1.11.2/java/org/silvercatcher/reforged/items/weapons/ItemCrossbow.java#L77

Edit 1: I should say, that the item "shakes" when I update its NBT in the onUpdate-method (same class)

@ThexXTURBOXx ThexXTURBOXx changed the title 1.11.2: shouldCauseReequipAnimation is ignored 1.11.2: Item.shouldCauseReequipAnimation is ignored May 22, 2017
@ThexXTURBOXx
Copy link
Author

Ok, this is where it gets funny:
I tried again and I found out, that I have to return "true", when the animation should not be played. I think this is a bug here (and the line after this one): https://github.com/MinecraftForge/MinecraftForge/blob/1.11.x/patches/minecraft/net/minecraft/client/renderer/ItemRenderer.java.patch#L79
Maybe you have to remove the "!" to invert the methods.
I don't know, if this, what you intended, but it is very confusing (how it is right now).

@ThexXTURBOXx
Copy link
Author

Even another issue here: If you return true (the animation gets not played), then the animation gets not played from the first person view. If I switch to third person view (F5), then the item "shakes" again. Other players can't see it though... Seems a little bit weird (definetly bug)

@LexManos
Copy link
Member

LexManos commented May 25, 2017

The patch is fine, I have no idea what you're doing. The code you posted returns true, which causes the requip animation. As it should...

But our test mod: https://github.com/MinecraftForge/MinecraftForge/blob/1.11.x/src/test/java/net/minecraftforge/debug/ItemLayerModelDebug.java#L59 Works just fine. No re-quip animations.

@ThexXTURBOXx
Copy link
Author

ThexXTURBOXx commented May 25, 2017

The funny part is, that I return true and it doesn't play it (if I return false, it does play). I already committed the change, but when I started this issue, the method was returning false (causing the animation)

Edit: Changed the title, the other one was misleading. I looked through your code and saw, that you return false, when the itemstack gets changed to cause the animation. But the function is called "shouldCauseReequipAnimation". If you don't want the animation, then you should return false, but that isn't the case at the moment (right now you have to return true to not play the animation) .

@ThexXTURBOXx ThexXTURBOXx changed the title 1.11.2: Item.shouldCauseReequipAnimation is ignored 1.11.2: Item.shouldCauseReequipAnimation is confusing May 25, 2017
@LexManos
Copy link
Member

LexManos commented May 25, 2017

Again we have a test mod that clearly demonstrates that it is working as intended.
Which leads me to believe it's something weird in your setup that is either screwing with our hook or causing the itemstacks to falter.
If this is really an issue for you please try to trim down your workspace and make as small of a reproduction mod as possible. Ours works fine.

Note about your project: You're missing all the gradle files so I can't even clone and test your project if I wanted to. {Without a annoying amount of work}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants