starting to organize warnings in a much more reasonable way.

abstutil/src/lib.rs

@@ -15,7 +15,7 @@ pub use crate::io::{
     load_all_objects, read_binary, read_json, save_object, serialize_btreemap, serialize_multimap,
     to_json, write_binary, write_json, FileWithProgress,
 };
-pub use crate::logs::{format_log_record, LogAdapter};
+pub use crate::logs::{format_log_record, LogAdapter, Warn};
 pub use crate::notes::note;
 pub use crate::random::{fork_rng, WeightedUsizeChoice};
 pub use crate::time::{elapsed_seconds, prettyprint_usize, MeasureMemory, Profiler, Timer};

abstutil/src/logs.rs

@@ -1,3 +1,4 @@
+use crate::Timer;
 use log::{Level, Log, Metadata, Record};
 use yansi::Paint;
 
@@ -31,3 +32,91 @@ pub fn format_log_record(record: &Record) -> String {
         record.args()
     )
 }
+
+// - If it doesn't make sense to plumb Timer to a library call, return Warn<T>.
+// - If there's no Timer, plumb the Warn<T>.
+// - If a Timer is available and there's a Warn<T>, use get() or with_context().
+// - If a Timer is available and something goes wrong, directly call warn().
+// - DO NOT prefer plumbing the Warn<T> and accumulating context. It's usually too tedious. Check
+//   out DrawIntersection for an example.
+pub struct Warn<T> {
+    value: T,
+    warnings: Vec<String>,
+}
+
+impl<T> Warn<T> {
+    pub fn ok(value: T) -> Warn<T> {
+        Warn {
+            value,
+            warnings: Vec::new(),
+        }
+    }
+
+    pub fn warn(value: T, warning: String) -> Warn<T> {
+        Warn {
+            value,
+            warnings: vec![warning],
+        }
+    }
+
+    pub fn warnings(value: T, warnings: Vec<String>) -> Warn<T> {
+        Warn { value, warnings }
+    }
+
+    pub fn unwrap(self) -> T {
+        if !self.warnings.is_empty() {
+            println!("{} warnings:", self.warnings.len());
+            for line in self.warnings {
+                println!("{}", line);
+            }
+        }
+        self.value
+    }
+
+    pub fn get(self, timer: &mut Timer) -> T {
+        // TODO Context from the current Timer phase, caller
+        for line in self.warnings {
+            timer.warn(line);
+        }
+        self.value
+    }
+
+    pub fn with_context(self, timer: &mut Timer, context: String) -> T {
+        for line in self.warnings {
+            timer.warn(format!("{}: {}", context, line));
+        }
+        self.value
+    }
+
+    /*pub fn get_and_append<X>(self, other: &mut Warn<X>) -> T {
+        other.warnings.extend(self.warnings);
+        self.value
+    }
+
+    pub fn get_with_context<X>(self, other: &mut Warn<X>, context: String) -> T {
+        if !self.warnings.is_empty() {
+            other.warnings.extend(self.warnings);
+            // TODO Just apply to the last; no explicit nesting structure...
+            let last_line = format!("{}:\n  {}", context, other.warnings.pop().unwrap());
+            other.warnings.push(last_line);
+        }
+        self.value
+    }*/
+}
+
+impl Warn<()> {
+    pub fn empty() -> Warn<()> {
+        Warn::ok(())
+    }
+
+    /*pub fn add_warning(&mut self, line: String) {
+        self.warnings.push(line);
+    }
+
+    pub fn wrap<T>(self, value: T) -> Warn<T> {
+        Warn {
+            value,
+            warnings: self.warnings,
+        }
+    }*/
+}

abstutil/src/notes.rs

@@ -1,6 +1,8 @@
 use lazy_static::lazy_static;
 use std::sync::Mutex;
 
+// TODO Maybe just use Timer.
+
 lazy_static! {
     static ref NOTES: Mutex<Vec<String>> = Mutex::new(Vec::new());
 }

abstutil/src/time.rs

@@ -79,6 +79,7 @@ pub struct Timer {
     outermost_name: String,
 
     notes: Vec<String>,
+    pub(crate) warnings: Vec<String>,
 }
 
 struct TimerSpan {
@@ -95,11 +96,17 @@ impl Timer {
             stack: Vec::new(),
             outermost_name: name.to_string(),
             notes: Vec::new(),
+            warnings: Vec::new(),
         };
         t.start(name);
         t
     }
 
