Proper UI for picking multiple traffic signals to edit together

game/src/edit/traffic_signals/mod.rs

@@ -1,4 +1,5 @@
 mod edits;
+mod picker;
 mod preview;
 
 use crate::app::{App, ShowEverything};
@@ -44,7 +45,7 @@ pub struct TrafficSignalEditor {
 }
 
 // For every member intersection, the full state of that signal
-#[derive(Clone)]
+#[derive(Clone, PartialEq)]
 pub struct BundleEdits {
     signals: Vec<ControlTrafficSignal>,
 }
@@ -181,6 +182,19 @@ impl State for TrafficSignalEditor {
 
         match self.side_panel.event(ctx) {
             Outcome::Clicked(x) => {
+                if x == "Edit multiple signals" {
+                    // 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)
+                        .unwrap_or_else(|| BundleEdits::get_current(app, &self.members));
+                    self.original.apply(app);
+                    changes.commit(ctx, app);
+                    return Transition::Replace(picker::SignalPicker::new(
+                        ctx,
+                        self.members.clone(),
+                        self.mode.clone(),
+                    ));
+                }
                 if x == "Edit entire signal" {
                     return Transition::Push(edits::edit_entire_signal(
                         ctx,
@@ -292,20 +306,7 @@ impl State for TrafficSignalEditor {
                     } else {
                         let changes = BundleEdits::get_current(app, &self.members);
                         self.original.apply(app);
-
+                        changes.commit(ctx, app);
-                        let mut edits = app.primary.map.get_edits().clone();
-                        // TODO Can we batch these commands somehow, so undo/redo in edit mode
-                        // behaves properly?
-                        for signal in changes.signals {
-                            edits.commands.push(EditCmd::ChangeIntersection {
-                                i: signal.id,
-                                old: app.primary.map.get_i_edit(signal.id),
-                                new: EditIntersection::TrafficSignal(
-                                    signal.export(&app.primary.map),
-                                ),
-                            });
-                        }
-                        apply_map_edits(ctx, app, edits);
                         return Transition::Pop;
                     }
                 }
@@ -557,7 +558,10 @@ fn make_side_panel(
         txt.add(Line(format!("One full cycle lasts {}", total)));
     }
 
-    let mut col = vec![txt.draw(ctx)];
+    let mut col = vec![
+        txt.draw(ctx),
+        Btn::text_bg2("Edit multiple signals").build_def(ctx, None),
+    ];
     if members.len() == 1 {
         col.push(Btn::text_bg2("Edit entire signal").build_def(ctx, hotkey(Key::E)));
     }
@@ -643,6 +647,24 @@ impl BundleEdits {
         }
     }
 
+    fn commit(self, ctx: &mut EventCtx, app: &mut App) {
+        // Skip if there's no change
+        if self == BundleEdits::get_current(app, &self.signals.iter().map(|s| s.id).collect()) {
+            return;
+        }
+
+        let mut edits = app.primary.map.get_edits().clone();
+        // TODO Can we batch these commands somehow, so undo/redo in edit mode behaves properly?
+        for signal in self.signals {
+            edits.commands.push(EditCmd::ChangeIntersection {
+                i: signal.id,
+                old: app.primary.map.get_i_edit(signal.id),
+                new: EditIntersection::TrafficSignal(signal.export(&app.primary.map)),
+            });
+        }
+        apply_map_edits(ctx, app, edits);
+    }
+
     fn get_current(app: &App, members: &BTreeSet<IntersectionID>) -> BundleEdits {
         let signals = members
             .iter()

game/src/edit/traffic_signals/picker.rs

@@ -0,0 +1,112 @@
+use crate::app::App;
+use crate::common::CommonState;
+use crate::edit::TrafficSignalEditor;
+use crate::game::{PopupMsg, State, Transition};
+use crate::helpers::ID;
+use crate::sandbox::gameplay::GameplayMode;
+use ezgui::{
+    hotkey, Btn, Color, Composite, EventCtx, GeomBatch, GfxCtx, HorizontalAlignment, Key, Line,
+    Outcome, VerticalAlignment, Widget,
+};
+use map_model::IntersectionID;
+use std::collections::BTreeSet;
+
+pub struct SignalPicker {
+    members: BTreeSet<IntersectionID>,
+    composite: Composite,
+    mode: GameplayMode,
+}
+
+impl SignalPicker {
+    pub fn new(
+        ctx: &mut EventCtx,
+        members: BTreeSet<IntersectionID>,
+        mode: GameplayMode,
+    ) -> Box<dyn State> {
+        Box::new(SignalPicker {
+            members,
+            composite: Composite::new(Widget::col(vec![
+                Widget::row(vec![
+                    Line("Select multiple traffic signals")
+                        .small_heading()
+                        .draw(ctx),
+                    Btn::text_fg("X")
+                        .build(ctx, "close", hotkey(Key::Escape))
+                        .align_right(),
+                ]),
+                Btn::text_fg("Edit").build_def(ctx, hotkey(Key::E)),
+            ]))
+            .aligned(HorizontalAlignment::Center, VerticalAlignment::Top)
+            .build(ctx),
+            mode,
+        })
+    }
+}
+
+impl State for SignalPicker {
+    fn event(&mut self, ctx: &mut EventCtx, app: &mut App) -> Transition {
+        ctx.canvas_movement();
+        if ctx.redo_mouseover() {
+            app.recalculate_current_selection(ctx);
+        }
+        if let Some(ID::Intersection(i)) = app.primary.current_selection {
+            if app.primary.map.maybe_get_traffic_signal(i).is_some() {
+                if !self.members.contains(&i)
+                    && app.per_obj.left_click(ctx, "add this intersection")
+                {
+                    self.members.insert(i);
+                } else if self.members.contains(&i)
+                    && app.per_obj.left_click(ctx, "remove this intersection")
+                {
+                    self.members.remove(&i);
+                }
+            } else {
+                app.primary.current_selection = None;
+            }
+        } else {
+            app.primary.current_selection = None;
+        }
+
+        match self.composite.event(ctx) {
+            Outcome::Clicked(x) => match x.as_ref() {
+                "close" => {
+                    return Transition::Pop;
+                }
+                "Edit" => {
+                    if self.members.is_empty() {
+                        return Transition::Push(PopupMsg::new(
+                            ctx,
+                            "Error",
+                            vec!["Select at least one intersection"],
+                        ));
+                    }
+                    return Transition::Replace(TrafficSignalEditor::new(
+                        ctx,
+                        app,
+                        self.members.clone(),
+                        self.mode.clone(),
+                    ));
+                }
+                _ => unreachable!(),
+            },
+            _ => {}
+        }
+
+        Transition::Keep
+    }
+
+    fn draw(&self, g: &mut GfxCtx, app: &App) {
+        self.composite.draw(g);
+        CommonState::draw_osd(g, app);
+
+        let mut batch = GeomBatch::new();
+        for i in &self.members {
+            batch.push(
+                Color::RED.alpha(0.8),
+                app.primary.map.get_i(*i).polygon.clone(),
+            );
+        }
+        let draw = g.upload(batch);
+        g.redraw(&draw);
+    }
+}

game/src/sandbox/mod.rs

@@ -441,9 +441,9 @@ impl ContextualActions for Actions {
                 EditMode::new(ctx, app, self.gameplay.clone()),
                 Box::new(StopSignEditor::new(ctx, app, i, self.gameplay.clone())),
             ),
-            (ID::Intersection(i), "explore uber-turns") => Transition::Push(
+            (ID::Intersection(i), "explore uber-turns") => {
-                uber_turns::UberTurnPicker::new(ctx, app, i, self.gameplay.clone()),
+                Transition::Push(uber_turns::UberTurnPicker::new(ctx, app, i))
-            ),
+            }
             (ID::Lane(l), "explore turns from this lane") => {
                 Transition::Push(TurnExplorer::new(ctx, app, l))
             }

game/src/sandbox/uber_turns.rs

@@ -1,10 +1,9 @@
 use crate::app::{App, ShowEverything};
 use crate::common::CommonState;
-use crate::edit::{ClusterTrafficSignalEditor, EditMode, TrafficSignalEditor};
+use crate::edit::ClusterTrafficSignalEditor;
 use crate::game::{DrawBaselayer, PopupMsg, State, Transition};
 use crate::helpers::ID;
 use crate::render::{DrawOptions, BIG_ARROW_THICKNESS};
-use crate::sandbox::gameplay::GameplayMode;
 use ezgui::{
     hotkey, Btn, Checkbox, Color, Composite, Drawable, EventCtx, GeomBatch, GfxCtx,
     HorizontalAlignment, Key, Line, Outcome, Text, TextExt, VerticalAlignment, Widget,
@@ -17,17 +16,10 @@ use std::collections::BTreeSet;
 pub struct UberTurnPicker {
     members: BTreeSet<IntersectionID>,
     composite: Composite,
-    // TODO Plumbing this everywhere is annoying, is it time for it to live in App?
-    gameplay: GameplayMode,
 }
 
 impl UberTurnPicker {
-    pub fn new(
+    pub fn new(ctx: &mut EventCtx, app: &App, i: IntersectionID) -> Box<dyn State> {
-        ctx: &mut EventCtx,
-        app: &App,
-        i: IntersectionID,
-        gameplay: GameplayMode,
-    ) -> Box<dyn State> {
         let mut members = BTreeSet::new();
         if let Some(list) = IntersectionCluster::autodetect(i, &app.primary.map) {
             members.extend(list);
@@ -47,13 +39,11 @@ impl UberTurnPicker {
                         .align_right(),
                 ]),
                 Btn::text_fg("View uber-turns").build_def(ctx, hotkey(Key::Enter)),
-                Btn::text_fg("Edit (old attempt)").build_def(ctx, None),
+                Btn::text_fg("Edit").build_def(ctx, hotkey(Key::E)),
-                Btn::text_fg("Edit (new attempt)").build_def(ctx, hotkey(Key::E)),
                 Btn::text_fg("Detect all clusters").build_def(ctx, hotkey(Key::D)),
             ]))
             .aligned(HorizontalAlignment::Center, VerticalAlignment::Top)
             .build(ctx),
-            gameplay,
         })
     }
 }
@@ -95,7 +85,7 @@ impl State for UberTurnPicker {
                         true,
                     ));
                 }
-                "Edit (old attempt)" => {
+                "Edit" => {
                     if self.members.len() < 2 {
                         return Transition::Push(PopupMsg::new(
                             ctx,
@@ -109,17 +99,6 @@ impl State for UberTurnPicker {
                         &IntersectionCluster::new(self.members.clone(), &app.primary.map).0,
                     ));
                 }
-                "Edit (new attempt)" => {
-                    return Transition::ReplaceThenPush(
-                        EditMode::new(ctx, app, self.gameplay.clone()),
-                        TrafficSignalEditor::new(
-                            ctx,
-                            app,
-                            self.members.clone(),
-                            self.gameplay.clone(),
-                        ),
-                    );
-                }
                 "Detect all clusters" => {
                     self.members.clear();
                     for ic in IntersectionCluster::find_all(&app.primary.map) {