Skip to content

Commit

Permalink
Don't update VoxelInstancer if its library changes while not in the t…
Browse files Browse the repository at this point in the history
…ree.

This notably covers the case where the user deletes the instancer in the
editor and edits the library afterwards.
This behavior is intented for common editor situations, so for now I'm
not handling other edge cases.
  • Loading branch information
Zylann committed Dec 20, 2024
1 parent 1f5f2a4 commit 9629cb6
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 4 deletions.
4 changes: 3 additions & 1 deletion doc/source/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ Primarily developped with Godot 4.3.
- Fixes
- Fixed potential deadlock when using detail rendering and various editing features (thanks to lenesxy, issue #693)
- `VoxelInstanceLibrary`: Editor: reworked the way items are exposed as a Blender-style list. Now removing an item while the library is open as a sub-inspector is no longer problematic
- `VoxelInstancer`: Fixed persistent instances reloading with wrong positions (in the air, underground...) when mesh block size is set to 32
- `VoxelInstancer`:
- Fixed persistent instances reloading with wrong positions (in the air, underground...) when mesh block size is set to 32
- Editor: fixed `!is_inside_world()` errors when editing a `VoxelBlockyLibrary` after deleting a `VoxelInstancer` that was using it
- `VoxelLodTerrain`:
- Fixed potential crash when when using the Clipbox streaming system with threaded update (thanks to lenesxy, issue #692)
- Fixed blocks were saved with incorrect LOD index when they get unloaded using Clipbox, leading to holes and mismatched terrain (#691)
Expand Down
23 changes: 20 additions & 3 deletions terrain/instancing/voxel_instancer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,18 @@ void VoxelInstancer::on_library_item_changed(int item_id, IInstanceLibraryItemLi
Ref<VoxelInstanceLibraryItem> item = _library->get_item(item_id);
ERR_FAIL_COND(item.is_null());
add_layer(item_id, item->get_lod_index());
regenerate_layer(item_id, true);
// In the editor, if you delete a VoxelInstancer, Godot doesn't actually delete it. Instead, it removes it
// from the scene tree and keeps it around in the UndoRedo history. But the node still receives
// notifications when the library gets modified... this leads to several issues:
// - Errors because the node needs to have access to World3D to update
// - In theory we could not require World3D, but then it still means a lot of processing has to occur to
// re-generate layers, which is wasted CPU for a node that isn't active or is "currently" deleted by the
// user.
// So we stop it from re-generating layers while in that state. I'm not sure to which extent we should
// be supporting out-of-tree automatic refresh... There might be more corner cases than this.
if (is_inside_tree()) {
regenerate_layer(item_id, true);
}
update_configuration_warnings();
} break;

Expand All @@ -921,7 +932,10 @@ void VoxelInstancer::on_library_item_changed(int item_id, IInstanceLibraryItemLi
break;

case IInstanceLibraryItemListener::CHANGE_GENERATOR:
regenerate_layer(item_id, false);
// Don't update in case the node was deleted in the editor...
if (is_inside_tree()) {
regenerate_layer(item_id, false);
}
break;

case IInstanceLibraryItemListener::CHANGE_VISUAL:
Expand All @@ -948,7 +962,10 @@ void VoxelInstancer::on_library_item_changed(int item_id, IInstanceLibraryItemLi
Lod &new_lod = _lods[layer.lod_index];
new_lod.layers.push_back(item_id);

regenerate_layer(item_id, true);
// Don't update in case the node was deleted in the editor...
if (is_inside_tree()) {
regenerate_layer(item_id, true);
}
} break;

default:
Expand Down

0 comments on commit 9629cb6

Please sign in to comment.