2piradians / Block-Armor

The mod can be downloaded on CurseForge at the following link:
https://minecraft.curseforge.com/projects/block-armor
6 stars 5 forks source link

Minecraft 1.12.2 - JEI and Block Armor Rendering Issue! (Debugging) #16

Closed P3rf3ctXZer0 closed 7 years ago

P3rf3ctXZer0 commented 7 years ago

System Specs and Mods and forge version https://gist.github.com/P3rf3ctXZer0/f470a73d8048e2bf58f12b2a3363f9d2

Basically the last highlighted block armor item becomes all of the block armors in the JEI gui. as seen here. 2017-09-25_15 39 49

P3rf3ctXZer0 commented 7 years ago

If I try and cheat one it all renders change but it does give me the correct item. That is how I got the gold helmet and diamond boots. Currently this bug is clearly JEI cosmetic. JEI last Updated 6 hours from this post.

https://github.com/mezz/JustEnoughItems/issues/1001

The ID for the item rendering appears random with each reboot of client.

Furgl commented 7 years ago

That is.. very interesting. We do use some unique rendering to create the inventory icons dynamically from block textures, so it is very possible that our code is making it difficult for JEI to find the correct icon. I can try to look into this and see what's going on. If mezz or someone else finds a solution before me, I will do what I can to help!

mezz commented 7 years ago

I added a rendering optimization in the latest versions of JEI, so I'll look into it first and let you know what I find.

mezz commented 7 years ago

One of my attempted optimizations is storing the IBakedModel for each item instead of querying it each time. So I call this just once:

bakedModel = itemModelMesher.getItemModel(itemStack);
bakedModel = bakedModel.getOverrides().handleItemState(bakedModel, itemStack, null, null);

It looks like you use a shared baked model for all your items and override the quads in ModelDynBlockArmor.BakedDynBlockArmorOverrideHandler#handleItemState which is not something I anticipated. That's why they all look the same in JEI. https://github.com/2piradians/Block-Armor/blob/57c1b188d4c11c10a01f5df36227d182ab05a3d6/src/main/java/twopiradians/blockArmor/client/model/ModelDynBlockArmor.java#L213-L220

Is there a reason to do it that way? It seems to me like you could just have different baked models for each item instead of storing it in a map and altering one shared model. I would just eliminate that optimization on my side since it is causing this issue, but it actually doubles the fps when rendering certain pages of modded items and doesn't seem to cause issues with other mods.

tterrag1098 commented 7 years ago

Not to mention that baked models are immutable by their nature. handleItemState is meant to return a new, modified instance.

Furgl commented 7 years ago

You're right mezz, I didn't have any reason to just use one shared model. This was my first attempt at working with baked models and I didn't entirely know what I was doing (I still am not an expert). I fixed this issue by creating different baked models for each item, as mezz suggested, and assigning the quads to the baked model in ModelDynBlockArmor.BakedDynBlockArmorOverrideHandler#handleItemState the first time that it is called (quads can't be assigned in ModelDynBlockArmor#bake because they rely on other block's quads to be baked first).

This fix is in v2.4.9: BlockArmor-1.12.2-2.4.9.jar.zip

Thank you very much mezz for taking the time to look through my code and figure out what's wrong! And thank you P3rf3ctXZer0 for reporting this issue.

P3rf3ctXZer0 commented 7 years ago

@Furgl always glad to help a modder since I found sadly I am not able to code. (Ironically despite being good at analytics I hyper focus on singular variables so coding can and would take me more than 7 years to could a single html because I also have a bad memory.)