Fix graphical overmap notes displays & mission arrow #50190

GoLoT · 2021-07-24T13:43:05Z

Summary

Bugfixes "Fix graphical overmap notes displays & mission arrow"

Purpose of change

The new graphical overmap is not displaying the notes preview window (top-left corner) and when creating/editing notes it only displays the creation window until a key is pressed, then it disappears (while still taking inputs and working as expected, only without any feedback).

There is also an issue with the mission marker that causes errors when the map Z level is different from the mission target Z level (I think a difference >2).

Describe the solution

This PR fixes the creation note window by preventing refreshing the overmap while the note editing window is open. Previously the draw call order would cause the overmap redraw to draw on top of the window. The window refreshes automatically on keypresses, that's why the window was only displayed right after being opened and before typing anything.

EDIT: Previous description was slightly wrong. The issue wasn't the draw order but the sum of two different issues: There were unnecessary calls to SDL_RenderSetClipRect() that clipped the viewport causing rendering problems in draw_om() and on top of that the clipping rectangle wasn't being reset at the end of the function so the clipping bounds were affecting the draw calls for the overlaid windows. Removing the unneeded calls and adding the clipping rectangle reset at the end of draw_om() fixed the issues.

A new note text window is now rendered next to the currently selected tile, instead of using the old top-left corner window. This is a very basic display at the moment as I'm not good at designing UIs. Feel free to give any feedback or tweak it.

Then there is the issue of the arrow marker causing errors because of the Z level difference. Fixing the Z level when pathfinding does the trick.

EDIT: Also added fixes to align labels properly to the tile grid (Thanks @Qrox ), enabled display of paths for auto-travel and fixed NPC notes displaying for un-seen NPCs.

Describe alternatives you've considered

Trying to reuse the old corner window for notes. It should be as easy as creating a new window and using the same drawing code at the end of the draw_om() function. I thought that an SDL-based window would look better (if designed properly) so I went with that instead.

Testing

GoLoT · 2021-07-24T13:45:26Z

Pinging @KorGgenT for a review.

Qrox · 2021-07-24T15:21:23Z

Other menus such as the keybindings menu and note list menu are also quirky when overmap is open. ui_manager is supposed to draw the UIs by the order they are created, but the overmap UI is somehow bypassing this logic.

Drawing the note next to the selected tile does look nice, but for other menus to work, the overmap UI code still needs to be fixed.

preventing refreshing the overmap while the note editing window is open

Refreshing is required when the note editing window is open and the game window is resized.

GoLoT · 2021-07-24T15:28:49Z

I tried invalidating the ui_adapter and forcing a redraw at the end of the SDL draw call: Both cases ended up in an infinite recursion as the ui_adaptor that is being invalidated is the same that does the invalidation.

I'll take a closer look at the code and see if it's possible to create a new ui_adaptor to trigger the draw calls for the SDL buffer so it can be added to the stack to control the drawing order.

GoLoT · 2021-07-24T16:28:51Z

