Safe getTileBlock

[dvantwisk]

I'm writing a renderer that has to repeatedly access the getTileBlock() method in Maps.cpp. I've had my debugger show crashes here before at the return statement:

return world->map.block_index[x >> 4][y >> 4][z];

It seems that it is trying to access an invalid memory address even though the coordinates are valid. This indicates sometimes dfhack can hit the map_block data before it has updated, causing the crash. Adding a check for null pointers seems to have resolved the crashes on my end. Let me know if this is a prudent change or if I ought to be doing something else when having to make many calls to getTileBlock().

[quietust]

Under normal circumstances, world->map.block_index, world->map.block_index[x], and world->map.block_index[x][y] should never be NULL as long as x/y are valid, but world->map.block_index[x][y][z] can definitely be NULL if a particular block happens to be "unallocated"*. If you're ever seeing intermediate NULL pointers, it's possible something managed to run while DF was in the process of rebuilding block_index (which I'm not sure how that would happen, unless you were walking around in Adventurer mode).

(* - though such unallocated blocks seem to be exceedingly uncommon in the current version - back in 0.47.05 and earlier, blocks up in the Sky were often unallocated, and way back in 0.28.181.40d and earlier, nearly all underground blocks were unallocated until you placed a designation inside them).

[dvantwisk]

The custom renderer I'm working on makes many calls to getTileBlock (although I'm rewriting a few parts to cut back on it). You're right that the situation that the block is unallocated is rare, however, I'd think the check is still prudent.

[dvantwisk]

@myk002
I guess this pull request has been up for a while. Is the problem I mentioned here worth trying to fix and is the change I made worth adding?

[ab9rf]

I just reviewed DF's own code for interacting with block_index and while there are a couple spots that check for block_index being NULL, DF never checks for the intermediate indices being NULL except in the function to delete block_index as a whole. I would conclude that if one of the intermediate indices is NULL, DF itself will likely crash, and so we're not really gaining anything here by doing this.

I would not expect these indices to ever be invalid in fortress mode as the map is loaded at load and remains loaded throughouot. In adventure mode, maps are loaded and unloaded as the player moves around the world. If you attempt to access the block_index while in adventure mode without first obtaining a CoreSuspender, it's possible that the game will be in the process of loading or unloading the map when your access occurs, which could result in inconsistent results.

At this point I'm not convinced that there is a problem to fix and would suggest that you share the code that is having problems so we can determine that there's not in fact an XY problem here.