Highlight OMTs revealed by maps #70482
Conversation
github-actions
bot
added
Info / User Interface
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
github-actions
bot
added
json-styled
Ehhh it defaults on, I'm not sure that's strictly necessary. |
|
I didn't realize this wasn't working in graphical overmap until today. It now is working. But the performance (for graphical overmap) is absolutely atrocious? We're talking several seconds of delay between blink, game unresponsive in the meanwhile. Toggling the option to show/hide reveals causes the same delay. |
A check for points within the displayed bounds might help? |
github-actions
bot
added
the
BasicBuildPassed
|
Does this allow you to re-use old maps to re-highlight the things they revealed? Because that would be very useful. It's easy to lose track of something like where a handwritten note is pointing if you don't immediately make a note. |
No, that information simply isn't saved and would be rather annoying to save. (The spot you use the map on would probably need to be emplaced back into the item and then kept along with the save) |
|
Saving the activation point of the map so that the map could be reused would be trivial, actually. |
| @@ -645,6 +646,8 @@ static void draw_ascii( | |||
| size_t count = 0; | |||
| }; | |||
| std::unordered_set<tripoint_abs_omt> npc_path_route; | |||
| std::unordered_set<tripoint_abs_omt> newly_revealed( player_character.map_revealed_omts.begin(), | |||
There was a problem hiding this comment.
You are only using newly_revealed variable inside of if statement below, so it could be put there and it won't populate it each time even if overmap uistate does not imply showing map revealed omts.
| std::vector<tripoint_abs_omt> &revealed_highlights = get_avatar().map_revealed_omts; | ||
| auto it = std::find( revealed_highlights.begin(), revealed_highlights.end(), omp ); | ||
| if( blink && show_map_revealed && it != revealed_highlights.end() ) { | ||
| draw_from_id_string( "highlight", omp.raw(), 0, 0, lit_level::LIT, false ); | ||
| } | ||
|
|
There was a problem hiding this comment.
It should be something like this to avoid iterating through map_revealed_omts when overmap uistate does not imply showing map revealed omts:
if( blink && show_map_revealed ) {
if( it != revealed_highlights.end() ) {
std::vector<tripoint_abs_omt> &revealed_highlights = get_avatar().map_revealed_omts;
auto it = std::find( revealed_highlights.begin(), revealed_highlights.end(), omp );
draw_from_id_string( "highlight", omp.raw(), 0, 0, lit_level::LIT, false );
}
}
There was a problem hiding this comment.
The real problem is using std::find on a vector of points. This is simply O(n²), and will always be slow. The points need to be stored in a hash table (or a set, which uses a hash table internally) so that testing for membership can be O(1).
There was a problem hiding this comment.
There is no need to search for anything if data would not even be used.
There was a problem hiding this comment.
Oh, that is certainly true as well. It’s just that the time is dominated by searching the whole list for every single tile that might be drawn.
Summary
Features "Toggle to highlight OMTs revealed by maps"
Purpose of change
A lot of people have mentioned using a map and not being able to find what, if anything, these maps revealed.
Describe the solution
Store a list of OMTs that the last map revealed, make it blink on the map screen. Add a toggle to turn this on/off. (Also respects blinking rules)
NB: Using a new map clears the list of tiles known to have been revealed by the previous map. This is to prevent them from sticking around forever.
Describe alternatives you've considered
@Procyonae was bouncing ideas back and forth with me and has a branch at Procyonae/Cataclysm-DDA@master...Procyonae:Cataclysm-DDA:MapActionHint
Their branch(not functional? as of this writing) would only highlight newly revealed tiles. This is a change that might be worth considering, if it can be made to work.
Testing
2023-12-27.17-17-53.mp4
Additional context
Known deficiencies:
-Any map tagged by the map will be counted as revealed regardless of whether or not it could be seen before. So you could use a map, reveal exactly 0 new tiles, and still have hundreds (or more) tiles marked as having been revealed.
-Since the
map_revealed_omtscontainer is added to by any function using reveal_map_actor::reveal_targets, missions and other things that reveal tiles(e.g. evac shelter computer) will also be added to it. And subsequently, can/will only be be cleared of highlighting by using a map.