PneumaticCraft: Repressurized

PneumaticCraft: Repressurized

50M Downloads

Pressure Chamber wall & glass blocks have ticking tile entities

desht opened this issue ยท 4 comments

commented

This isn't great from a performance point of view. There's no need in theory for these to tick, but it will take some non-trivial rearrangement of the class hierarchy to achieve this.

(Ideally these don't even need to be tile entities, but that needs more investigation...)

commented

The reason of them being TileEntities is mainly to keep track of the master TE (the valve). That way, when the block is removed, it can invalidate the multiblock itself. This prevents having the valve to once in a while go through all the blocks to check if the multiblock still is OK.

I agree with the not needing to tick part. They might use the 'onFirstServerUpdate' part which is called from the update method. That by itself is something to revisit, I know at the time I required it, because the built in method designed for this (TileEntity#onLoad()) was not suitable (crashing stuff on world load IIRC). It might work better nowadays though.

commented

The proposed type hierarchy looks great.

About TileEntity#onLoad, I think the problem was that onLoad was called as soon as the TileEntity was added to the world. This previously caused issues on world/chunk load, as at the point a Pressure Chamber Wall for example gets loaded, that doesn't mean the corresponding valve was at that point.

However, I've looked at the caller of onLoad, and they now seem to defer calling this method now until all TileEntities have been added already, so I think it should be good now. But this obviously needs testing.

commented

Yeah, it would be difficult without tile entities at all. Non-ticking tile entities much less of an issue (especially since Forge recently fixed a vanilla problem which caused serious lag when unloading a lot of tile entities).

I guess the only reason they tick is because they inherit TileEntityBase, which also ticks. The vast majority of PNC TE's do need to tick, but we could do with a TileEntityNonTickable or similar for structural blocks like the walls & glass. Hence my comment about hierarchy rearrangement. Maybe:

TileEntityBase (doesn't tick)
+- TileEntityTickable
    +- (most machines inherit this)
+- TileEntityPressureChamberWall

And the glass looks pretty nice now that connected textures are working, so it would be cool to be able to use it in builds :)

re: TileEntity#onLoad - IIRC the TE's world & pos are null at that point, which is one reason why you can't do much there. I do see the need for the onFirstServerUpdate method actually that's not the case - the world and pos are available in onLoad, though they may well not have been in earlier versions - see World#addTileEntity. We should revisit this...

commented

This is done now.