illusivesoulworks / constructsarmory

A Tinkers' Construct add-on for those looking to enter the world of armor
https://www.curseforge.com/minecraft/mc-mods/constructs-armory
Other
54 stars 36 forks source link

`PreviewPlayer` can have null playerInfo but return true in `hasPlayerInfo()` #277

Open serenibyss opened 2 years ago

serenibyss commented 2 years ago

Versions (Be specific, do not write "latest"):

Observed Behavior:

Calling hasPlayerInfo() on an instance of AbstractClientPlayer (specifically in RenderPlayerEvent.Pre in this case) and receiving a return value of true still leads to a NPE on accessing the playerInfo field.

Expected Behavior:

When calling hasPlayerInfo() on an instance of AbstractClientPlayer, a return value of true should mean that accessing the playerInfo field of that Object should not be null.

Steps to Reproduce:

This requires some other mod since this breaks an invariant of the AbstractClientPlayer class that is expected to be true. This can be observed with any currently released version of GregTech CE Unofficial (v2.1.4 or earlier, as we have worked around it in subsequent releases).

In c4.conarm.client.gui.PreviewPlayer, an override of hasPlayerInfo() should be sufficient to address this issue. Unfortunately since getPlayerInfo() is protected, there is no way for us to call that instead of directly accessing the field. As a workaround, we have resulted in null-checking the playerInfo field, but this issue could cause issues with other mods who have not done this fix.

Crash Log: https://pastebin.com/dAbNcjcd

Issue from our end: https://github.com/GregTechCEu/GregTech/issues/615