+    // TODO Shouldn't use this much.
+    pub fn throwaway() -> Timer {
+        Timer::new("throwaway")
+    }
+
     // Log immediately, but also repeat at the end, to avoid having to scroll up and find
     // interesting debug stuff.
     pub fn note(&mut self, line: String) {
@@ -107,6 +114,10 @@ impl Timer {
         self.notes.push(line);
     }
 
+    pub fn warn(&mut self, line: String) {
+        self.warnings.push(line);
+    }
+
     pub fn done(mut self) {
         let stop_name = self.outermost_name.clone();
         self.stop(&stop_name);
@@ -116,6 +127,7 @@ impl Timer {
             println!("{}", line);
         }
         println!();
+
         if !self.notes.is_empty() {
             for line in self.notes {
                 println!("{}", line);
@@ -123,6 +135,14 @@ impl Timer {
             println!();
         }
         notes::dump_notes();
+
+        if !self.warnings.is_empty() {
+            println!("{} warnings:", self.warnings.len());
+            for line in self.warnings {
+                println!("{}", line);
+            }
+            println!();
+        }
     }
 
     pub fn start(&mut self, name: &str) {

debug_initialmap/src/main.rs

@@ -111,7 +111,7 @@ impl viewer::ObjectID for ID {
 }
 
 fn load_initial_map(filename: &str, canvas: &mut Canvas, prerender: &Prerender) -> World<ID> {
-    let data: raw_data::InitialMap = read_binary(filename, &mut Timer::new("load data")).unwrap();
+    let data: raw_data::InitialMap = read_binary(filename, &mut Timer::throwaway()).unwrap();
 
     let mut w = World::new(&data.bounds);
 

editor/src/render/intersection.rs

@@ -1,6 +1,7 @@
 use crate::colors::ColorScheme;
 use crate::objects::{DrawCtx, ID};
 use crate::render::{DrawCrosswalk, DrawTurn, RenderOptions, Renderable};
+use abstutil::Timer;
 use ezgui::{Color, Drawable, GfxCtx, Prerender, ScreenPt, Text};
 use geom::{Bounds, Circle, Distance, Duration, Line, Polygon, Pt2D};
 use map_model::{
@@ -25,6 +26,7 @@ impl DrawIntersection {
         map: &Map,
         cs: &ColorScheme,
         prerender: &Prerender,
+        timer: &mut Timer,
     ) -> DrawIntersection {
         // Order matters... main polygon first, then sidewalk corners.
         let mut default_geom = vec![(
@@ -52,7 +54,7 @@ impl DrawIntersection {
             }
             let r = map.get_r(*i.roads.iter().next().unwrap());
             default_geom.extend(
-                calculate_border_arrows(i, r)
+                calculate_border_arrows(i, r, timer)
                     .into_iter()
                     .map(|p| (cs.get_def("incoming border node arrow", Color::PURPLE), p)),
             );
@@ -414,7 +416,7 @@ fn find_pts_between(pts: &Vec<Pt2D>, start: Pt2D, end: Pt2D) -> Option<Vec<Pt2D>
     None
 }
 
-fn calculate_border_arrows(i: &Intersection, r: &Road) -> Vec<Polygon> {
+fn calculate_border_arrows(i: &Intersection, r: &Road, timer: &mut Timer) -> Vec<Polygon> {
     let mut result = Vec::new();
 
     // These arrows should point from the void to the road
@@ -436,7 +438,8 @@ fn calculate_border_arrows(i: &Intersection, r: &Road) -> Vec<Polygon> {
                 line.unbounded_dist_along(Distance::meters(-9.5)),
                 line.unbounded_dist_along(Distance::meters(-0.5)),
             )
-            .make_arrow(width / 3.0),
+            .make_arrow(width / 3.0)
+            .with_context(timer, format!("outgoing border arrows for {}", r.id)),
         );
     }
 
@@ -458,7 +461,8 @@ fn calculate_border_arrows(i: &Intersection, r: &Road) -> Vec<Polygon> {
                 line.unbounded_dist_along(Distance::meters(-0.5)),
                 line.unbounded_dist_along(Distance::meters(-9.5)),
             )
-            .make_arrow(width / 3.0),
+            .make_arrow(width / 3.0)
+            .with_context(timer, format!("incoming border arrows for {}", r.id)),
         );
     }
     result

editor/src/render/lane.rs

@@ -1,6 +1,7 @@
 use crate::colors::ColorScheme;
 use crate::objects::{DrawCtx, ID};
 use crate::render::{RenderOptions, Renderable, BIG_ARROW_THICKNESS, PARCEL_BOUNDARY_THICKNESS};
+use abstutil::Timer;
 use ezgui::{Color, Drawable, GfxCtx, Prerender};
 use geom::{Bounds, Circle, Distance, Line, Polygon, Pt2D};
 use map_model::{
@@ -22,6 +23,7 @@ impl DrawLane {
         draw_lane_markings: bool,
         cs: &ColorScheme,
         prerender: &Prerender,
+        timer: &mut Timer,
     ) -> DrawLane {
         let road = map.get_r(lane.parent);
         let polygon = lane.lane_center_pts.make_polygons(LANE_THICKNESS);
@@ -46,7 +48,7 @@ impl DrawLane {
                 }
                 LaneType::Driving | LaneType::Bus => {
                     draw.extend(calculate_driving_lines(lane, road, cs));
-                    draw.extend(calculate_turn_markings(map, lane, cs));
+                    draw.extend(calculate_turn_markings(map, lane, cs, timer));
                 }
                 LaneType::Biking => {}
             };
@@ -236,7 +238,12 @@ fn calculate_stop_sign_line(
     ))
 }
 
-fn calculate_turn_markings(map: &Map, lane: &Lane, cs: &ColorScheme) -> Vec<(Color, Polygon)> {
+fn calculate_turn_markings(
+    map: &Map,
+    lane: &Lane,
+    cs: &ColorScheme,
+    timer: &mut Timer,
+) -> Vec<(Color, Polygon)> {
     let mut results: Vec<(Color, Polygon)> = Vec::new();
 
     // Are there multiple driving lanes on this side of the road?
@@ -248,12 +255,17 @@ fn calculate_turn_markings(map: &Map, lane: &Lane, cs: &ColorScheme) -> Vec<(Col
     }
 
     for turn in map.get_turns_from_lane(lane.id) {
-        results.extend(turn_markings(turn, map, cs));
+        results.extend(turn_markings(turn, map, cs, timer));
     }
     results
 }
 
-fn turn_markings(turn: &Turn, map: &Map, cs: &ColorScheme) -> Vec<(Color, Polygon)> {
+fn turn_markings(
+    turn: &Turn,
+    map: &Map,
+    cs: &ColorScheme,
+    timer: &mut Timer,
+) -> Vec<(Color, Polygon)> {
     let lane = map.get_l(turn.id.src);
     let len = lane.length();
     if len < Distance::meters(7.0) {
@@ -278,6 +290,7 @@ fn turn_markings(turn: &Turn, map: &Map, cs: &ColorScheme) -> Vec<(Color, Polygo
     result.extend(
         turn_line
             .make_arrow(Distance::meters(0.05))
+            .with_context(timer, format!("turn_markings for {}", turn.id))
             .into_iter()
             .map(|p| (color, p)),
     );

editor/src/render/map.rs

@@ -82,6 +82,7 @@ impl DrawMap {
                 !flags.dont_draw_lane_markings,
                 cs,
                 prerender,
+                timer,
             ));
         }
 
@@ -104,7 +105,7 @@ impl DrawMap {
             .iter()
             .map(|i| {
                 timer.next();
-                DrawIntersection::new(i, map, cs, prerender)
+                DrawIntersection::new(i, map, cs, prerender, timer)
             })
             .collect();
         let draw_all_unzoomed_intersections = prerender.upload_borrowed(
@@ -281,7 +282,14 @@ impl DrawMap {
     ) {
         // No need to edit the quadtree; the bbox shouldn't depend on lane type.
         // TODO Preserve flags.dont_draw_lane_markings
-        self.lanes[id.0] = DrawLane::new(map.get_l(id), map, true, cs, prerender);
+        self.lanes[id.0] = DrawLane::new(
+            map.get_l(id),
+            map,
+            true,
+            cs,
+            prerender,
+            &mut Timer::throwaway(),
+        );
     }
 
     pub fn edit_remove_turn(&mut self, id: TurnID) {

ezgui/src/drawing.rs

@@ -107,7 +107,7 @@ impl<'a> GfxCtx<'a> {
     }
 
     pub fn draw_arrow(&mut self, color: Color, thickness: Distance, line: &Line) {
-        let polygons = line.make_arrow(thickness);
+        let polygons = line.make_arrow(thickness).unwrap();
         self.draw_polygon_batch(polygons.iter().map(|poly| (color, poly)).collect());
     }
 

geom/Cargo.toml

@@ -6,6 +6,7 @@ edition = "2018"
 
 [dependencies]
 aabb-quadtree = "0.1.0"
+abstutil = { path = "../abstutil" }
 geo = "0.11.0"
 ordered-float = "1.0.1"
 serde = "1.0.87"

geom/src/line.rs

@@ -1,4 +1,5 @@
 use crate::{Angle, Distance, PolyLine, Polygon, Pt2D, EPSILON_DIST};
+use abstutil::Warn;
 use serde_derive::{Deserialize, Serialize};
 use std::fmt;
 
@@ -48,15 +49,17 @@ impl Line {
     }
 
     // TODO One polygon, please :)
-    pub fn make_arrow(&self, thickness: Distance) -> Vec<Polygon> {
+    pub fn make_arrow(&self, thickness: Distance) -> Warn<Vec<Polygon>> {
         let head_size = thickness * 2.0;
         let angle = self.angle();
         let triangle_height = (head_size / 2.0).sqrt();
         if self.length() < triangle_height {
-            println!("Can't make_arrow of thickness {} for {}", thickness, self);
-            return Vec::new();
+            return Warn::warn(
+                Vec::new(),
+                format!("Can't make_arrow of thickness {} for {}", thickness, self),
+            );
         }
-        vec![
+        Warn::ok(vec![
             Polygon::new(&vec![
                 //self.pt2(),
                 //self.pt2().project_away(head_size, angle.rotate_degs(-135.0)),
@@ -78,7 +81,7 @@ impl Line {
                     .project_away(head_size, angle.rotate_degs(-135.0)),
                 self.pt2().project_away(head_size, angle.rotate_degs(135.0)),
             ]),
-        ]
+        ])
     }
 
     pub fn length(&self) -> Distance {

halloween/src/main.rs

@@ -31,11 +31,11 @@ impl UI {
             Map::new(
                 &flags.load_map,
                 MapEdits::new("map name"),
-                &mut Timer::new("load map"),
+                &mut Timer::throwaway(),
             )
             .unwrap()
         } else {
-            abstutil::read_binary(&flags.load_map, &mut Timer::new("load map")).unwrap()
+            abstutil::read_binary(&flags.load_map, &mut Timer::throwaway()).unwrap()
         };
 
         UI {

synthetic/src/model.rs

@@ -377,7 +377,7 @@ impl Model {
 
     // TODO Directly use raw_data and get rid of Model? Might be more maintainable long-term.
     pub fn import(path: &str) -> (Model, QuadTree<ID>) {
-        let data: raw_data::Map = read_binary(path, &mut Timer::new("load raw map")).unwrap();
+        let data: raw_data::Map = read_binary(path, &mut Timer::throwaway()).unwrap();
         let gps_bounds = data.get_gps_bounds();
 
         let mut m = Model::new();

tests/src/map_conversion.rs

@@ -19,8 +19,8 @@ pub fn run(t: &mut TestRunner) {
             fast_dev: false,
         };
 
-        let map1 = convert_osm::convert(&flags, &mut abstutil::Timer::new("convert map"));
-        let map2 = convert_osm::convert(&flags, &mut abstutil::Timer::new("convert map"));
+        let map1 = convert_osm::convert(&flags, &mut abstutil::Timer::throwaway());
+        let map2 = convert_osm::convert(&flags, &mut abstutil::Timer::throwaway());
 
         if map1 != map2 {
             // TODO tmp files
@@ -34,13 +34,13 @@ pub fn run(t: &mut TestRunner) {
         let map1 = map_model::Map::new(
             "../data/raw_maps/montlake.abst",
             map_model::MapEdits::new("montlake"),
-            &mut abstutil::Timer::new("raw to map"),
+            &mut abstutil::Timer::throwaway(),
         )
         .unwrap();
         let map2 = map_model::Map::new(
             "../data/raw_maps/montlake.abst",
             map_model::MapEdits::new("montlake"),
-            &mut abstutil::Timer::new("raw to map"),
+            &mut abstutil::Timer::throwaway(),
         )
         .unwrap();
 
@@ -56,7 +56,7 @@ pub fn run(t: &mut TestRunner) {
         map_model::Map::new(
             "../data/raw_maps/23rd.abst",
             map_model::MapEdits::new("23rd"),
-            &mut abstutil::Timer::new("raw to map"),
+            &mut abstutil::Timer::throwaway(),
         )
         .expect("23rd broke");
     });
@@ -65,7 +65,7 @@ pub fn run(t: &mut TestRunner) {
         map_model::Map::new(
             "../data/raw_maps/small_seattle.abst",
             map_model::MapEdits::new("small_seattle"),
-            &mut abstutil::Timer::new("raw to map"),
+            &mut abstutil::Timer::throwaway(),
         )
         .expect("small_seattle broke");
     });

tests/src/parking.rs

@@ -9,7 +9,7 @@ pub fn run(t: &mut TestRunner) {
         let (map, mut sim) = sim::load(
             sim::SimFlags::synthetic_test("parking_test", "park_on_goal_st"),
             None,
-            &mut Timer::new("setup test"),
+            &mut Timer::throwaway(),
         );
         let north_bldg = map.bldg("north").id;
         let south_bldg = map.bldg("south").id;
@@ -39,7 +39,7 @@ pub fn run(t: &mut TestRunner) {
         let (map, mut sim) = sim::load(
             sim::SimFlags::synthetic_test("parking_test", "wander_around_for_parking"),
             None,
-            &mut Timer::new("setup test"),
+            &mut Timer::throwaway(),
         );
         let north_bldg = map.bldg("north").id;
         let south_bldg = map.bldg("south").id;

tests/src/sim_completion.rs

@@ -7,7 +7,7 @@ pub fn run(t: &mut TestRunner) {
         let (map, mut sim) = sim::load(
             sim::SimFlags::for_test("aorta_model_completes"),
             Some(sim::Tick::from_seconds(30)),
-            &mut Timer::new("setup test"),
+            &mut Timer::throwaway(),
         );
         sim.small_spawn(&map);
         h.setup_done(&sim);

tests/src/sim_determinism.rs

@@ -7,7 +7,7 @@ pub fn run(t: &mut TestRunner) {
         let (map, mut sim) = sim::load(
             sim::SimFlags::for_test("serialization"),
             None,
-            &mut Timer::new("setup test"),
+            &mut Timer::throwaway(),
         );
         sim.small_spawn(&map);
 
@@ -22,7 +22,7 @@ pub fn run(t: &mut TestRunner) {
         let (map, mut sim1) = sim::load(
             sim::SimFlags::for_test("from_scratch_1"),
             None,
-            &mut Timer::new("setup test"),
+            &mut Timer::throwaway(),
         );
         let mut sim2 = sim::Sim::new(&map, "from_scratch_2".to_string(), Some(42), None);
         sim1.small_spawn(&map);
@@ -47,7 +47,7 @@ pub fn run(t: &mut TestRunner) {
         let (map, mut sim1) = sim::load(
             sim::SimFlags::for_test("with_savestating_1"),
             None,
-            &mut Timer::new("setup test"),
+            &mut Timer::throwaway(),
         );
         let mut sim2 = sim::Sim::new(&map, "with_savestating_2".to_string(), Some(42), None);
         sim1.small_spawn(&map);

tests/src/transit.rs

@@ -8,7 +8,7 @@ pub fn run(t: &mut TestRunner) {
         let (map, mut sim) = sim::load(
             SimFlags::for_test("bus_reaches_stops"),
             Some(Tick::from_seconds(30)),
-            &mut Timer::new("setup test"),
+            &mut Timer::throwaway(),
         );
         let route = map.get_bus_route("49").unwrap();
         let buses = sim.seed_bus_route(route, &map);
@@ -30,7 +30,7 @@ pub fn run(t: &mut TestRunner) {
         let (map, mut sim) = sim::load(
             SimFlags::for_test("ped_uses_bus"),
             Some(Tick::from_seconds(30)),
-            &mut Timer::new("setup test"),
+            &mut Timer::throwaway(),
         );
         let route = map.get_bus_route("49").unwrap();
         let buses = sim.seed_bus_route(route, &map);