Get light from chunk

[mgueydan]

Hello,

I searched how to get chunk enlightenment in order to make mobs spawn right, but founded it nowhere.

Even Sugarcane and Pumpkins seems not to need light to grow.

As you mentioned earlier that it was something tricky (I understood that enlightenment calculation was mad on separated thread), I do prefer to be carefull and ask before doing.

Do you think I could replace

Code:

bool cChunk::UnboundedRelGetBlock(int a_RelX, int a_RelY, int a_RelZ, BLOCKTYPE & a_BlockType, NIBBLETYPE & a_BlockMeta) const

by :

Code:

bool cChunk::UnboundedRelGetBlock(int a_RelX, int a_RelY, int a_RelZ, BLOCKTYPE & a_BlockType, NIBBLETYPE & a_BlockMeta, NIBBLETYPE & a_BlockLight, NIBBLETYPE & a_BlockSkyLight) const

?

Thanks

[xoft]

I don't think that's the right way around this. We have a "GetBlock" function that retrieves the block type and meta (which is what really comprises the block). the light doesn't comprise a block, it's something external. I'd recommend making an extra "UnboundedRelGetBlockBlockLight()" and "UnboundedRelGetBlockSkyLight()" functions for retrieving the light.
If, for performance reason, you believe getting the type, meta and light all together would be beneficial, then make another function, "UnboundedRelGetBlockFull()".

Also keep in mind that these functions make it into the Lua API.

[mgueydan]

I see what you mean,

In order to achieve your proposal, and as I'm not a fan of copy-paste, I propose a refactoring of this part of cChunk (and cChunkMap in fact) that would use polymorphism to include all those methods inside a single one.

You'll find this proposition soon on another pull-request.

I used another branch because the refactoring is not related to mob spawning, and could be used to grown pumpkin or sugar cane event if mob spawning is not accepted.

[xoft]

Polymorphism? Could you sketch how? I'm really intrigued.

[mgueydan]

It's not a big deal :
- BlockDataReader is proposing a pure virtual read(Chunk&, BlockId) method, that is implemented by BlockTypeReader, BlockMetaReader, BlockLightReader, BlockDayLightReader.... whatever you want
- Chunk::UnBoundGetXXX receive a BlockDataReader& and call it's read method (not knowing what is read).
- Finally the value can be retrieved from the BlockDataReader object.

Additionally, I made a Composite of BlockDataReader, that allow to pass multiple reader to the same method call.

Edit 2 : Additionally, I add a templated value-storing intermediate class.

As you responded on GitHub, I assume You've already red it anyway.
Thanks

Edit : I often encourage my teams to work that way. Very short classes with very few and very short public methods. Each one having a small (and so easy to fulfill) responsibility. The may picture being drawn by assembling those small parts. Of course, we cannot do that here because :
1. the history is strong, there would be a lot to do
2. what we get in terms of maintainability, we may lose it (or some) in terms performances.

[xoft]

I thought you were just starting to write this code, so I thought I'd ask for a preview before all the work
Seems reasonable from the maintainability point of view, we'll see about the performance, hopefully soon.