From dd27098ca888a64621fe8504661c2387ea6bc926 Mon Sep 17 00:00:00 2001 From: Dustin Carlino Date: Wed, 25 Aug 2021 11:16:16 -0700 Subject: [PATCH] Recalculate the hovered lane in the road editor very carefully. Any time (#734) * Recalculate the hovered lane in the road editor very carefully. Any time the map is edited, lane IDs change. There were many possible crashes before. * Don't deselect lane when clicking in screen-space --- game/src/edit/roads.rs | 99 +++++++++++++++++++----------------------- 1 file changed, 44 insertions(+), 55 deletions(-) diff --git a/game/src/edit/roads.rs b/game/src/edit/roads.rs index 1f8035effe..88129c17f3 100644 --- a/game/src/edit/roads.rs +++ b/game/src/edit/roads.rs @@ -22,6 +22,7 @@ use crate::edit::{apply_map_edits, can_edit_lane, speed_limit_choices}; pub struct RoadEditor { r: RoadID, selected_lane: Option, + // This is only for hovering on a lane in the map, not for hovering on a lane card. hovering_on_lane: Option, top_panel: Panel, main_panel: Panel, @@ -29,10 +30,6 @@ pub struct RoadEditor { // (cache_key: (selected, hovering), Drawable) lane_highlights: ((Option, Option), Drawable), - // Immediately after modifying the map but before the mouse moves, we should recalculate - // mouseover - recalculate_mouseover: bool, - // Undo/redo management num_edit_cmds_originally: usize, redo_stack: Vec, @@ -69,8 +66,6 @@ impl RoadEditor { lane_highlights: ((None, None), Drawable::empty(ctx)), hovering_on_lane: None, - recalculate_mouseover: false, - num_edit_cmds_originally: app.primary.map.get_edits().commands.len(), redo_stack: Vec::new(), orig_road_state: app.primary.map.get_r_edit(r), @@ -124,9 +119,9 @@ impl RoadEditor { self.selected_lane = select_new_lane_offset .map(|offset| self.lane_for_idx(app, (idx as isize + offset) as usize)); + self.recalc_hovering(ctx, app); self.recalc_all_panels(ctx, app); - self.recalculate_mouseover = true; Transition::Keep } @@ -176,6 +171,15 @@ impl RoadEditor { } None } + + // Lane IDs may change with every edit. So immediately after an edit, recalculate mouseover. + fn recalc_hovering(&mut self, ctx: &EventCtx, app: &mut App) { + app.recalculate_current_selection(ctx); + self.hovering_on_lane = match app.primary.current_selection.take() { + Some(ID::Lane(l)) if can_edit_lane(app, l) => Some(l), + _ => None, + }; + } } impl State for RoadEditor { @@ -210,10 +214,11 @@ impl State for RoadEditor { app.primary.map.get_config(), ), }); - self.selected_lane = None; - self.hovering_on_lane = None; apply_map_edits(ctx, app, edits); + self.redo_stack.clear(); + self.selected_lane = None; + self.recalc_hovering(ctx, app); panels_need_recalc = true; } "undo" => { @@ -222,6 +227,7 @@ impl State for RoadEditor { apply_map_edits(ctx, app, edits); self.selected_lane = None; + self.recalc_hovering(ctx, app); panels_need_recalc = true; } "redo" => { @@ -230,6 +236,7 @@ impl State for RoadEditor { apply_map_edits(ctx, app, edits); self.selected_lane = None; + self.recalc_hovering(ctx, app); panels_need_recalc = true; } "jump to road" => { @@ -310,6 +317,7 @@ impl State for RoadEditor { self.redo_stack.clear(); self.selected_lane = Some(self.lane_for_idx(app, idx)); + self.recalc_hovering(ctx, app); panels_need_recalc = true; } else if x == "Access restrictions" { // The RoadEditor maintains an undo/redo stack for a single road, but the @@ -368,55 +376,36 @@ impl State for RoadEditor { _ => debug!("main_panel had unhandled outcome"), } - // let prev_hovering_on_lane = self.hovering_on_lane.take(); - if ctx.canvas.get_cursor_in_map_space().is_some() { - self.recalculate_mouseover = false; - app.recalculate_current_selection(ctx); - - let hovering = match app.primary.current_selection.take() { - Some(ID::Lane(l)) if can_edit_lane(app, l) => Some(l), - _ => None, - }; - - if let Some(l) = hovering { - if self.hovering_on_lane != Some(l) { - self.hovering_on_lane = Some(l); - panels_need_recalc = true; - } - - if ctx.normal_left_click() { - if app.primary.map.get_l(l).parent == self.r { - self.selected_lane = Some(l); - panels_need_recalc = true; - } else { - // Switch to editing another road, first compressing the edits here if - // needed. - if let Some(edits) = self.compress_edits(app) { - apply_map_edits(ctx, app, edits); - } - return Transition::Replace(RoadEditor::new_state(ctx, app, l)); - } - } - } else { - if self.hovering_on_lane.is_some() { - self.hovering_on_lane = None; - panels_need_recalc = true; - } - - if self.selected_lane.is_some() && ctx.normal_left_click() { - // Deselect the current lane - self.selected_lane = None; - self.hovering_on_lane = None; - panels_need_recalc = true; - } - } - } else { - // If we're not hovering on the map, then we're not hovering on a lane. - if self.hovering_on_lane.is_some() { - self.hovering_on_lane = None; + if ctx.redo_mouseover() { + let hovering_before = self.hovering_on_lane; + self.recalc_hovering(ctx, app); + if self.hovering_on_lane != hovering_before { panels_need_recalc = true; } } + if let Some(l) = self.hovering_on_lane { + if ctx.normal_left_click() { + if app.primary.map.get_l(l).parent == self.r { + self.selected_lane = Some(l); + panels_need_recalc = true; + } else { + // Switch to editing another road, first compressing the edits here if + // needed. + if let Some(edits) = self.compress_edits(app) { + apply_map_edits(ctx, app, edits); + } + return Transition::Replace(RoadEditor::new_state(ctx, app, l)); + } + } + } else if self.selected_lane.is_some() + && ctx.canvas.get_cursor_in_map_space().is_some() + && ctx.normal_left_click() + { + // Deselect the current lane + self.selected_lane = None; + self.hovering_on_lane = None; + panels_need_recalc = true; + } if panels_need_recalc { self.recalc_all_panels(ctx, app);