From c55f34780fa3ad5e2408ada7b4560ae925eb7a2d Mon Sep 17 00:00:00 2001 From: Rbanh Date: Mon, 13 Jul 2026 00:43:10 -0400 Subject: [PATCH] Fix live operator UI regressions --- crates/editor/src/ext/extensibility.rs | 36 +++++++++++++++---- crates/editor/src/history/mod.rs | 16 +++++++-- .../evaluations/operator-invariants/README.md | 15 ++++++-- docs/editor/operator-regression-testing.md | 4 +-- 4 files changed, 58 insertions(+), 13 deletions(-) diff --git a/crates/editor/src/ext/extensibility.rs b/crates/editor/src/ext/extensibility.rs index dac3996..1828e73 100644 --- a/crates/editor/src/ext/extensibility.rs +++ b/crates/editor/src/ext/extensibility.rs @@ -563,6 +563,7 @@ fn command_palette_ui( } let mut open = palette.open; + let mut dismiss_requested = false; let palette_width = (ctx.content_rect().width() - 32.0).clamp(320.0, 520.0); egui::Window::new("Command Palette") .open(&mut open) @@ -611,13 +612,11 @@ fn command_palette_ui( if ui.input_mut(|input| input.consume_key(egui::Modifiers::NONE, egui::Key::Enter)) { if let Some(entry) = filtered_entries.get(palette.selected_index) { palette.pending_run = Some(entry.name.clone()); - palette.open = false; - ui.close(); + dismiss_requested = true; } } if ui.input_mut(|input| input.consume_key(egui::Modifiers::NONE, egui::Key::Escape)) { - palette.open = false; - ui.close(); + dismiss_requested = true; } ui.separator(); @@ -639,17 +638,20 @@ fn command_palette_ui( if response.clicked() { palette.selected_index = index; palette.pending_run = Some(entry.name.clone()); - palette.open = false; - ui.close(); + dismiss_requested = true; } } }); }); - palette.open = open; + palette.open = resolve_palette_open(open, dismiss_requested); Ok(()) } +fn resolve_palette_open(window_open: bool, dismiss_requested: bool) -> bool { + window_open && !dismiss_requested +} + fn filtered_command_entries( registry: &EditorCommandRegistry, filter: &str, @@ -766,6 +768,13 @@ mod tests { assert_eq!(entries[1].name, "scene.reset_lighting"); } + #[test] + fn command_palette_dismissal_wins_over_stale_window_open_state() { + assert!(!resolve_palette_open(true, true)); + assert!(!resolve_palette_open(false, false)); + assert!(resolve_palette_open(true, false)); + } + #[test] fn grouping_dispatch_is_one_history_transaction_and_round_trips() { let mut world = command_world(); @@ -824,10 +833,12 @@ mod tests { assert!(committed .iter() .any(|(name, parent, _)| name == "Second" && parent.as_deref() == Some("Group"))); + assert_group_has_visibility_hierarchy(&mut world); apply_command_undo(&mut world); assert_eq!(hierarchy_projection(&mut world), initial); apply_command_redo(&mut world); assert_eq!(hierarchy_projection(&mut world), committed); + assert_group_has_visibility_hierarchy(&mut world); assert!(world.get::(unselected).is_none()); apply_command_undo(&mut world); assert_eq!(hierarchy_projection(&mut world), initial); @@ -836,6 +847,17 @@ mod tests { assert!(world.get::(unselected).is_none()); } + fn assert_group_has_visibility_hierarchy(world: &mut World) { + let group = world + .query::<(Entity, &Name)>() + .iter(world) + .find_map(|(entity, name)| (name.as_str() == "Group").then_some(entity)) + .expect("group command should spawn the group actor"); + assert!(world.get::(group).is_some()); + assert!(world.get::(group).is_some()); + assert!(world.get::(group).is_some()); + } + #[test] fn reset_lighting_dispatch_is_one_undoable_group() { let mut world = command_world(); diff --git a/crates/editor/src/history/mod.rs b/crates/editor/src/history/mod.rs index 8d5e836..03ff134 100644 --- a/crates/editor/src/history/mod.rs +++ b/crates/editor/src/history/mod.rs @@ -198,7 +198,20 @@ fn spawn_snapshot_with_parent( snapshot: &EditorEntitySnapshot, parent: Option, ) -> Entity { - let mut entity_mut = world.spawn((LevelObject, snapshot.actor_kind, snapshot.transform)); + let visibility = if snapshot.editor_visibility.visible { + Visibility::Visible + } else { + Visibility::Hidden + }; + let mut entity_mut = world.spawn(( + LevelObject, + snapshot.actor_kind, + snapshot.transform, + snapshot.editor_visibility, + visibility, + InheritedVisibility::default(), + ViewVisibility::default(), + )); entity_mut.insert( snapshot .actor_id @@ -291,7 +304,6 @@ fn spawn_snapshot_with_parent( entity_mut.insert(link.clone()); } entity_mut.insert(HierarchySiblingIndex(snapshot.hierarchy_sibling_index)); - entity_mut.insert(snapshot.editor_visibility); if let Some(order) = &snapshot.inspector_order { entity_mut.insert(order.clone()); } diff --git a/docs/editor/evaluations/operator-invariants/README.md b/docs/editor/evaluations/operator-invariants/README.md index d6c7871..1899283 100644 --- a/docs/editor/evaluations/operator-invariants/README.md +++ b/docs/editor/evaluations/operator-invariants/README.md @@ -11,9 +11,11 @@ release candidate or replace the clean-checkout rerun required by production-rea ## Accepted Coverage - Registered commands distinguish immediate completion from modal Preview ownership; validation - failures terminate with their stable command ID. + failures terminate with their stable command ID. Palette execution and cancellation fully dismiss + the palette instead of leaving an empty title bar. - Group Selection is one history transaction and preserves unrelated hierarchy rows through repeated - undo/redo even though the transient group entity is respawned. + undo/redo even though the transient group entity is respawned. Newly spawned and redone group + actors own their visibility hierarchy before viewport icon children attach. - Reset Lighting and Project Sun changes are grouped, undoable light transactions. - Asset/sub-asset placement; material/texture/audio/animation assignment; and exact viewport material drops cover commit, block/failure, cancel where applicable, and semantic undo/redo projections. @@ -39,6 +41,15 @@ The publication worktree passed: The Gitea closure comment records these results with the exact publication commit. +## Native Editor Follow-up + +A native-editor replay on 2026-07-13 loaded the collider diagnostics scene, exercised Draw Brush +Preview/cancel, and grouped two hierarchy actors through undo/redo. The replay exposed two issues +that source-only checks did not make visible: command execution left a collapsed palette title bar, +and the viewport icon child briefly inherited from a group actor without Bevy visibility hierarchy +components. Both paths now have focused regression assertions; a rebuilt replay dismissed the +palette completely and emitted no hierarchy warning. + No screenshot is required for this ticket: the accepted surface is lifecycle, rollback, cleanup, and history behavior exercised through headless production entry points. Live command/status checks add confidence but are not substituted for semantic assertions. Packaged-runtime testing remains diff --git a/docs/editor/operator-regression-testing.md b/docs/editor/operator-regression-testing.md index 7b88c6e..600ebcc 100644 --- a/docs/editor/operator-regression-testing.md +++ b/docs/editor/operator-regression-testing.md @@ -43,8 +43,8 @@ The test must mutate a world and prove restoration. | Workflow | Coverage | |----------|----------| | Generic `EditorOperator` | Commit cleanup, preview/commit failure rollback, blocked no-op, terminal ID/phase | -| Palette and registered commands | Typed immediate commit versus modal preview ownership; failed CSG start terminates | -| Selection and scene commands | Atomic Group Selection across repeated undo/redo; grouped Reset Lighting and Project Sun history | +| Palette and registered commands | Typed immediate commit versus modal preview ownership; Enter/click/Escape dismissal cannot be overwritten by stale window state; failed CSG start terminates | +| Selection and scene commands | Atomic Group Selection across repeated undo/redo; spawned group actors have an immediate Bevy visibility hierarchy; grouped Reset Lighting and Project Sun history | | Asset workflows | Asset/sub-asset placement; material/texture group assignment; audio/animation assignment and incompatible targets | | Viewport material drop | Exact renderer slot/primitive/brush-face preview, cancel, commit, cleanup, undo, and redo | | Draw Brush and CSG | Modal cancel; decomposed commit; CSG validation/read-only block, cancel, grouped commit, and deleted-brush restoration |