Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix inconsistencies with margin reconciliation logic #386

Merged
merged 4 commits into from
Apr 30, 2024

Conversation

patowen
Copy link
Collaborator

@patowen patowen commented Apr 30, 2024

This PR fixes two bugs related to #379.

Bug 1

Some of the logic assumed that world generation would never create "solid" chunks unless it was sure that the chunk was deep enough underground or high enough up in the air to not touch any surfaces. This assumption allowed us to avoid reconciling margins between a solid chunk and a dense chunk unless the dense chunk was "modified".

However, since this assumption was incorrect (for instance, due to world generation not turning solid "void" chunks into dense chunks unless a voxel is actually generated), this produced gaps in the terrain whenever a dense chunk met a solid void chunk.

Bug 2

When a voxel is modified, we assumed that only neighboring chunks' margins needs to update, since all other margins should already be correct. However, if we want to be certain, we also need to update the margins of the chunk the modified voxel is in. This is because we only guarantee that margin voxels are the correct material when they would be rendered.

Because of this, it would have been possible to dig down into a solid dirt chunk, dig across a bit, and dig up again, seeing what looks like dirt in a neighboring chunk's voxel no matter what material that voxel actually is.

Solution

The fix to bug 1 is to remove the assumption, adding logic to fix_margins to handle either chunk being solid.

The fix to bug 2 is to change update_margin_voxel into a bidirectional update, updating the margins of both relevant chunks. Since this changes the public interface of the function, I also renamed it to reconcile_margin_voxels.

Bonus cleanup

The only purpose of the modified boolean in Chunk::Populated was to determine whether assumptions based on world generation would hold when determining whether chunks should be solid or not. Since we no longer make assumptions based on world generation, this field is now obsolete, allowing us to remove it completely.

@patowen patowen requested a review from Ralith April 30, 2024 03:12
common/src/margins.rs Outdated Show resolved Hide resolved
common/src/node.rs Show resolved Hide resolved
common/src/margins.rs Outdated Show resolved Hide resolved
@patowen
Copy link
Collaborator Author

patowen commented Apr 30, 2024

I believe the extra changes I made to this PR are noncontroversial enough, so I'll go ahead and merge when CI passes.

@patowen patowen merged commit 7c2bd56 into Ralith:master Apr 30, 2024
6 checks passed
@patowen patowen deleted the margins-bugfix branch April 30, 2024 05:48
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.

None yet

2 participants