Cold Sweat

Cold Sweat

5M Downloads

Possible bug in ItemInsulationManager

judiraal opened this issue ยท 6 comments

commented

While trying to use InsulationTickEvent I noticed that insulated armor pieces work fine, but armor insulated through the sewing table does not seem to add to the insulation values reported in the event.

I think I've tracked it down to ItemInsulationManager:86 in getAllEffectiveInsulation(). There is a call to InsulatorData.test() that supplies the armor itemStack. That stack might be something like iron boots, and that is then tested against minecraft:leather. I think the test should be against pair.getFirst(), i.e. the itemStack of the insulationCap. Or am I missing a config option or misinterpreting what the code should do?

commented

You know what, I think you were right. The requirement should be testing the insulation item rather than the armor item. Guess I forgot how my own system worked :
I tested the insulation system and you're also right about that. It doesn't work properly, though the small amount of insulation provided by armor protection value does work. I will put out new versions of 2.3.10 that fix this issue and a couple other related ones I found.

commented

Great. I'm not crazy ;). Thanks for the quick reply. Really nice mod.

Btw, I'm working on an add-on for Cold Sweat. More compat, specifically with Mekanism, which has machines and heat conductors that dissipate heat to the environment and seems a nice way to heat a tech-base.

commented

There may also be circumstances with a risk for item duplication when you remove leather from a sewing table using shears. Still investigating. Will let you know when I have more info.

commented

I've also tested this in game: a full leather armor set vs an iron armor set with full leather insulation using the sewing table. Standing near lava there is a clear effect of wearing the leather armor set and no effect when wearing the iron armor set with full leather insulation.

With the default config an insulation item like leather is defined as ["minecraft:leather", 1, 1]. This leads to an InsulatorData object that has an item requirement on minecraft:leather. The code in getAllEffectiveInsulation() then supplies the armor (i.e. minecraft:iron_helmet) to the test which then fails. If that is expected behavior with the default config then that is obviously fine ;).

What should the config be to make iron armor with leather insulation work if not with the default config?

commented

The insulation item "requirement" tests the entity wearing the insulation and the armor item the insulation is sewn to. This check is done to filter out any insulation items that do not meet their requirements, and thus should not be applied to the player. This method is used when calculating the player's armor insulation, so I'm pretty sure it works as intended.

For a more technical breakdown:
An armor item's insulation is a collection of ItemStacks that each have a list of InsulatorDatas attached to them (as items can have more than one insulation valued configured for them). The InsulatorData is what performs the test() check. If it passes, it will be returned by the getAllEffectiveInsulation() method.

commented

I see you already fixed the duplication bug when using the sewing table. A similar issue exists when insulation items are dropped because they no longer meet (changed) limits on the armor piece. Specifically for the 1.21 branch its in ProcessEquipmentInsulation line 125. There should be something like this after that line:

armorStack.set(ModItemComponents.ARMOR_INSULATION, cap);