Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix map memory edge case #37460

Merged
merged 8 commits into from
Feb 11, 2020
Merged

Fix map memory edge case #37460

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8bf6486
Fix map connections
Trioct Jan 25, 2020
57c1a63
Fix more cache stuff
Trioct Jan 28, 2020
00b8d67
Minor tweak
Trioct Jan 28, 2020
817737e
Minor tweak
Trioct Jan 28, 2020
d4dd605
Factor out variable
Trioct Jan 28, 2020
698303a
Replace auto with type
Trioct Jan 28, 2020
07aaa42
Use alternative solution
Trioct Jan 28, 2020
9df4d5d
Use suggested solution
Trioct Feb 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/cata_tiles.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1294,6 +1294,9 @@ void cata_tiles::draw( const point &dest, const tripoint &center, int width, int
formatted_text( text, catacurses::red, NORTH ) );
}
}
if( !p.invisible[0] ) {
g->m.check_and_set_seen_cache( p.pos );
}
}
}
// tile overrides are already drawn in the previous code
Expand Down Expand Up @@ -2179,8 +2182,14 @@ bool cata_tiles::draw_terrain( const tripoint &p, const lit_level ll, int &heigh
// do something to get other terrain orientation values
}
const std::string &tname = t.id().str();
const tripoint abs_pos = g->m.getabs( p );
if( g->m.check_seen_cache( p ) ) {
g->u.memorize_tile( g->m.getabs( p ), tname, subtile, rotation );
g->u.memorize_tile( abs_pos, tname, subtile, rotation );
} else {
const memorized_terrain_tile &cached_tile = g->u.get_memorized_tile( abs_pos );
if( cached_tile.subtile != subtile && cached_tile.tile == tname ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rotation also affects appearance of tiles, so it should also be compared. Anyway, I think you can just memorize the tile without comparing it to the old value.

Also I'm not sure if it fully fixes #36609. Could you test this case:

  1. Move to a place with low light-level and straight walls, so you can only see a 3x3 area around the character.
  2. Move near the wall so you can see three wall tiles, which are now memorized.
  3. Move away from the wall so you can't see it, move 3 steps along the wall's direction, and then move near the wall again, so you see another three wall tiles.
  4. Does the six wall tiles look like this: ====== (what we want) or ==><== (what we had)?

Copy link
Contributor Author

@Trioct Trioct Jan 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rotation does change the appearance of the tile, but I don't think rotation should be changed without resetting the cache at that index, whereas revealing more information about the map does change the subtile without telling the cache.

And I believe the tile needs to be compared to the old one because (as far as I can tell) only the top level "thing" is stored in map memory. Furniture, for example, could be overwritten by terrain, as only one can be remembered. So if something has been seen there, it can't be overwritten unless I know it's terrain.
An improvement I might actually be able to do is to just check if it's terrain and overwrite it.
Another option is to un-see the tiles being drawn before re-seeing them (so they are re-memorized), but I feel like this would remove the purpose of the map memory seen cache, if I'm thinking correctly.

And I had noticed that situation in the test would happen, but I would consider that a different bug. You can also fix the memory by just sweeping over the disconnections, and it'll fix it. I can't think of a convenient way to fix that bug.

g->u.memorize_tile( abs_pos, tname, subtile, rotation );
}
}
// draw the actual terrain if there's no override
if( !neighborhood_overridden ) {
Expand Down
2 changes: 2 additions & 0 deletions src/game.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9422,6 +9422,7 @@ void game::place_player_overmap( const tripoint &om_dest )
m.clear_vehicle_cache( z );
m.clear_vehicle_list( z );
}
m.access_cache( get_levz() ).map_memory_seen_cache.reset();
// offset because load_map expects the coordinates of the top left corner, but the
// player will be centered in the middle of the map.
const tripoint map_om_pos( tripoint( 2 * om_dest.x, 2 * om_dest.y,
Expand Down Expand Up @@ -10427,6 +10428,7 @@ void game::vertical_shift( const int z_after )
m.clear_vehicle_cache( z_before );
m.access_cache( z_before ).vehicle_list.clear();
m.access_cache( z_before ).zone_vehicles.clear();
m.access_cache( z_before ).map_memory_seen_cache.reset();
m.set_transparency_cache_dirty( z_before );
m.set_outside_cache_dirty( z_before );
m.load( tripoint( get_levx(), get_levy(), z_after ), true );
Expand Down