Stuck at level 3 of 15-min Santa

[HSalat]

panicked at 'called Result::unwrap() on an Err value: Can't find https://www.openstreetmap.org/node/53218389', apps/santa/src/before_level.rs:50:66

[dabreegster]

The problem is that the config for Santa levels uses OSM IDs to specify where the player starts the level. This was a very bad idea in hindsight, since OSM IDs aren't stable over time. At some point I downloaded new OSM data for the area and so this broke.

The fix should be to use geom::LonLat instead. apps/santa/src/level.rs defines the per-level config, and currently we have start: osm::NodeID. It shouldn't be too much work to manually fix up the 7 levels using OSM node history. Then when starting the level, finding the matching intersection should be OK -- instead of map.find_i_by_osm_id, we can use geom::FindClosest, plug in map.all_intersections(), and grab the closest one.

@stuartlynn, this is maybe a better-scoped good first issue if you're interested?

[dabreegster]

And this is more generally related to #995

[stuartlynn]

Happy to figure this out. Want to assign the issue to me?

[stuartlynn]

A few thoughts / questions here

There seem to be a few places in the code where level.start is used to find the intersectionID

abstreet/apps/santa/src/game.rs

Lines 93 to 97 in 7000260

	let start = app
	.map
	.find_i_by_osm_id(level.start)
	.unwrap_or_else(|_| panic!("can't find {}", level.start));
	let player = Player::new(ctx, app, start);

abstreet/apps/santa/src/before_level.rs

Lines 48 to 53 in 7000260

	let start = app
	.map
	.get_i(app.map.find_i_by_osm_id(level.start).unwrap())
	.polygon
	.center();
	ctx.canvas.center_on_map_pt(start);

abstreet/apps/santa/src/after_level.rs

Lines 31 to 35 in 7000260

	let start = app
	.map
	.get_i(app.map.find_i_by_osm_id(level.start).unwrap())
	.polygon
	.center();

We probably dont want to construct the FindClosest quad tree individually for each of these cases and so it seems like the best idea is to write a new method find_intersection_from_lon_lat on the Map struct.

This would invlove creating the QuadTree from intersections at Map instantiation and storing the result for future use on the Map struct.

This would add some overhead to the creation of Map structs though and it seems like this functionality really is just used for the santa game. So I want to check that this is ok?

We could also lazily generate the quad tree by having FindClosest stored on the Map struct as a RefCel and populating it on the first call to find_intersection_from_lon_lat.

@dabreegster not sure what your preferred solution is here? Would find_intersection_from_lon_lat be more generally useful or should we just take the hit of potentially constructing the QuadTree multiple times?

[dabreegster]

Thanks for taking this on!

We could also lazily generate the quad tree by having FindClosest stored on the Map struct as a RefCel and populating it on the first call to find_intersection_from_lon_lat.

This gets my vote. Unlike road_to_buildings, it might be a bit more expensive to compute and we don't need it everywhere. RefCell is perfect, with #[serde(skip_serializing, skip_deserializing)].

FWIW, approaches to similar problems in other apps are to build a PerMap struct that changes with the map. For example, the one for ltn has a bunch of cached per-map state. Santa is simpler, and this is very tied to the Map.

As bonus, a quick grep suggests apps/fifteen_min/src/single_start.rs could be refactored to make use of this new struct. I'd also suggest making it take Pt2D and use map.gps_bounds to convert LonLat separately. Pt2D is Euclidean space per map: https://a-b-street.github.io/docs/tech/map/details.html#coordinate-system

[stuartlynn]

Sounds great. Should have a PR tomorrow!

[stuartlynn]

I think this can be closed out unless there is some work deploying the solution

[dabreegster]

Yep, thanks! I'll roll a new release (ignore the "every Sunday" bit, hasn't been true in a while) at some point