Add a Variable phase (#433)

* Add a Variable phase

Variable provides a min duration, a delay duration, and an additional duration. The maximum cycle time is min + additional. Once min has been exhausted, if there is demand, the cycle is extended by delay until there isn't any demand or the additional duration has been consumed.

#295

game/src/edit/traffic_signals/edits.rs

@@ -32,7 +32,7 @@ impl ChangeDuration {
                 Btn::close(ctx),
             ]),
             Widget::row(vec![
-                "Seconds:".draw_text(ctx),
+                "Seconds:".draw_text(ctx).centered_vert(),
                 Spinner::new(
                     ctx,
                     (
@@ -57,9 +57,44 @@ impl ChangeDuration {
                     match signal.stages[idx].phase_type {
                         PhaseType::Fixed(_) => true,
                         PhaseType::Adaptive(_) => false,
+                        PhaseType::Variable(_, _, _) => false,
                     },
                 ),
             ]),
+            Widget::row(vec![Line("Additional time this stage can last?")
+                .small_heading()
+                .draw(ctx)]),
+            Widget::row(vec![
+                "Seconds:".draw_text(ctx).centered_vert(),
+                Spinner::new(
+                    ctx,
+                    (1, 300),
+                    match signal.stages[idx].phase_type {
+                        PhaseType::Fixed(_) => 0,
+                        PhaseType::Adaptive(_) => 0,
+                        PhaseType::Variable(_, _, additional) => {
+                            additional.inner_seconds() as isize
+                        }
+                    },
+                )
+                .named("additional"),
+            ]),
+            Widget::row(vec![Line("How long with no demand to end stage?")
+                .small_heading()
+                .draw(ctx)]),
+            Widget::row(vec![
+                "Seconds:".draw_text(ctx).centered_vert(),
+                Spinner::new(
+                    ctx,
+                    (1, 300),
+                    match signal.stages[idx].phase_type {
+                        PhaseType::Fixed(_) => 0,
+                        PhaseType::Adaptive(_) => 0,
+                        PhaseType::Variable(_, delay, _) => delay.inner_seconds() as isize,
+                    },
+                )
+                .named("delay"),
+            ]),
             Line("Minimum time is set by the time required for crosswalk")
                 .secondary()
                 .draw(ctx),
@@ -79,7 +114,9 @@ impl SimpleState<App> for ChangeDuration {
                 let new_type = if panel.is_checked("phase type") {
                     PhaseType::Fixed(dt)
                 } else {
-                    PhaseType::Adaptive(dt)
+                    let delay = Duration::seconds(panel.spinner("delay") as f64);
+                    let additional = Duration::seconds(panel.spinner("additional") as f64);
+                    PhaseType::Variable(dt, delay, additional)
                 };
                 let idx = self.idx;
                 Transition::Multi(vec![

game/src/edit/traffic_signals/mod.rs

@@ -644,6 +644,13 @@ fn make_side_panel(
                     match canonical_stage.phase_type {
                         PhaseType::Fixed(d) => format!("Stage {}: {}", idx + 1, d),
                         PhaseType::Adaptive(d) => format!("Stage {}: {} (adaptive)", idx + 1, d),
+                        PhaseType::Variable(min, delay, additional) => format!(
+                            "Stage {}: {}, {}, {} (variable)",
+                            idx + 1,
+                            min,
+                            delay,
+                            additional
+                        ),
                     }
                     .draw_text(ctx),
                     Btn::svg_def("system/assets/tools/edit.svg").build(

game/src/info/intersection.rs

@@ -284,6 +284,13 @@ pub fn traffic_signal(
             match stage.phase_type {
                 PhaseType::Fixed(d) => Line(format!("Stage {}: {}", idx + 1, d)),
                 PhaseType::Adaptive(d) => Line(format!("Stage {}: {} (adaptive)", idx + 1, d)),
+                PhaseType::Variable(min, delay, additional) => Line(format!(
+                    "Stage {}: {}, {}, {} (variable)",
+                    idx + 1,
+                    min,
+                    delay,
+                    additional
+                )),
             }
             .draw(ctx),
         );

map_gui/src/render/traffic_signal.rs

@@ -1,7 +1,7 @@
 use std::collections::BTreeSet;
 
 use geom::{Angle, ArrowCap, Circle, Distance, Duration, Line, PolyLine, Pt2D};
-use map_model::{IntersectionID, Movement, Stage, TurnPriority, SIDEWALK_THICKNESS};
+use map_model::{IntersectionID, Movement, PhaseType, Stage, TurnPriority, SIDEWALK_THICKNESS};
 use widgetry::{Color, GeomBatch, Line, Prerender, RewriteColor, Text};
 
 use crate::options::TrafficSignalStyle;
@@ -36,14 +36,24 @@ pub fn draw_signal_stage(
             }
 
             let (yellow_light, percent) = if let Some(t) = time_left {
-                (
-                    t <= Duration::seconds(5.0),
-                    (t / stage.phase_type.simple_duration()) as f32,
-                )
+                if stage.phase_type.simple_duration() > Duration::ZERO {
+                    (
+                        t <= Duration::seconds(5.0),
+                        (t / stage.phase_type.simple_duration()) as f32,
+                    )
+                } else {
+                    (true, 1.0)
+                }
             } else {
                 (false, 1.0)
             };
-            let yellow = Color::YELLOW;
+            // The warning color for fixed is yellow, for anything else its orange to clue the
+            // user into it possibly extending.
+            let indicator_color = if let PhaseType::Fixed(_) = stage.phase_type {
+                Color::YELLOW
+            } else {
+                Color::ORANGE
+            };
             for m in &stage.protected_movements {
                 if !m.crosswalk {
                     // TODO Maybe less if shoulders meet
@@ -62,7 +72,7 @@ pub fn draw_signal_stage(
                     if let Ok(pl) = pl.maybe_exact_slice(slice_start, pl.length() - slice_end) {
                         batch.push(
                             if yellow_light {
-                                yellow
+                                indicator_color
                             } else {
                                 app.cs().signal_protected_turn.alpha(percent)
                             },
@@ -98,7 +108,7 @@ pub fn draw_signal_stage(
                 );
                 batch.extend(
                     if yellow_light {
-                        yellow
+                        indicator_color
                     } else {
                         app.cs().signal_protected_turn.alpha(percent)
                     },
@@ -212,7 +222,8 @@ fn draw_time_left(
 ) {
     let radius = Distance::meters(2.0);
     let center = app.map().get_i(i).polygon.center();
-    let percent = time_left / stage.phase_type.simple_duration();
+    let duration = stage.phase_type.simple_duration();
+    let percent = if duration > Duration::ZERO { time_left / duration } else { 1.0 };
     batch.push(
         app.cs().signal_box,
         Circle::new(center, 1.2 * radius).to_polygon(),

map_model/src/objects/traffic_signals.rs

@@ -7,7 +7,7 @@ use abstutil::{deserialize_btreemap, retain_btreeset, serialize_btreemap, Timer}
 use geom::{Distance, Duration, Speed};
 
 use crate::make::traffic_signals::{brute_force, get_possible_policies};
-use crate::objects::traffic_signals::PhaseType::{Adaptive, Fixed};
+use crate::objects::traffic_signals::PhaseType::{Adaptive, Fixed, Variable};
 use crate::raw::OriginalRoad;
 use crate::{
     osm, CompressedMovementID, DirectedRoadID, Direction, IntersectionID, Map, Movement,
@@ -50,13 +50,24 @@ pub enum PhaseType {
     /// repeat the stage entirely.
     // TODO This is a silly policy, but a start towards variable timers.
     Adaptive(Duration),
+    /// Minimum is the minimum duration, 0 allows cycle to be skipped if no demand.
+    /// Delay is the elapsed time with no demand that ends a cycle.
+    /// Additional is the additional duration for an extended cycle.
+    Variable(Duration, Duration, Duration),
 }
 
 impl PhaseType {
     // TODO Maybe don't have this; force callers to acknowledge different policies
     pub fn simple_duration(&self) -> Duration {
         match self {
             PhaseType::Fixed(d) | PhaseType::Adaptive(d) => *d,
+            PhaseType::Variable(duration, delay, _) => {
+                if *duration > Duration::ZERO {
+                    *duration
+                } else {
+                    *delay
+                }
+            }
         }
     }
 }
@@ -315,6 +326,7 @@ impl Stage {
             self.phase_type = match self.phase_type {
                 PhaseType::Adaptive(_) => Adaptive(time),
                 PhaseType::Fixed(_) => Fixed(time),
+                PhaseType::Variable(_, delay, additional) => Variable(time, delay, additional),
             };
         }
     }
@@ -345,6 +357,13 @@ impl ControlTrafficSignal {
                         PhaseType::Adaptive(d) => {
                             seattle_traffic_signals::PhaseType::Adaptive(d.inner_seconds() as usize)
                         }
+                        PhaseType::Variable(min, delay, additional) => {
+                            seattle_traffic_signals::PhaseType::Variable(
+                                min.inner_seconds() as usize,
+                                delay.inner_seconds() as usize,
+                                additional.inner_seconds() as usize,
+                            )
+                        }
                     },
                 })
                 .collect(),
@@ -393,6 +412,13 @@ impl ControlTrafficSignal {
                         seattle_traffic_signals::PhaseType::Adaptive(d) => {
                             PhaseType::Adaptive(Duration::seconds(d as f64))
                         }
+                        seattle_traffic_signals::PhaseType::Variable(min, delay, additional) => {
+                            PhaseType::Variable(
+                                Duration::seconds(min as f64),
+                                Duration::seconds(delay as f64),
+                                Duration::seconds(additional as f64),
+                            )
+                        }
                     },
                 });
             } else {

sim/src/mechanics/intersection.rs

@@ -70,8 +70,12 @@ struct State {
 
 #[derive(Clone, Serialize, Deserialize)]
 struct SignalState {
+    // The current stage of the signal, zero based
     current_stage: usize,
+    // The time when the signal is checked for advancing
     stage_ends_at: Time,
+    // The count of time an adaptive signal has been extended during the current stage.
+    extensions_count: usize,
 }
 
 #[derive(PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, Clone, Debug)]
@@ -134,6 +138,7 @@ impl IntersectionSimState {
         }
         if self.break_turn_conflict_cycles {
             if let AgentID::Car(car) = agent {
+                // todo: when drain_filter() is no longer experimental, use it instead of retian_btreeset()
                 retain_btreeset(&mut self.blocked_by, |(_, c)| *c != car);
             }
         }
@@ -248,16 +253,24 @@ impl IntersectionSimState {
         map: &Map,
         scheduler: &mut Scheduler,
     ) {
+        // trivial function that advances the signal stage and returns duration
+        fn advance(signal_state: &mut SignalState, signal: &ControlTrafficSignal) -> Duration {
+            signal_state.current_stage = (signal_state.current_stage + 1) % signal.stages.len();
+            signal.stages[signal_state.current_stage]
+                .phase_type
+                .simple_duration()
+        }
+
         let state = self.state.get_mut(&id).unwrap();
         let signal_state = state.signal.as_mut().unwrap();
         let signal = map.get_traffic_signal(id);
-
+        let duration: Duration;
         // Switch to a new stage?
         assert_eq!(now, signal_state.stage_ends_at);
         let old_stage = &signal.stages[signal_state.current_stage];
         match old_stage.phase_type {
             PhaseType::Fixed(_) => {
-                signal_state.current_stage += 1;
+                duration = advance(signal_state, signal);
             }
             PhaseType::Adaptive(_) => {
                 // TODO Make a better policy here. For now, if there's _anyone_ waiting to start a
@@ -269,22 +282,60 @@ impl IntersectionSimState {
                 if state.waiting.keys().all(|req| {
                     old_stage.get_priority_of_turn(req.turn, signal) != TurnPriority::Protected
                 }) {
-                    signal_state.current_stage += 1;
+                    duration = advance(signal_state, signal);
                     self.events.push(Event::Alert(
                         AlertLocation::Intersection(id),
                         "Repeating an adaptive stage".to_string(),
                     ));
+                } else {
+                    duration = signal.stages[signal_state.current_stage]
+                        .phase_type
+                        .simple_duration();
+                }
+            }
+            PhaseType::Variable(min, delay, additional) => {
+                // test if anyone is waiting in current stage, and if so, extend the signal cycle.
+                // Filter out pedestrians, as they've had their chance and the delay
+                // could be short enough to keep them on the curb.
+                let delay = std::cmp::max(Duration::const_seconds(1.0), delay);
+                // Only extend for the fixed additional time
+                if signal_state.extensions_count as f64 * delay.inner_seconds()
+                    >= additional.inner_seconds()
+                {
+                    self.events.push(Event::Alert(
+                        AlertLocation::Intersection(id),
+                        format!(
+                            "exhausted a variable stage {},{},{},{}",
+                            min, delay, additional, signal_state.extensions_count
+                        ),
+                    ));
+                    duration = advance(signal_state, signal);
+                    signal_state.extensions_count = 0;
+                } else if state.waiting.keys().all(|req| {
+                    if let AgentID::Pedestrian(_) = req.agent {
+                        return true;
+                    }
+                    // Should we only allow protected to extend or any not banned?
+                    // currently only the protected demand control extended.
+                    old_stage.get_priority_of_turn(req.turn, signal) != TurnPriority::Protected
+                }) {
+                    signal_state.extensions_count = 0;
+                    duration = advance(signal_state, signal);
+                } else {
+                    signal_state.extensions_count += 1;
+                    duration = delay;
+                    self.events.push(Event::Alert(
+                        AlertLocation::Intersection(id),
+                        format!(
+                            "Extending a variable stage {},{},{},{}",
+                            min, delay, additional, signal_state.extensions_count
+                        ),
+                    ));
                 }
             }
-        }
-        if signal_state.current_stage == signal.stages.len() {
-            signal_state.current_stage = 0;
         }
 
-        signal_state.stage_ends_at = now
-            + signal.stages[signal_state.current_stage]
-                .phase_type
-                .simple_duration();
+        signal_state.stage_ends_at = now + duration;
         scheduler.push(signal_state.stage_ends_at, Command::UpdateIntersection(id));
         self.wakeup_waiting(now, id, scheduler, map);
     }
@@ -900,6 +951,7 @@ impl SignalState {
         let mut state = SignalState {
             current_stage: 0,
             stage_ends_at: now,
+            extensions_count: 0,
         };
 
         let signal = map.get_traffic_signal(id);