From 4f60d6cb77b23cd5143ce896553412f01ba69b24 Mon Sep 17 00:00:00 2001
From: Dustin Carlino <dabreegster@gmail.com>
Date: Fri, 24 Jan 2020 15:30:32 -0800
Subject: [PATCH] make the all-walk conversion idempotent, and some other
 bugfixes to the editor

---
 game/src/edit/traffic_signals.rs | 18 +++++++++++-------
 map_model/src/traffic_signals.rs | 27 ++++++++++++++++++++-------
 2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/game/src/edit/traffic_signals.rs b/game/src/edit/traffic_signals.rs
index 7c292e90ff..63519980bc 100644
--- a/game/src/edit/traffic_signals.rs
+++ b/game/src/edit/traffic_signals.rs
@@ -94,7 +94,9 @@ impl State for TrafficSignalEditor {
                     self.redo_stack.clear();
                     self.top_panel = make_top_panel(true, false, ctx);
                     change_traffic_signal(new_signal, ui, ctx);
-                    self.change_phase(0, ui, ctx);
+                    // Don't use change_phase; it tries to preserve scroll
+                    self.current_phase = 0;
+                    self.composite = make_diagram(self.i, self.current_phase, ui, ctx);
                     return Transition::Keep;
                 }
                 x if x == "Use preset" => {
@@ -102,12 +104,13 @@ impl State for TrafficSignalEditor {
                 }
                 x if x == "Make all-walk" => {
                     let mut new_signal = orig_signal.clone();
-                    new_signal.convert_to_ped_scramble(&ui.primary.map);
-                    self.command_stack.push(orig_signal.clone());
-                    self.redo_stack.clear();
-                    self.top_panel = make_top_panel(true, false, ctx);
-                    change_traffic_signal(new_signal, ui, ctx);
-                    self.change_phase(0, ui, ctx);
+                    if new_signal.convert_to_ped_scramble(&ui.primary.map) {
+                        self.command_stack.push(orig_signal.clone());
+                        self.redo_stack.clear();
+                        self.top_panel = make_top_panel(true, false, ctx);
+                        change_traffic_signal(new_signal, ui, ctx);
+                        self.change_phase(0, ui, ctx);
+                    }
                     return Transition::Keep;
                 }
                 x if x.starts_with("change duration of #") => {
@@ -227,6 +230,7 @@ impl State for TrafficSignalEditor {
                     self.redo_stack.clear();
                     self.top_panel = make_top_panel(true, false, ctx);
                     change_traffic_signal(new_signal, ui, ctx);
+                    self.change_phase(self.current_phase, ui, ctx);
                     return Transition::Keep;
                 }
             }
diff --git a/map_model/src/traffic_signals.rs b/map_model/src/traffic_signals.rs
index 2c8ed11bff..e7545cc8f6 100644
--- a/map_model/src/traffic_signals.rs
+++ b/map_model/src/traffic_signals.rs
@@ -449,10 +449,26 @@ impl ControlTrafficSignal {
         ts.validate().ok()
     }
 
-    pub fn convert_to_ped_scramble(&mut self, map: &Map) {
+    // Returns true if this did anything
+    pub fn convert_to_ped_scramble(&mut self, map: &Map) -> bool {
+        let orig = self.clone();
+
+        let mut all_walk_phase = Phase::new();
+        for g in self.turn_groups.values() {
+            if g.turn_type == TurnType::Crosswalk {
+                all_walk_phase.edit_group(g, TurnPriority::Protected, &self.turn_groups, map);
+            }
+        }
+
         // Remove Crosswalk groups from existing phases.
         let mut replaced = std::mem::replace(&mut self.phases, Vec::new());
+        let mut has_all_walk = false;
         for phase in replaced.iter_mut() {
+            if !has_all_walk && phase == &all_walk_phase {
+                has_all_walk = true;
+                continue;
+            }
+
             // Crosswalks are only in protected_groups.
             retain_btreeset(&mut phase.protected_groups, |g| {
                 self.turn_groups[g].turn_type != TurnType::Crosswalk
@@ -472,13 +488,10 @@ impl ControlTrafficSignal {
         }
         self.phases = replaced;
 
-        let mut phase = Phase::new();
-        for g in self.turn_groups.values() {
-            if g.turn_type == TurnType::Crosswalk {
-                phase.edit_group(g, TurnPriority::Protected, &self.turn_groups, map);
-            }
+        if !has_all_walk {
+            self.phases.push(all_walk_phase);
         }
-        self.phases.push(phase);
+        self != &orig
     }
 }