[1.8] Getting BlockAir BlockPos Passed

[RANKSHANK]

I'm getting random periodic (happens within about 10 minutes) crashes due to BlockAir being passed through my block's isAir(IBlockAccess, BlockPos).

 

Crash line

java.lang.IllegalArgumentException: Cannot get property PropertyInteger{name=meta, clazz=class java.lang.Integer, values=[0, 1, 2, 3]} as it does not exist in BlockState{block=minecraft:air, properties=[]}

 

isAir method

@Override
public boolean isAir(IBlockAccess w, BlockPos pos){
	 return (Integer)w.getBlockState(pos).getValue(meta) != 3;
}

 

I can live without using this method, but I'm curious as to why the wrong position would be getting passed... Perhaps a vanilla bug?

[Earthcomputer]

I don't know why the wrong block is being passed to your method, but you could possibly solve your problem by using an instanceof check:

if (!(w.getBlockState(pos).getBlock() instanceof BlockYourAir))
   return false; // Not too sure what to return here, but at least it will stop the crash

[FLUFFY2]

"return (Integer)w.getBlockState(pos).getValue(meta) != 3;"

 

Now, where the hell you check, if the block is air?

There is a builtin method for that!

 

return w.isAirBlock(pos);

 

 

[Choonster]

"return (Integer)w.getBlockState(pos).getValue(meta) != 3;"

 

Now, where the hell you check, if the block is air?

There is a builtin method for that!

 

return w.isAirBlock(pos);

 

The OP is overriding

Block#isAir

, which is what

World#isAirBlock

uses.

[RANKSHANK]

I don't know why the wrong block is being passed to your method, but you could possibly solve your problem by using an instanceof check:

if (!(w.getBlockState(pos).getBlock() instanceof BlockYourAir))
   return false; // Not too sure what to return here, but at least it will stop the crash

Yup it's easily worked around with an instance check.

 

 

It could help if you .... idk, posted the full stacktrace? That's what the stacktrace does, it tells you where something was called from, so that you can look at that "where" and find out why it's broken.

Sorry! Stacktrace:

at net.minecraft.block.state.BlockState$StateImplementation.getValue(BlockState.java:167)
at rankshank.pikmine.block.BlockOnionAir.isAir(BlockOnionAir.java:86)
at net.minecraft.world.World.playMoodSoundAndCheckLight(World.java:2835)
at net.minecraft.world.WorldServer.updateBlocks(WorldServer.java:394)
at net.minecraft.world.WorldServer.tick(WorldServer.java:228)

 

So it happens in World in the final line here

Block block = p_147467_3_.getBlock(blockpos); //queiries the chunk for
            l += p_147467_1_;
            i1 += p_147467_2_;

            if (block.isAir(this, blockpos) && this.getLight(blockpos) <= this.rand.nextInt( && this.getLightFor(EnumSkyBlock.SKY, blockpos) <= 0)

Which makes no sense since it's grabbing the block at the position and the calling its isAir.

 

I've confirmed this happens when any other block class overrides, and it's always when the light tables are being updated as well.

 

I'm guessing now reading through a bit more that the Chunk class is failing a sanity check in getBlock0

if (extendedblockstorage != null)
      try
          {
                 block = extendedblockstorage.getBlockByExtId(x, y & 15, z);
          }

If that extended Block storage is null it passes air as the block

[RANKSHANK]

You should put a breakpoint in that method and check what is actually happening.

Without any in built else statements there it's... a bit more than painful running a breakpoint (actually managed to crash because it's called so many times a tick and takes a few minutes for a failure to occur). The work around  is pretty simple and not heavy enough to call to warrant using ASM to add an else{ just to add a  breakpoint... I'll just toss it out as one of the many 'unique' characteristics in the lighting engine

[jeffryfisher]

Indeed there is at least one vanilla bug (I know of one client-side) that makes some calls to blocks during the breaking of a block but *after* the block has been replaced by air in the world. If any method in the resulting stack tries to get its block's properties (vanilla ones don't happen to do so), then an exception is raised.

 

It is possible that the call to isAir() is trying to protect some other code from just this problem, and your override using state info raised the very exception that the caller was trying to avoid.

 

As you found, you must preface the getProperties call with a check for actual air (instanceof) and then return true (or see what block is really there and call isAir on it if it's not yours). Several of us have had to do likewise in custom getHardness methods in blocks with subtypes having variable hardness.

[RANKSHANK]

.

Indeed there is at least one vanilla bug (I know of one client-side) that makes some calls to blocks during the breaking of a block but *after* the block has been replaced by air in the world. If any method in the resulting stack tries to get its block's properties (vanilla ones don't happen to do so), then an exception is raised.

 

It is possible that the call to isAir() is trying to protect some other code from just this problem, and your override using state info raised the very exception that the caller was trying to avoid.

 

Well my crashes happen on the WorldServer#tick() with none of my blocks being broken (I leave the game running to go grab a beer and will return to a crash log). So the block at the given location is persistent. Mine seems to have to do with the Chunk#extendedblockstorage being null during the light table update... which I'd wager has something to do with chunkloading / chunksaving setting the block storage as temporarily unavailable... but I wonder if there are any ramifications for a block being incorrectly considered as air (lighting glitches being a major possibility).

[ShetiPhian]

I was getting random WorldServer#tick() crashes on the blocks I had overridden getLightOpacity on and used the blockstate to set the value. (now I check if the block is mine first)

 

The issue is in Chunk.setBlockState(BlockPos, IBlockState)

this line: int j1 = block.getLightOpacity(this.worldObj, pos);

 

block is the new block, but at that point in time the old block is at that location.

The next line sets the world position to the new block.

 

j1 appears to be used to store the old light value to later compare to the new one, and if they are different the lighting gets updated.

 

I believe that line should be: int j1 = block1.getLightOpacity(this.worldObj, pos);

[Earthcomputer]

Going back to my previous answer about the instanceof check, instead of returning false, you could return

IBlockState state = w.getBlockState(pos);
return state.getBlock().isAir(w, state);

[LexManos]

Or, once again you guys could you know, REPORT THE ISSUE.

Seems vanilla has a bug where its passing chunk coords instead of world position like it should.

Taking 10 seconds to look at the code which is calling that function in the stack trace would see the issue.

Seriously...

https://github.com/MinecraftForge/MinecraftForge/commit/1246f1a79119959039f68b2fcdceadfc320f3318#diff-0facadd57a1c485eed926033d315a5f0R594

 

[RANKSHANK]

Or, once again you guys could you know, REPORT THE ISSUE.

Seems vanilla has a bug where its passing chunk coords instead of world position like it should.

Off to update my environment then, thanks for the fix Lex!

Just saying, you can make breakpoints that don't suspend the threads. You can make breakpoints that only suspend on a certain condition. You can make breakpoints that merely log something to the console. Etc. Etc.

Thanks Diesi! Looks like I've got some reading to do