Skip to content

fix(undo): restore deleted entities under their original parent (#75) - #78

Merged
RenzoraEngine merged 1 commit into
renzora:mainfrom
dreamersilly:fix/undo-delete-reparent
Jul 22, 2026
Merged

fix(undo): restore deleted entities under their original parent (#75)#78
RenzoraEngine merged 1 commit into
renzora:mainfrom
dreamersilly:fix/undo-delete-reparent

Conversation

@dreamersilly

Copy link
Copy Markdown
Contributor

Fixes #75.

Both reported symptoms are one bug, and it's in the delete-undo restore path rather than in anything UI-specific.

What happens

snapshot_entity_subtrees deliberately keeps ChildOf — its doc comment says so — "so restore puts each entity back under its original parent". But the parent is outside the snapshot: only the deleted roots and their descendants are captured. So the parent's id is never in the EntityHashMap that write_to_world remaps references through, and SceneEntityMapper::get_mapped maps anything absent to a freshly reserved dead id:

/// Returns the corresponding mapped entity or reserves a new dead entity ID in the current world if it is absent.
fn get_mapped(&mut self, source: Entity) -> Entity {

(bevy_ecs-0.19.0/src/entity/map_entities.rs:273)

The relationship hook then sees a ChildOf pointing at an entity that doesn't exist and discards it. The restored entity has no parent at all — which is symptom 1, hierarchy not respected.

Symptom 2 follows from the same thing. A UI node that loses its ancestor chain is no longer under its canvas, and UiTargetCamera / ComputedUiTargetCamera are denied at snapshot time on the assumption that they get re-derived from that ancestor. So the modal comes back as an independent root node with no target camera and lands wherever the default layout puts it — the box at a random position in the viewport.

Worth noting this was never UI-only. Any nested entity had its parent link dropped on undo; UI just makes it obvious because the misplacement is visible rather than a silent change in the hierarchy panel.

The fix

Identity-map every live entity that isn't itself part of the snapshot before the write, so references pointing outside it resolve to the real entity instead of a dead placeholder. Entities that are in the snapshot are excluded from the seeding, so they still receive fresh ids and the links between them follow the remap rather than the stale originals.

The returned map is narrowed to the snapshot's own old→new pairs afterwards — the identity seeds are scaffolding for the remap, not results. The only caller, DeleteEntitiesCmd::undo, looks up restored roots in it, so it's unaffected.

This also covers any other component holding a reference to a still-live entity outside the snapshot, not just ChildOf.

Verification

The first test fails on current main with exactly the reported behaviour:

assertion `left == right` failed: restored entity must point at the original live parent
  left: None
 right: Some(2v0)

left: None is the whole bug — not the wrong parent, no parent.

Both tests pass with the change. Full suite: 695 passed, 0 failed (693 before, plus these two). Clippy clean on renzora_engine under the workflow's flags.

Tests run with --profile dist; the debug profile can't link on Windows (too many exported symbols (got 878424, max 65535)).

One caveat

CI on this branch will be red, and not because of this change — main has been failing since 2026-06-14. #76 fixes those two causes. This branch is cut from main so it inherits them; it should go green once that lands, or if you'd rather I rebase this on top of #76 to demonstrate that, say the word.

I haven't driven this in a running editor — the verification above is the test, not a repro of the screenshots. If the reporter can confirm the modal comes back in place, that closes the loop.

Undoing a delete dropped the entity's parent link entirely: it came back
at the scene root instead of where it was, and a deleted UI node came
back detached from its canvas — rendering at a default position with no
target camera. Reported in #75.

snapshot_entity_subtrees deliberately keeps ChildOf so the entity restores
under its original parent, but the parent itself is outside the snapshot,
so its id is absent from the map write_to_world remaps through.
SceneEntityMapper maps anything absent to a freshly reserved DEAD id, and
the relationship hook then discards the ChildOf rather than point at a
non-existent entity.

Identity-map every live entity that isn't part of the snapshot before the
write, so external references resolve to the real entity. Entities that
ARE in the snapshot are excluded from the seeding and still get fresh ids,
so links between them follow the remap.

The returned map is now narrowed to the snapshot's own old->new pairs; the
identity seeds are scaffolding for the remap, not results. The sole caller
(DeleteEntitiesCmd::undo) looks up restored roots, so it is unaffected.

Two tests: a child restores under its untouched parent, and a subtree
restores with internal links following the remap while its root reattaches
to the grandparent.
@RenzoraEngine RenzoraEngine reopened this Jul 22, 2026
@RenzoraEngine
RenzoraEngine merged commit dd8816e into renzora:main Jul 22, 2026
2 of 6 checks passed
RenzoraEngine added a commit that referenced this pull request Jul 23, 2026
Undo of a delete dropped the entity's parent link, so a deleted child came
back at the scene root instead of under its parent. spawn_entities_from_snapshot
now seeds the entity map with only the snapshot's own external ChildOf parents
(read off the reflected components) so those links survive write_to_world's
remap, replacing the whole-world iter_entities scan merged in #78. Keeps that
PR's two regression tests.
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.

delete all kind of ui entity and undo redo will result in unexpected outcomes

2 participants