Be sure to validate signals before committing the results, since GMNS imports aren't validated upfront. #626

game/src/edit/traffic_signals/mod.rs

@@ -1,5 +1,7 @@
 use std::collections::{BTreeSet, VecDeque};
 
+use anyhow::Result;
+
 use abstutil::Timer;
 use geom::{Distance, Line, Polygon, Pt2D};
 use map_gui::options::TrafficSignalStyle;
@@ -175,6 +177,14 @@ impl TrafficSignalEditor {
         self.draw_current = ctx.upload(batch);
         self.movements = movements;
     }
+
+    // We may have imported the signal configuration without validating it.
+    fn validate_all_members(&self, app: &App) -> Result<()> {
+        for i in &self.members {
+            app.primary.map.get_traffic_signal(*i).validate()?;
+        }
+        Ok(())
+    }
 }
 
 impl State<App> for TrafficSignalEditor {
@@ -299,6 +309,15 @@ impl State<App> for TrafficSignalEditor {
                                 changes to include them.",
                             ],
                         ));
+                    } else if let Err(err) = self.validate_all_members(app) {
+                        // TODO There's some crash between usvg and lyon trying to tesellate the
+                        // error text!
+                        error!("{}", err);
+                        return Transition::Push(PopupMsg::new_state(
+                            ctx,
+                            "Error",
+                            vec!["This signal configuration is somehow invalid; check the console logs"]
+                        ));
                     } else {
                         let changes = BundleEdits::get_current(app, &self.members);
                         self.original.apply(app);
@@ -320,6 +339,15 @@ impl State<App> for TrafficSignalEditor {
                     ));
                 }
                 "Edit multiple signals" => {
+                    if let Err(err) = self.validate_all_members(app) {
+                        error!("{}", err);
+                        return Transition::Push(PopupMsg::new_state(
+                            ctx,
+                            "Error",
+                            vec!["This signal configuration is somehow invalid; check the console logs"]
+                        ));
+                    }
+
                     // First commit the current changes, so we enter SignalPicker with clean state.
                     // This UX flow is a little unintuitive.
                     let changes = check_for_missing_turns(app, &self.members)

map_model/src/objects/traffic_signals.rs

@@ -104,7 +104,7 @@ impl ControlTrafficSignal {
         Duration::seconds(time.inner_seconds().ceil())
     }
 
-    pub(crate) fn validate(&self) -> Result<()> {
+    pub fn validate(&self) -> Result<()> {
         // Does the assignment cover the correct set of movements?
         let expected_movements: BTreeSet<MovementID> = self.movements.keys().cloned().collect();
         let mut actual_movements: BTreeSet<MovementID> = BTreeSet::new();