This is still not fixed correctly in 1.13.2.
The 1.13.2 code for TileEntityRendererDispatcher.render
(in MCP names):
if (!hasNoBlock && (!tileEntityIn.hasWorld() || !tileEntityIn.getBlockState().getBlock().hasTileEntity())) {
return;
}
tileentityrenderer.render(tileEntityIn, x, y, z, partialTicks, destroyStage);
In this case the previous check for tileEntityIn.getBlockState().isAir()
has been replaced with a check for tileEntityIn.getBlockState().getBlock().hasTileEntity()
, but this is still not fully correct.
In the case of two blocks: A (with associated block entity X), and B (with associated block entity Y), where A and B do not have the same state properties, then rapidly switching A <-> B will still crash.
One of the principle underlying causes of the issue here is blocks being replaced with a different block. The key property of the new block here is simply that it is not the old block and therefore can have an entirely different set of state properties and behaviours. It is not appropriate for an entity associated with the first block to operate on the second, which is why, when such a block change occurs, the old block entity is removed.
The issue here is that the render code still has a reference to that old entity and attempts to render it.
The proper solution for that issue is simply to ensure that the intended removal of the old block entity is correctly detected, which is probably best comunicated directly through the block entity itself.
The remaining part of the issue is the matter of crashes upon respawning, which are caused due to attempting to render block entities from unloaded chunks. Either unloaded chunks need to be checked for here, or, preferably, block entities in unloading chunks should be properly marked as pending removal.
See https://github.com/MinecraftForge/MinecraftForge/pull/5512 for further explanation.
MC-134542 is not a duplicate - it clearly affects the server-side logic, whereas this crash is in client-side rendering code.
This most likely has the same cause and fix as MC-123363, as that issue does not appear to have been resolved correctly in-code.
Assuming the relevant code hasn't changed since 1.13, this does not appear to have been fixed correctly.
Crashes can still occur in the cases where the affected blocks are replaced with anything that isn't air.
The check for air should be replaced with a check of the appropriate flag, as per the original recommendation.
Minecraft.setDimensionAndSpawnPlayer
creates a new player entity, mostly losing the field values from the old one.
This causes the client to "forget" that the player was previously sneaking, and so it will not send a status update packet to the server as it believes that the sneak status is unchanged, causing the issue.
This issue can be fixed by changing setDimensionAndSpawnPlayer
to copy the values used to determine when status update packets are sent (inside EntityPlayerSP.onUpdateWalkingPlayer
) across to the newly-created player.
Reposting explanation and fix from https://github.com/MinecraftForge/MinecraftForge/pull/4811
When rendering tile entities, RenderGlobal.renderEntities
loops over the list given by CompiledChunk.getTileEntities
for each chunk section, and calls TileEntityRendererDispatcher.render
on each one.
The list of tile entities with custom renderers is populated from RenderChunk.rebuildChunk
, while looping over block positions and rendering regular block models.
As rebuildChunk
may be running on a worker thread, it is possible for a tile entity that is added to that list to subsequently be removed from the world by the main thread.
Then, when renderEntities
is called, it may try to render a tile entity that no longer exists at that position, which causes a crash if the render
method attempts to access any property of the no-longer-present blockstate.
Vanilla's TESR implementations are generally unaffected in 1.12 because they tend to use TileEntity.getBlockMetadata
instead of direct property value lookups. It seems that the removal of metadata in the 1.13 snapshots caused the problem to become visible there.
This is resolved here by adding a check for TileEntity.isInvalid
to the renderer lookup. Tile entities that have been removed from the world will have been invalidated, so adding this check prevents attempts to render them from causing crashes.
This is most likely caused by the chunk section the entity is in (as determined by the entity's position, at the bottom-centre of it's AABB) moving out of the view frustum, while part of the entity's body remains within.
While the entity would be considered visible according to its own render bounding box, the game only considers entities in the chunk sections that aren't culled, so the entity isn't rendered.
This is why it mostly affects entities standing at a position just under the top of a chunk section (y-level just under a multiple of 16).
One possible solution is to add an entity-sized margin to the render chunk bounding box checks (in RenderGlobal#setupTerrain
).
As stated previously, the problematic code code here is inside World.getRawLight
.
The current code:
IBlockState iblockstate1 = this.getBlockState(pos);
int j2 = lightType == EnumSkyBlock.SKY ? 0 : iblockstate1.getLightValue();
int k2 = iblockstate1.getLightOpacity();
if (k2 >= 15 && iblockstate1.getLightValue() > 0)
{
k2 = 1;
}
if (k2 < 1)
{
k2 = 1;
}
if (k2 >= 15)
{
return 0;
}
else if (j2 >= 14)
{
return j2;
}
else
{
// check light values at adjacent positions
}
Suggested fix:
IBlockState iblockstate1 = this.getBlockState(pos);
int j2 = lightType == EnumSkyBlock.SKY ? 0 : iblockstate1.getLightValue();
int k2 = iblockstate1.getLightOpacity();
if (k2 < 1)
{
k2 = 1;
}
if (k2 >= 15 || j2 >= 14)
{
return j2;
}
else
{
// check light values at adjacent positions
}
Rather than setting light-emitting opaque blocks to have an effective opacity of 1, the new code just returns the source light for the block, instead of 0.
This is the value that would be returned if the full logic ran - the reason for the early return is just to save running the extra code if it is known not to affect the result.
Given a maximum light value of 15, and a minimum reduction of 1 (due to distance), this is the case if:
The block has an opacity of at least 15, and so will nullify any contribution from adjacent blocks.
The block has a source light level of at least 14, which is the maximum it can receive from adjacent blocks.
Saw this in 1.12.2
[File IO Thread/INFO]: [STDERR]: java.io.IOException: Stream Closed
[File IO Thread/INFO]: [STDERR]: at java.io.RandomAccessFile.seek0(Native Method)
[File IO Thread/INFO]: [STDERR]: at java.io.RandomAccessFile.seek(Unknown Source)
[File IO Thread/INFO]: [STDERR]: at ayj.a(SourceFile:323)
[File IO Thread/INFO]: [STDERR]: at ayj.a(SourceFile:263)
[File IO Thread/INFO]: [STDERR]: at ayj$a.close(SourceFile:244)
[File IO Thread/INFO]: [STDERR]: at java.util.zip.DeflaterOutputStream.close(Unknown Source)
[File IO Thread/INFO]: [STDERR]: at java.io.FilterOutputStream.close(Unknown Source)
[File IO Thread/INFO]: [STDERR]: at java.io.FilterOutputStream.close(Unknown Source)
[File IO Thread/INFO]: [STDERR]: at aye.b(SourceFile:160)
[File IO Thread/INFO]: [STDERR]: at aye.a(SourceFile:145)
[File IO Thread/INFO]: [STDERR]: at bgx.c(SourceFile:37)
[File IO Thread/INFO]: [STDERR]: at bgx.run(SourceFile:30)
[File IO Thread/INFO]: [STDERR]: at java.lang.Thread.run(Unknown Source)
As for code where this can happen, sendToPlayers
is called by PlayerChunkMap#getOrCreateEntry
if it creates a new entry (as there's no existing one). As the entry has just been created, it has no players attached to it yet.
Then, provided the chunk was successfully loaded, and is a chunk that has previously been populated, the situation described can occur.
Having had a chance to look at this earlier, here's an explanation of the bug.
The issue is in EntityRenderer.updateFogColor()
and is the cause of both MC-4647 and MC-10480.
Looking at the relevant section of code there (based on MC 1.12.1):
double d1 = (entity.lastTickPosY + (entity.posY - entity.lastTickPosY) * (double)partialTicks) * world.provider.getVoidFogYFactor();
if (entity instanceof EntityLivingBase && ((EntityLivingBase)entity).isPotionActive(MobEffects.BLINDNESS))
{
int i = ((EntityLivingBase)entity).getActivePotionEffect(MobEffects.BLINDNESS).getDuration();
if (i < 20)
{
d1 *= (double)(1.0F - (float)i / 20.0F);
}
else
{
d1 = 0.0D;
}
}
if (d1 < 1.0D)
{
if (d1 < 0.0D)
{
d1 = 0.0D;
}
d1 = d1 * d1;
this.fogColorRed = (float)((double)this.fogColorRed * d1);
this.fogColorGreen = (float)((double)this.fogColorGreen * d1);
this.fogColorBlue = (float)((double)this.fogColorBlue * d1);
}
if (this.bossColorModifier > 0.0F)
{
float f14 = this.bossColorModifierPrev + (this.bossColorModifier - this.bossColorModifierPrev) * partialTicks;
this.fogColorRed = this.fogColorRed * (1.0F - f14) + this.fogColorRed * 0.7F * f14;
this.fogColorGreen = this.fogColorGreen * (1.0F - f14) + this.fogColorGreen * 0.6F * f14;
this.fogColorBlue = this.fogColorBlue * (1.0F - f14) + this.fogColorBlue * 0.6F * f14;
}
if (entity instanceof EntityLivingBase && ((EntityLivingBase)entity).isPotionActive(MobEffects.NIGHT_VISION))
{
float f15 = this.getNightVisionBrightness((EntityLivingBase)entity, partialTicks);
float f6 = 1.0F / this.fogColorRed;
if (f6 > 1.0F / this.fogColorGreen)
{
f6 = 1.0F / this.fogColorGreen;
}
if (f6 > 1.0F / this.fogColorBlue)
{
f6 = 1.0F / this.fogColorBlue;
}
this.fogColorRed = this.fogColorRed * (1.0F - f15) + this.fogColorRed * f6 * f15;
this.fogColorGreen = this.fogColorGreen * (1.0F - f15) + this.fogColorGreen * f6 * f15;
this.fogColorBlue = this.fogColorBlue * (1.0F - f15) + this.fogColorBlue * f6 * f15;
}
After some calculations, the fog colour values are multiplied by the value d1
. If this value is zero, which can occur both due to being subject to the blindness effect and also a negative y position, then all three colour components of the fog will be zero upon entering the block of code applying the night-vision effect. This is what triggers the bug.
Within the night-vision code, the value f6
is calculated as the minimum of the reciprocal of the red, green, blue colour values. For the case where these are all 0, this value will be infinity.
The colour values are then linearly interpolated (based on the night-vision intensity value, f15
) between their original values, and the value multiplied by the calculated factor f6
. In the case described above, this multiplication will be 0 * infinity, which has no defined value, and will evaluate to NaN (Not a Number). The NaN value will propagate here, leading to a final NaN value being assigned as the fog colours.
These invalid values are then passed to various OpenGL fog functions, which is what seems to be causing the effects described. As NVidia cards do have alternative fog rendering methods (that are used by Minecraft - the optional "GL_NV_fog_distance" extension) it's likely that the implementation used handling this differently is the reason why it's hardware dependent.
The night-vision code needs to handle this edge case, to avoid passing invalid values here. If the developers do intend on having the end result, it should be coded specifically, rather than relying on implementation quirks.
Notes:
See the OpenGL spec (section 2.3.4.1: Floating-Point Computation) for some info about how various floating-point values are to be handled.
The issue appears to be that some values can be outside of the range expected by the night vision effect code.
Fix is easy, just requires moving the code in EntityRenderer.updateLightmap()
a little:
private void updateLightmap(float partialTicks)
{
// ...
// move this block from here ---
if (this.mc.player.isPotionActive(MobEffects.NIGHT_VISION))
{
float f15 = this.getNightVisionBrightness(this.mc.player, partialTicks);
float f12 = 1.0F / f8;
if (f12 > 1.0F / f9)
{
f12 = 1.0F / f9;
}
if (f12 > 1.0F / f10)
{
f12 = 1.0F / f10;
}
f8 = f8 * (1.0F - f15) + f8 * f12 * f15;
f9 = f9 * (1.0F - f15) + f9 * f12 * f15;
f10 = f10 * (1.0F - f15) + f10 * f12 * f15;
}
if (f8 > 1.0F)
{
f8 = 1.0F;
}
if (f9 > 1.0F)
{
f9 = 1.0F;
}
if (f10 > 1.0F)
{
f10 = 1.0F;
}
// to here ---
float f16 = this.mc.gameSettings.gammaSetting;
float f17 = 1.0F - f8;
float f13 = 1.0F - f9;
float f14 = 1.0F - f10;
f17 = 1.0F - f17 * f17 * f17 * f17;
f13 = 1.0F - f13 * f13 * f13 * f13;
f14 = 1.0F - f14 * f14 * f14 * f14;
f8 = f8 * (1.0F - f16) + f17 * f16;
f9 = f9 * (1.0F - f16) + f13 * f16;
f10 = f10 * (1.0F - f16) + f14 * f16;
f8 = f8 * 0.96F + 0.03F;
f9 = f9 * 0.96F + 0.03F;
f10 = f10 * 0.96F + 0.03F;
// ...
}
This is still an issue for reading the file in 1.14. Writing the file does correctly specify a fixed UTF-8 format.