Ok, I think I have a proper solution now. I had to flag the overmap ui_adaptor as disable_uis_below to prevent the redraw call from invalidating windows below the overmap window (it's fullscreen anyway) and then invalidate the region for the overmap buffer.

It's working with any windows I could open from the overmap (keybinds, notes list, create note and so on). I'll do some more testing now with resizing and other interactions.

EDIT: Resizing works partially. It works for the overlaid windows but the actual map turns black. I'll see if I can fix it by forcing a draw call from on_resize().

EDIT2: Resizing also causes issues with other windows that only redraw partially but I think it's not as big an issue as the note creation window disappearing while typing the note text. I think the resizing part should be tackled on a different PR. For now this appears to be working fine until the window is resized, and then the problem is just a visual glitch that's easily fixed by closing and reopening the window. The other adaptors might need to be flagged as disable_uis_below too.

Qrox · 2021-07-24T16:44:04Z

Keybindings menu is not working correctly: if you press ? twice and press ESC, the first keybindings menu is open but not displayed.

I tried to move the tilecontext->draw_om call to cata_cursesport::curses_drawwindow, similar to how w_terrain is redrawn, but it does not seem to fix the issue...

Also, notes are cut off and color tags are not correctly handled:

(The cut-off issue also happens with city names, perhaps it is related to the font/tile size?)

GoLoT · 2021-07-24T16:54:37Z

I'll have to flag the keybind window as disable_uis_below , it seems to be the only way to prevent redrawing the overmap. Also I'll need to check the function that parses color tags to create a new one that returns SDLcolors.

But before that I'll give creating an exclusive adaptor for the overmap another try. That way I might be able to control better when it should be redrawn.

Qrox · 2021-07-24T17:11:34Z

I removed the city name drawing code and the keybindings menu seemed to render correctly, so there might be something wrong with city name drawing. I'll look into it.

GoLoT · 2021-07-24T17:20:42Z

Hmm... there is this comment // Labels need to be drawn last, as anything that attempts to draw a sprite after will fail. that I assumed would be referring to the fact that SDL_RenderSetClipRect() is clipping the viewport and anything that he tried to draw after that (ouside bounds) didn't work, so I didn't think much else of it. If it's only the city labels that are causing issues maybe we can find a different way to draw them.

EDIT: Wow, that was it. I don't know what made you suspect the city labels but you were spot-on. Removing the viewport clipping calls is working perfectly for me with all overlay windows.

EDIT2: Well, now there is a new problem: NPCs aren't rendered on the overmap. I'll check that out.

GoLoT · 2021-07-24T17:40:28Z

@Qrox Can you repeat your tests after pulling the last commit? I'm trying to reproduce a problem I had where NPCs disappeared from the overmap but I'm not able to. What I noticed is that NPCs are rendered in all Z levels instead of their current level.

I'll fix that and if you can't find any other issues I'll consider the redrawing issue fixed and start working on the color tags in map notes.

Qrox · 2021-07-24T17:45:30Z

I think I know what's wrong here:

When drawing the submap terrain, SDL_RenderSetClipRect( renderer.get(), nullptr ) is called at the end of cata_tiles::draw to reset the clip rect, so subsequent draws are not clipped.

However, that call is missing from the cata_tiles::draw_om, which causes all subsequent UIs to be clipped.

When first opening a menu in the overmap UI, ui_manager skips the overmap drawing code and reuses the previously drawn overmap, so the clip rect is not set, and the menu is drawn correctly. When the menu is refreshed by resizing etc, though, overmap is also refreshed, so the clip rect is set, which causes all subsequent menus to be clipped and not displayed.

Resetting the clip rect at the end of cata_tiles::draw_om fixes all menus and NPCs are also correctly displayed, though I also notice that NPCs (and the player) are drawn on all z-levels.

GoLoT · 2021-07-24T17:54:25Z

Ok, I added the missing SDL_RenderSetClipRect( renderer.get(), nullptr ) call at the end of draw_om(). That should take care of any problems if SDL_RenderSetClipRect() calls are added. Also fixed the NPCs rendering at all Z levels. Now I think the only thing missing is the color tag parsing.

Thanks for the help.

src/sdltiles.cpp

Co-authored-by: Jianxiang Wang (王健翔) <[email protected]>

Qrox · 2021-07-24T20:04:51Z

I've pushed a PR to your branch to fix some issues with label and note position: GoLoT#1. I'll solve the conflicts later.

GoLoT · 2021-07-24T20:16:59Z

There we go. Example of colored notes:

logros13 · 2021-07-25T00:28:27Z

don't know if you know already but while you're at it, the fast travel preview (shift+W) and map search (/) where also not working last i tried.

Qrox · 2021-07-25T05:51:28Z

Fast travel preview needs to be implemented for the tiles version. Map search is fixed by this PR already.

Fix city label & note locations

GoLoT · 2021-07-25T10:21:33Z

don't know if you know already but while you're at it, the fast travel preview (shift+W) and map search (/) where also not working last i tried.

Added player paths using the same sprite as the cursor. Does anyone know if there is a sprite for this specifically or can think of a good one that could be used for pathing?

EDIT: Also noticed that NPC notes (not the sprites) are displaying even when out of range to see the NPCs. This is happening with the curses version too so I don't know if it's intended or not. Probably not.

GoLoT · 2021-07-25T12:19:40Z

src/overmap_ui.cpp

-    for( const auto &npc : overmap_buffer.get_npcs_near_omt( center, 0 ) ) {
-        if( !npc->marked_for_death ) {
-            corner_text.emplace_back( npc->basic_symbol_color(), npc->name );
+    if( has_debug_vision || overmap_buffer.seen( center ) ) {


Should this part be moved to a new PR so it can be backported?

Qrox · 2021-07-25T13:10:56Z

Added player paths using the same sprite as the cursor. Does anyone know if there is a sprite for this specifically or can think of a good one that could be used for pathing?

You can use "highlight".

GoLoT · 2021-07-25T15:02:26Z

You can use "highlight".

Yup, this looks much better. Thanks again.

KorGgenT · 2021-08-06T15:15:23Z

src/sdltiles.cpp

+
+        const int starting_x = draw_point.x;
+
+        for( auto &line : notes_window_text ) {


Suggested change

for( auto &line : notes_window_text ) {

for( std::pair<nc_color, std::string> &line : notes_window_text ) {

we generally like to limit our usage of auto.

KorGgenT · 2021-08-06T15:22:42Z

src/sdltiles.cpp

+        const int starting_x = draw_point.x;
+
+        for( auto &line : notes_window_text ) {
+            const auto color_segments = split_by_color( line.second );


Suggested change

const auto color_segments = split_by_color( line.second );

const std::vector<std::string> color_segments = split_by_color( line.second );

as here

KorGgenT · 2021-08-06T15:22:57Z

src/sdltiles.cpp

+            draw_point.x = starting_x;
+
+            int line_length = 0;
+            for( auto seg : color_segments ) {


Suggested change

for( auto seg : color_segments ) {

for( std::string seg : color_segments ) {

and here

KorGgenT · 2021-08-06T15:23:57Z

src/sdltiles.cpp

+            geometry->rect( renderer, background_rect, SDL_Color{ 0, 0, 0, 175 } );
+
+            // Draw colored text segments
+            for( auto &colored_line : colored_lines ) {


Suggested change

for( auto &colored_line : colored_lines ) {

for( std::pair<nc_color, std::string> &colored_line : colored_lines ) {

Qrox · 2021-08-06T15:45:05Z

Probably fixes #50529 and #49708, based on their descriptions.

Fix graphical overmap notes display & mission arrow

9faacca

BrettDong added <Bugfix> This is a fix for a bug (or closes open issue) Info / User Interface Game - player communication, menus, etc. [C++] Changes (can be) made in C++. Previously named `Code` labels Jul 24, 2021

actual-nh requested a review from KorGgenT July 24, 2021 15:27

Invalidate region to properly refresh overmap windows

9b72a71

actual-nh added the SDL: Tiles / Sound Tiles visual interface and sounds. label Jul 24, 2021

Avoid SDL_RenderSetClipRect() calls

7a760cf

GoLoT added 2 commits July 24, 2021 19:49

Draw NPCs only on their Z level

3c92e3f

Add missing SDL_RenderSetClipRect() reset at the end of draw_om()

95e024f

Qrox reviewed Jul 24, 2021

View reviewed changes

src/sdltiles.cpp Outdated Show resolved Hide resolved

src/sdltiles.cpp Show resolved Hide resolved

GoLoT and others added 4 commits July 24, 2021 20:10

Update src/sdltiles.cpp

4edf4b6

Co-authored-by: Jianxiang Wang (王健翔) <[email protected]>

Remove unneeded comment

3965011

Add color tag support to graphical overmap notes

250f9b1

You didn't see this. Don't make me rebase.

f3c1e71

Remove unused variables

71dd903

Fix city label & note position

a355853

Merge pull request #1 from Qrox/overmap

ae229e6

Fix city label & note locations

Fix NPC notes displaying when NPCs aren't visible

af19abd

GoLoT commented Jul 25, 2021

View reviewed changes

Change auto-travel path to use higlight instead of cursor sprite

04ba48d

KorGgenT reviewed Aug 6, 2021

View reviewed changes

KorGgenT merged commit 0b250e7 into CleverRaven:master Aug 6, 2021

KorGgenT mentioned this pull request Aug 6, 2021

Graphical Overmap Causes Map Notes Text To Disappear #49708

Closed

anothersimulacrum mentioned this pull request Aug 6, 2021

No visuals on "Preview Route" on overmap with "Use tiles to display overmap" set to true I-am-Erk/CDDA-Tilesets#787

Closed

GoLoT mentioned this pull request Aug 6, 2021

Fix overmap NPC names showing for NPCs out of range #50534

Merged

olanti-p mentioned this pull request Oct 31, 2021

[DDA Port] Draw overmap using sprites (aka the new Graphical Overmap) + bugfixes cataclysmbnteam/Cataclysm-BN#1101

Merged

RenechCDDA mentioned this pull request Jul 1, 2023

universal awareness of npc #48435

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix graphical overmap notes displays & mission arrow #50190

Fix graphical overmap notes displays & mission arrow #50190

GoLoT commented Jul 24, 2021 •

edited

Loading

GoLoT commented Jul 24, 2021

Qrox commented Jul 24, 2021

GoLoT commented Jul 24, 2021

GoLoT commented Jul 24, 2021 •

edited

Loading

Qrox commented Jul 24, 2021 •

edited

Loading

GoLoT commented Jul 24, 2021

Qrox commented Jul 24, 2021

GoLoT commented Jul 24, 2021 •

edited

Loading

GoLoT commented Jul 24, 2021

Qrox commented Jul 24, 2021 •

edited

Loading

GoLoT commented Jul 24, 2021

Qrox commented Jul 24, 2021 •

edited

Loading

GoLoT commented Jul 24, 2021

logros13 commented Jul 25, 2021

Qrox commented Jul 25, 2021

GoLoT commented Jul 25, 2021 •

edited

Loading

GoLoT Jul 25, 2021

Qrox commented Jul 25, 2021

GoLoT commented Jul 25, 2021

KorGgenT Aug 6, 2021 •

edited

Loading

KorGgenT Aug 6, 2021

KorGgenT Aug 6, 2021

KorGgenT Aug 6, 2021

Qrox commented Aug 6, 2021


		const int starting_x = draw_point.x;

		for( auto &line : notes_window_text ) {

	for( auto &line : notes_window_text ) {
	for( std::pair<nc_color, std::string> &line : notes_window_text ) {

	const auto color_segments = split_by_color( line.second );
	const std::vector<std::string> color_segments = split_by_color( line.second );

	for( auto seg : color_segments ) {
	for( std::string seg : color_segments ) {

	for( auto &colored_line : colored_lines ) {
	for( std::pair<nc_color, std::string> &colored_line : colored_lines ) {

Fix graphical overmap notes displays & mission arrow #50190

Fix graphical overmap notes displays & mission arrow #50190

Conversation

GoLoT commented Jul 24, 2021 • edited Loading

Summary

Purpose of change

Describe the solution

Describe alternatives you've considered

Testing

GoLoT commented Jul 24, 2021

Qrox commented Jul 24, 2021

GoLoT commented Jul 24, 2021

GoLoT commented Jul 24, 2021 • edited Loading

Qrox commented Jul 24, 2021 • edited Loading

GoLoT commented Jul 24, 2021

Qrox commented Jul 24, 2021

GoLoT commented Jul 24, 2021 • edited Loading

GoLoT commented Jul 24, 2021

Qrox commented Jul 24, 2021 • edited Loading

GoLoT commented Jul 24, 2021

Qrox commented Jul 24, 2021 • edited Loading

GoLoT commented Jul 24, 2021

logros13 commented Jul 25, 2021

Qrox commented Jul 25, 2021

GoLoT commented Jul 25, 2021 • edited Loading

GoLoT Jul 25, 2021

Choose a reason for hiding this comment

Qrox commented Jul 25, 2021

GoLoT commented Jul 25, 2021

KorGgenT Aug 6, 2021 • edited Loading

Choose a reason for hiding this comment

KorGgenT Aug 6, 2021

Choose a reason for hiding this comment

KorGgenT Aug 6, 2021

Choose a reason for hiding this comment

KorGgenT Aug 6, 2021

Choose a reason for hiding this comment

Qrox commented Aug 6, 2021

GoLoT commented Jul 24, 2021 •

edited

Loading

GoLoT commented Jul 24, 2021 •

edited

Loading

Qrox commented Jul 24, 2021 •

edited

Loading

GoLoT commented Jul 24, 2021 •

edited

Loading

Qrox commented Jul 24, 2021 •

edited

Loading

Qrox commented Jul 24, 2021 •

edited

Loading

GoLoT commented Jul 25, 2021 •

edited

Loading

KorGgenT Aug 6, 2021 •

edited

Loading