Use a SparseSet in BlockEntity data, instead of manually managing a dense list. #1470
Conversation
| return; | ||
| } | ||
|
|
||
| const movedEntry = storage.items[dataIndex]; |
There was a problem hiding this comment.
Okay, cool, so what's the plan for unloading block entities when chunk they come from gets unloaded? Do you just iterate all the block entities whenever a chunk is unloaded?
There was a problem hiding this comment.
I'd just iterate over the chunk hashmap
There was a problem hiding this comment.
Alright I guess, why did I remove it the way I did...?
Was it because there is no direct external access for deleting? So caller will have to explicitly call proper callback and separately clear hash map entry.
There was a problem hiding this comment.
Alright I guess, why did I remove it the way I did...?
To keep the list sparse, which is no longer needed due to using the SparseSet.
src/block_entity.zig
Outdated
| const mesh_storage = main.renderer.mesh_storage; | ||
|
|
||
| pub const BlockEntityIndex = u32; | ||
| pub const BlockEntityIndex = enum(u32) { |
There was a problem hiding this comment.
this should just be
pub const BlockEntityIndex = main.utils.DenseId(u32);There was a problem hiding this comment.
Btw it was a very poor choice of name I suggested, since this is an ID used to index sparse array in many contexts (xD)
There was a problem hiding this comment.
Btw it was a very poor choice of name I suggested
Please suggest a better name in a PR then, or make an issue for it.
There was a problem hiding this comment.
Sure, after this PR gets merged
Argmaster
left a comment
Argmaster
left a comment
There was a problem hiding this comment.
I think that part is mostly fine, but since BEs are no longer cleaning up after themselves shouldn't you add cleanup code next to onBreakClient and alike callbacks that add and remove BEs to chunk hash maps? Some of them are already called in code.
I don't really understand the problem. |
Hmm I see, alr then its all fine ig. |
|
Apparently my brain already garbage collected my understanding of block entity system :3 |
fixes #1453