Skip to content

Refactor how block entity unloading works and unloading things on the client side#1475

Merged
IntegratedQuantum merged 2 commits into
masterfrom
blockEntity_better_unload
May 21, 2025
Merged

Refactor how block entity unloading works and unloading things on the client side#1475
IntegratedQuantum merged 2 commits into
masterfrom
blockEntity_better_unload

Conversation

@IntegratedQuantum

Copy link
Copy Markdown
Member

extracted from #1446

@Argmaster Argmaster left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume it was tested manually as much as possible (so not much at all). I am curious where you will get and how will the complete system work.
Although I have impression that you are consistently getting closer to looking just like another ECS component.

PS. Honestly, for me this is too small PR - I don't get the bigger picture, so all I can catch is micro-mistakes and I see none of those atm.

Comment thread src/block_entity.zig
pub fn onLoadClient(_: Vec3i, _: *Chunk) void {}
pub fn onUnloadClient(_: Vec3i, _: *Chunk) void {}
pub fn onUnloadClient(_: BlockEntityIndex) void {}
pub fn onLoadServer(_: Vec3i, _: *Chunk) void {}

@Argmaster Argmaster May 21, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why inconsistent interface? Will it be changed in the future?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loading needs to know where it is, in case the block entity makes use of the position (e.g. to render something at the position, or maybe spawn particles or whatever), in which it has to store it in storage
Furthermore the chunk is needed because we need to add the index to the chunk hashmap.

@IntegratedQuantum

Copy link
Copy Markdown
Member Author

I assume it was tested manually as much as possible (so not much at all).

Well I tested the original code in the signs command, and this is basically the same with same small refactoring changes from the earlier PRs.

@IntegratedQuantum

Copy link
Copy Markdown
Member Author

Although I have impression that you are consistently getting closer to looking just like another ECS component.

Well it kind of is similar, just the interface is going to be a little different, and of course we don't want to flood the nice and tight u16 entity index space with potentially even billions of block entities.

@IntegratedQuantum

Copy link
Copy Markdown
Member Author

PS. Honestly, for me this is too small PR - I don't get the bigger picture, so all I can catch is micro-mistakes and I see none of those atm.

Alright, then I guess I'll just move everything else into the signs PR for now.

@IntegratedQuantum IntegratedQuantum merged commit c852393 into master May 21, 2025
1 check passed
@IntegratedQuantum IntegratedQuantum deleted the blockEntity_better_unload branch June 7, 2025 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants