From 522b902453cf9b9f18e1a949d4f7e897c7851c31 Mon Sep 17 00:00:00 2001 From: Dustin Carlino Date: Thu, 11 Mar 2021 11:39:58 -0800 Subject: [PATCH] Improve the traffic signal timing UI. There's a choice between fixed and variable timing, but currently you have to remember to toggle it; the two extra spinners get ignored otherwise. The new version is still confusing, but I think it's an improvement. --- game/src/debug/uber_turns.rs | 2 +- game/src/edit/lanes.rs | 2 +- game/src/edit/traffic_signals/edits.rs | 126 +++++++++++++++---------- game/src/edit/traffic_signals/mod.rs | 1 + widgetry/src/app_state.rs | 9 +- widgetry/src/widgets/spinner.rs | 10 +- 6 files changed, 93 insertions(+), 57 deletions(-) diff --git a/game/src/debug/uber_turns.rs b/game/src/debug/uber_turns.rs index 15fd8e23bf..6f3203b5b9 100644 --- a/game/src/debug/uber_turns.rs +++ b/game/src/debug/uber_turns.rs @@ -243,7 +243,7 @@ impl SimpleState for UberTurnViewer { &mut self, ctx: &mut EventCtx, app: &mut App, - panel: &Panel, + panel: &mut Panel, ) -> Option { Some(Transition::Replace(UberTurnViewer::new( ctx, diff --git a/game/src/edit/lanes.rs b/game/src/edit/lanes.rs index 8c596ed2ad..3454f98244 100644 --- a/game/src/edit/lanes.rs +++ b/game/src/edit/lanes.rs @@ -169,7 +169,7 @@ impl SimpleState for LaneEditor { &mut self, ctx: &mut EventCtx, app: &mut App, - panel: &Panel, + panel: &mut Panel, ) -> Option { let mut edits = app.primary.map.get_edits().clone(); edits.commands.push(app.primary.map.edit_road_cmd( diff --git a/game/src/edit/traffic_signals/edits.rs b/game/src/edit/traffic_signals/edits.rs index 17ee08bf93..e776a9c46a 100644 --- a/game/src/edit/traffic_signals/edits.rs +++ b/game/src/edit/traffic_signals/edits.rs @@ -4,8 +4,8 @@ use map_model::{ ControlStopSign, ControlTrafficSignal, EditCmd, EditIntersection, IntersectionID, StageType, }; use widgetry::{ - Choice, DrawBaselayer, EventCtx, Key, Line, Panel, SimpleState, Spinner, State, TextExt, - Toggle, Widget, + Choice, DrawBaselayer, EventCtx, Key, Line, Panel, SimpleState, Spinner, State, Text, TextExt, + Widget, }; use crate::app::{App, Transition}; @@ -20,6 +20,7 @@ pub struct ChangeDuration { impl ChangeDuration { pub fn new( ctx: &mut EventCtx, + app: &App, signal: &ControlTrafficSignal, idx: usize, ) -> Box> { @@ -45,55 +46,54 @@ impl ChangeDuration { ) .named("duration"), ]), - Widget::row(vec![ - "Type:".text_widget(ctx), - Toggle::choice( - ctx, - "stage type", - "fixed", - "variable", - None, - match signal.stages[idx].stage_type { - StageType::Fixed(_) => true, - StageType::Variable(_, _, _) => false, - }, - ), - ]), - Widget::row(vec![Line("Additional time this stage can last?") - .small_heading() - .into_widget(ctx)]), - Widget::row(vec![ - "Seconds:".text_widget(ctx).centered_vert(), - Spinner::widget( - ctx, - (1, 300), - match signal.stages[idx].stage_type { - StageType::Fixed(_) => 0, - StageType::Variable(_, _, additional) => { - additional.inner_seconds() as isize - } - }, - ) - .named("additional"), - ]), - Widget::row(vec![Line("How long with no demand to end stage?") - .small_heading() - .into_widget(ctx)]), - Widget::row(vec![ - "Seconds:".text_widget(ctx).centered_vert(), - Spinner::widget( - ctx, - (1, 300), - match signal.stages[idx].stage_type { - StageType::Fixed(_) => 0, - StageType::Variable(_, delay, _) => delay.inner_seconds() as isize, - }, - ) - .named("delay"), - ]), Line("Minimum time is set by the time required for crosswalk") .secondary() .into_widget(ctx), + Widget::col(vec![ + Text::from_all(match signal.stages[idx].stage_type { + StageType::Fixed(_) => vec![ + Line("Fixed timing").small_heading(), + Line(" (Adjust both values below to enable variable timing)"), + ], + StageType::Variable(_, _, _) => vec![ + Line("Variable timing").small_heading(), + Line(" (Set either values below to 0 to use fixed timing."), + ], + }) + .into_widget(ctx) + .named("timing type"), + "How much additional time can this stage last?".text_widget(ctx), + Widget::row(vec![ + "Seconds:".text_widget(ctx).centered_vert(), + Spinner::widget( + ctx, + (0, 300), + match signal.stages[idx].stage_type { + StageType::Fixed(_) => 0, + StageType::Variable(_, _, additional) => { + additional.inner_seconds() as isize + } + }, + ) + .named("additional"), + ]), + "How long with no demand before the stage ends?".text_widget(ctx), + Widget::row(vec![ + "Seconds:".text_widget(ctx).centered_vert(), + Spinner::widget( + ctx, + (0, 300), + match signal.stages[idx].stage_type { + StageType::Fixed(_) => 0, + StageType::Variable(_, delay, _) => delay.inner_seconds() as isize, + }, + ) + .named("delay"), + ]), + ]) + .padding(10) + .bg(app.cs.inner_panel_bg) + .outline(ctx.style().section_outline), ctx.style() .btn_solid_primary .text("Apply") @@ -111,11 +111,11 @@ impl SimpleState for ChangeDuration { "close" => Transition::Pop, "Apply" => { let dt = Duration::seconds(panel.spinner("duration") as f64); - let new_type = if panel.is_checked("stage type") { + let delay = Duration::seconds(panel.spinner("delay") as f64); + let additional = Duration::seconds(panel.spinner("additional") as f64); + let new_type = if delay == Duration::ZERO || additional == Duration::ZERO { StageType::Fixed(dt) } else { - let delay = Duration::seconds(panel.spinner("delay") as f64); - let additional = Duration::seconds(panel.spinner("additional") as f64); StageType::Variable(dt, delay, additional) }; let idx = self.idx; @@ -133,6 +133,30 @@ impl SimpleState for ChangeDuration { } } + fn panel_changed( + &mut self, + ctx: &mut EventCtx, + _: &mut App, + panel: &mut Panel, + ) -> Option { + let new_label = Text::from_all( + if panel.spinner("delay") == 0 || panel.spinner("additional") == 0 { + vec![ + Line("Fixed timing").small_heading(), + Line(" (Adjust both values below to enable variable timing)"), + ] + } else { + vec![ + Line("Variable timing").small_heading(), + Line(" (Set either values below to 0 to use fixed timing."), + ] + }, + ) + .into_widget(ctx); + panel.replace(ctx, "timing type", new_label); + None + } + fn other_event(&mut self, ctx: &mut EventCtx, _: &mut App) -> Transition { if ctx.normal_left_click() && ctx.canvas.get_cursor_in_screen_space().is_none() { return Transition::Pop; diff --git a/game/src/edit/traffic_signals/mod.rs b/game/src/edit/traffic_signals/mod.rs index 0f87a1ba06..95c3e219d7 100644 --- a/game/src/edit/traffic_signals/mod.rs +++ b/game/src/edit/traffic_signals/mod.rs @@ -223,6 +223,7 @@ impl State for TrafficSignalEditor { let idx = x.parse::().unwrap() - 1; return Transition::Push(edits::ChangeDuration::new( ctx, + app, &canonical_signal, idx, )); diff --git a/widgetry/src/app_state.rs b/widgetry/src/app_state.rs index d3369149f2..e02e714442 100644 --- a/widgetry/src/app_state.rs +++ b/widgetry/src/app_state.rs @@ -221,7 +221,12 @@ pub trait SimpleState { ) -> Transition; /// Called when something on the panel has changed. If a transition is returned, stop handling /// the event and immediately apply the transition. - fn panel_changed(&mut self, _: &mut EventCtx, _: &mut A, _: &Panel) -> Option> { + fn panel_changed( + &mut self, + _: &mut EventCtx, + _: &mut A, + _: &mut Panel, + ) -> Option> { None } /// Called when the mouse has moved. @@ -257,7 +262,7 @@ impl State for SimpleStateWrapper { Outcome::Clicked(action) => self.inner.on_click(ctx, app, &action, &self.panel), Outcome::Changed => self .inner - .panel_changed(ctx, app, &self.panel) + .panel_changed(ctx, app, &mut self.panel) .unwrap_or_else(|| self.inner.other_event(ctx, app)), Outcome::Nothing => self.inner.other_event(ctx, app), } diff --git a/widgetry/src/widgets/spinner.rs b/widgetry/src/widgets/spinner.rs index c2716c3c1f..a816de7cca 100644 --- a/widgetry/src/widgets/spinner.rs +++ b/widgetry/src/widgets/spinner.rs @@ -71,11 +71,17 @@ impl Spinner { up.get_dims().height + down.get_dims().height + 1.0, ); if current < low { + warn!( + "Spinner's initial value is out of bounds! {}, bounds ({}, {})", + current, low, high + ); current = low; - warn!("Spinner current value is out of bounds!"); } else if high < current { + warn!( + "Spinner's initial value is out of bounds! {}, bounds ({}, {})", + current, low, high + ); current = high; - warn!("Spinner current value is out of bounds!"); } let mut spinner = Spinner {