Check map bounds before checking Character::sees()

[irwiss]

Summary

None

Purpose of change

Fixes #66427

Describe the solution

Makes pl_sees overload on tinymap/fake_map return false without accessing lightmap

Doesn't solve the issue completely - the out of bounds read can still be reached just not from this chunk of code, but this will stop the random crash.

Describe alternatives you've considered

Testing

Mapgen is triggering it randomly, spawning default evacuee in shelter a few times triggers this bug on MSVC

Additional context

[andrei8l]

I don't think this is correct. Character::pos_bub() returns position relative to the main map, but the map running this function at mapgen is just a temporary tinymap. So the coordinate origins don't necessarily line up.

player_character.sees( p ) also only makes sense if this map is the main map. In both cases the tripoint needs to be converted to absolute from this map, then to local coordinates for get_map()

[irwiss]

I agree this isn't technically "correct", but I don't see an obvious way to call sees() with the bounds check but without making every non-mapgen caller pay the perf cost for it; Character::sees() calls into get_map().pl_sees() that reads out of bounds at line 799 during mapgen because the avatar isn't yet initialized

Cataclysm-DDA/src/lightmap.cpp

Lines 792 to 799 in d505f22

	Character &player_character = get_player_character();
	if( max_range >= 0 && square_dist( t, player_character.pos() ) > max_range &&
	map_cache.camera_cache[t.x][t.y] == 0 ) {
	return false; // Out of range!
	}
	const apparent_light_info a = apparent_light_helper( map_cache, t );
	const float light_at_player = map_cache.lm[player_character.posx()][player_character.posy()].max();

pos_bub will return relative to main map but that is what that function is checking

When ter_set/furn_set is called during mapgen the avatar position isn't in bounds of any map including the main one (it's hardcoded 20,10,-500 in Creature ctor, and at some point before world loads changes to -4768,-4634,-500)

Another option is a dynamic_cast/typeid hack, since the map/tinymap inheritance is somehow flipped around

Or just paying the price of a bounds check inside, but have it useless for most calls after game is past loading

Do you or anyone else have a better solution or opinion on one?