Skin Layers 3D (Fabric/Forge)

Skin Layers 3D (Fabric/Forge)

47M Downloads

No null check for Minecraft.getInstance().player, causing crash

enjarai opened this issue ยท 6 comments

commented

I've noticed that in PlayerRendererMixin::setModelProperties you use Minecraft.getInstance().player without a null check.
image

Normally this wouldn't be a problem, since no players will get rendered unless the user has a world loaded. However, this causes issues when a mod tries to render a PlayerEntity while no world is loaded, which one of my mods does. See crash report here.

This could likely be fixed very easily by adding a null check that either: cancels the method, or preferably, skips the distance check that causes the issue.

commented

Will add a null-check in the next release.

commented

I've recieved another crash report from a player having issues with a missing null check.

crash-2022-09-22_10.49.46-client.txt

After scanning through your code, I think the problem is on this line, but I'm not completely sure. You're probably more familiar with your own code.

if(livingEntity.distanceToSqr(Minecraft.getInstance().player) > SkinLayersModBase.config.renderDistanceLOD*SkinLayersModBase.config.renderDistanceLOD) {

Could you add another null check here? I can also try to find all problematic spots myself and open a PR if needed, but I might miss some.

commented

Looks like nl.enjarai.showmeyourskin.gui.widget.ArmorConfigWindow renders players while not inside a world, but sets the world and not the player? I had to add checks in the past for mods that render players in the main menu, but somehow that mod seems to break something about it?

commented

I'm not sure if other mods modify client.player, but I don't, since that feels like something that would have unintented consequences. I don't know what else could be different about my mod compared to other mods that render players outside a normal world.

commented

I've had surprisingly few reports about incompatibilities with mods that don't nullcheck client.player, so its not a big problem imo. But like you say, there's no one size fits all solution to this.

I'll probably add a "compatibility mode" or something to my mod in the future, making it disable the previews when no world is loaded. By default they'll still be enabled though, since they're quite important to my UI design.

Its fully understandable if you don't want to bother supporting this, but I'd appreciate it if you do. :)

commented

Ok now I'm home and can actually look at the stack trace with mappings. The issue is that it's rendering a player head(the item) while not in a world.

I don't know what else could be different about my mod compared to other mods that render players outside a normal world.

This is in general an issue since there is no vanilla equivalent. So there is no right way to do it, and any way will cause other problems. You must have some dummy world to render that fake entity, so it could be argued that during the rendering, the Minecraft.level should be set to that. But then you have a level and no player, which is an invalid state, so adding a fake player there too(or having the rendered player be the fake player). This would fix issues with this mod, but might cause other mods to misbehave.

That's the reason I decided to not bother, and just don't show player previews while not ingame(3d skin layers/wave capes). Guess I'll add another null check for the next version, but it's more a bandaid than anything.