Ring has duplicate adjacent points when fastforwarding in tutorial mode

Steps to Reproduce

Run cargo run --bin game
Start traffic simulation tutorial
Fast forward to tutorial step 7/9
Click "Try It"
Click the "jump to specific time" button
Observe crash

Log Output
thread 'main' panicked at 'called Result::unwrap() on an Err value: Ring has duplicate adjacent points near Pt2D(500, 50)', geom\src\ring.rs:73:24

abstreet_ring_crash.txt

Expected Behaviour
Tutorial fast forwards successfully with no crash

I've been investigating this and this is what I've found so far. Tracing the stack trace:

The trace starts here, trying to create a new ring with a vector of points:

abstreet/geom/src/ring.rs

Line 72 in 412d771

Ring::new(pts).unwrap()

These points trigger an error condition that checks for any 2 points that are the exact same and next to each other:

abstreet/geom/src/ring.rs

Line 26 in 412d771

if let Some(pair) = pts.windows(2).find(|pair| pair[0] == pair[1]) {

The points vector at this point is:
[Pt2D { x: 0.0, y: 50.0 }, Pt2D { x: 0.0006, y: 47.5 }, Pt2D { x: 0.0102, y: 45.0 }, Pt2D { x: 0.0171, y: 42.5 }, Pt2D { x: 0.0246, y: 40.0 }, Pt2D { x: 0.0317, y: 37.5 }, Pt2D { x: 0.0404, y: 35.0 }, Pt2D { x: 0.0482, y: 32.5 }, Pt2D { x: 0.0549, y: 30.0 }, Pt2D { x: 0.0622, y: 27.5 }, Pt2D { x: 0.0775, y: 25.0 }, Pt2D { x: 0.0839, y: 22.5 }, Pt2D { x: 0.0904, y: 20.0 }, Pt2D { x: 0.1066, y: 17.5 }, Pt2D { x: 0.1132, y: 15.0 }, Pt2D { x: 0.1423, y: 12.5 }, Pt2D { x: 0.1483, y: 10.0 }, Pt2D { x: 0.1718, y: 7.5 }, Pt2D { x: 0.1795, y: 5.0 }, Pt2D { x: 0.2018, y: 2.5 }, Pt2D { x: 0.231, y: 0.0 }, Pt2D { x: 0.539, y: 2.5 }, Pt2D { x: 0.8647, y: 5.0 }, Pt2D { x: 0.9114, y: 7.5 }, Pt2D { x: 1.0376, y: 10.0 }, Pt2D { x: 1.0376, y: 12.5 }, Pt2D { x: 1.1102, y: 15.0 }, Pt2D { x: 1.1341, y: 17.5 }, Pt2D { x: 1.1508, y: 20.0 }, Pt2D { x: 1.1834, y: 22.5 }, Pt2D { x: 1.1971, y: 25.0 }, Pt2D { x: 1.2476, y: 27.5 }, Pt2D { x: 1.2595, y: 30.0 }, Pt2D { x: 1.2679, y: 32.5 }, Pt2D { x: 1.3267, y: 35.0 }, Pt2D { x: 1.6413, y: 37.5 }, Pt2D { x: 1.7586, y: 40.0 }, Pt2D { x: 1.872, y: 42.5 }, Pt2D { x: 1.9351, y: 45.0 }, Pt2D { x: 2.3843, y: 47.5 }, Pt2D { x: 2.5705, y: 50.0 }, Pt2D { x: 500.0, y: 50.0 }, Pt2D { x: 500.0, y: 50.0 }, Pt2D { x: 0.0, y: 50.0 }]

The 2 duplicate adjacent points are the 2nd and 3rd last points Pt2D { x: 500.0, y: 50.0 }, Pt2D { x: 500.0, y: 50.0 }

If we look at the caller of Ring::must_new that eventually causes the error in Ring::new, we find this line:

abstreet/apps/game/src/sandbox/time_warp.rs

Line 422 in 412d771

downsampled.push(Pt2D::new(width, height));

This line is specifically adding the 2nd last point (the latter of the 2 points causing the error). Above this line, the function is using an algorithm to add points to the vector downsampled, but in this line we are manually adding onto the algorithm's output.

@dabreegster I could really use your input here as to a possible fix. Should this line be removed, should a simple if condition be added to prevent a duplicate being added, or does this require more investigation? Thanks!

Hi @Antel0pe, thank you for the clear repro and diving into the cause! This time warp tool is supposed to show a plot of how many active agents there are (y-axis) over time (x), so people can choose to jump to a crowded or uncrowded time:

downsampled.push(Pt2D::new(width, height)); is meant to be the bottom-right of the polygon, and downsampled.push(downsampled[0]); closes off the polygon. I guess the rightmost point is already the bottom of the curve, hence the crash. I would try changing it to Ring::deduping_new(downsampled).unwrap().into_polygon(). Even better, crashing the entire app if we fail to draw this area curve is a bad idea -- we should just hide the whole thing and log a warning if we can't render it correctly.

Also note that I haven't attempted to maintain tutorial mode in a while: #1078. The root cause here may be that the scenario in the tutorial is broken, or the prebaked simulation results for it haven't been updated in a while

Hey @dabreegster! Thank you very much for the help!

Yes while reading through the project docs and issues I understood that you had prioritized some of the other features in the app.

I noticed that you completely disabled the tutorial mode button and I was wondering if it was cool with you if I continued working on it, fixing any issues I find? I've re-enabled tutorial mode on my end and fixed this issue as you suggested and am interested in continuing to work on some of the other bugs I found, with a little bit of your guidance from you hopefully haha.

This is kinda my first real set of contributions to open source, first big project in rust, and I would love to eventually help you with some of the features you've prioritized... just once I've gotten my bearings 😅

I'd be more than happy if you wanted to work on the tutorial mode! It would be greatly appreciated to revive this. I ran through all of the tasks with your PR and actually nothing crashed. I haven't looked through all of the crashes people have reported in #1078, but from past rounds of maintenance, one of the things that occasionally breaks is all the map.find_b_by_osm_id and map.find_r_by_osm_id calls. The tutorial spawns agents and references specific roads and buildings, but when either we update the input OSM data (very rarely, but in principle it'd be nice to do this more frequently) or some code for splitting / merging roads changes, these hardcoded IDs can get out-of-sync and break. I have a few ideas to maybe improve this, but I'm not sure. If that's one of the problems you've started looking into, any thoughts about how to do it differently and not rely on IDs that might change?

And thanks for jumping into a large codebase for your first OSS contributions! Let me know what kinds of things you're interested in (UI? Traffic simulation? Map importing? Any of the apps particularly? Working on a big new feature like public transit, or polishing/fixing something existing?), and we can work together to find some fun things to work on that aren't impossibly complex first steps.

Thank you very much 😄

I created a new issue to discuss the id problem you mentioned just so it's organized for me

Thank you for being so welcoming about my contributions and so willing to help me work on things I'm interested in. I'm honestly interested in a little bit of everything haha 😅 Perhaps I can start off by fixing some simple bugs and small tasks to get more familiar with Rust and how A/B Street works?