From d9e52d753b3a9422d3f3ba1c35987a17b044e586 Mon Sep 17 00:00:00 2001
From: Rbanh <Rbanh15@gmail.com>
Date: Sat, 6 Jun 2026 06:25:43 -0400
Subject: [PATCH] Reject invalid brush element edits

---
 crates/editor/src/viewport/brush_edit.rs | 65 ++++++++++++++++++------
 docs/editor/brushes.md                   |  2 +-
 2 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/crates/editor/src/viewport/brush_edit.rs b/crates/editor/src/viewport/brush_edit.rs
index d396d39..73c2488 100644
--- a/crates/editor/src/viewport/brush_edit.rs
+++ b/crates/editor/src/viewport/brush_edit.rs
@@ -2,7 +2,10 @@
 
 use bevy::prelude::*;
 use bevy_egui::{egui, EguiContexts};
-use shared::{BrushDesc, BrushFaceDesc, BrushPlaneDesc, ComponentInstanceId};
+use shared::{
+    brush_math::{validate_brush, BrushDiagnosticSeverity},
+    BrushDesc, BrushFaceDesc, BrushPlaneDesc, ComponentInstanceId,
+};
 
 use crate::camera::EditorCamera;
 use crate::history::{push_command, EditorCommand};
@@ -297,23 +300,41 @@ fn brush_element_drag(
     if buttons.just_released(MouseButton::Left) {
         if let (Some(brush), Some(old)) = (drag.brush, drag.old.take()) {
             if drag.changed {
-                if let Ok((brush_desc, _)) = brushes.get(brush) {
+                if let Ok((mut brush_desc, _)) = brushes.get_mut(brush) {
                     let new = brush_desc.clone();
-                    commands.queue(move |world: &mut World| {
-                        push_command(
-                            world,
-                            EditorCommand::SetBrush {
-                                entity: brush,
-                                old: Some(old),
-                                new,
-                            },
+                    let validation = validate_brush(&new);
+                    if validation.is_valid() {
+                        commands.queue(move |world: &mut World| {
+                            push_command(
+                                world,
+                                EditorCommand::SetBrush {
+                                    entity: brush,
+                                    old: Some(old),
+                                    new,
+                                },
+                            );
+                        });
+                        set_brush_edit_status(
+                            &mut active_operator,
+                            OperatorPhase::Committed,
+                            "Committed brush element edit",
                         );
-                    });
-                    set_brush_edit_status(
-                        &mut active_operator,
-                        OperatorPhase::Committed,
-                        "Committed brush element edit",
-                    );
+                    } else {
+                        *brush_desc = old;
+                        let message = validation
+                            .diagnostics
+                            .iter()
+                            .find(|diagnostic| {
+                                diagnostic.severity == BrushDiagnosticSeverity::Error
+                            })
+                            .map(|diagnostic| diagnostic.message.as_str())
+                            .unwrap_or("Brush edit produced invalid geometry");
+                        set_brush_edit_status(
+                            &mut active_operator,
+                            OperatorPhase::Blocked,
+                            format!("Rejected brush edit: {message}"),
+                        );
+                    }
                 }
             }
         }
@@ -783,4 +804,16 @@ mod tests {
         assert!(changed);
         assert_eq!(before, after);
     }
+
+    #[test]
+    fn collapsed_face_move_produces_invalid_brush() {
+        let mut brush = BrushDesc::default();
+        let face = brush.faces[0].id.clone();
+        assert!(move_selected_elements(
+            &mut brush,
+            &[BrushElementKey::Face { face }],
+            Vec3::new(-1.0, 0.0, 0.0),
+        ));
+        assert!(!validate_brush(&brush).is_valid());
+    }
 }
diff --git a/docs/editor/brushes.md b/docs/editor/brushes.md
index 7614b97..28ab47f 100644
--- a/docs/editor/brushes.md
+++ b/docs/editor/brushes.md
@@ -17,7 +17,7 @@ Brushes are persisted blockout geometry stored as `ActorKind::Brush + BrushDesc`
 
 With a brush actor selected, `1`/`2`/`3`/`4` enter vertex, edge, face, and clip modes. Element modes show viewport overlays for the selected brush, route LMB to element picking, support Shift+LMB multi-select, and display a `Brush: <mode>` badge. `Esc` returns to object mode.
 
-Vertex, edge, and face selections can be dragged on the viewport floor plane. The editor moves matching duplicated corner vertices across all brush faces so simple cube/prism brushes stay welded, recomputes face planes, and commits one undoable brush edit when the drag releases. Clip mode currently provides element visualization/selection only; split application is future CSG work.
+Vertex, edge, and face selections can be dragged on the viewport floor plane. The editor moves matching duplicated corner vertices across all brush faces so simple cube/prism brushes stay welded, recomputes face planes, validates the result, and commits one undoable brush edit when the drag releases. Invalid drag results are rejected with operator status text and reverted to the pre-drag brush. Clip mode currently provides element visualization/selection only; split application is future CSG work.
 
 When one or more faces are selected in Face mode, the Brush inspector shows selected-face controls for UV offset, UV scale, UV rotation, material ref, and texture ref. Material and texture rows use Asset Browser-aware picker controls with clear, locate, browse, and compatible drag/drop assignment. These fields edit the persisted `BrushFaceDesc` data through undoable brush updates. Per-face material batching in hydration is still follow-up work.