Add separate overmap font, Fix framebuffer artifacts

[SeanMirrsen]

Adds support for a separate, different-sized ASCII font for the overmap.
Now almost all map/terrain display in the game can be decoupled from the text, allowing to use readable rectangular fonts for one, and pretty square fonts for the other.

In the making of this feature, my previous attempt at fixing the framebuffer artifacts as reported in #9348 was broken on the overmap, and as a result it was folded into this feature as well, as it depends on some of the new window IDs.

[BevapDin]

I think you can skip those changes in wincurses.cpp - not even the existing map_typeface is used there at all.

Neither of map_fontwidth, map_fontheight, map_fontsize nor map_typeface are used after they have been loaded from json. They seem to be pointless. Note that sdltiles.cpp has its own instances of those variable, they are not the same as those used here.

[SeanMirrsen]

I was under the impression that they store default config data, seeing as the game's config files are generated if not present (as they are on first run).

[BevapDin]

You're right, I retract my statement.

[SeanMirrsen]

Screenshots to illustrate the differences:
Default:

Square font:

[SeanMirrsen]

Okay, this error is odd. No member named "draw" in the WINDOW struct? It works fine in both configurations on my end, what could be causing this?

[BevapDin]

The WINDOW struct from catacurse.h is only used in builds without ncurse. With ncurses, the WINDOW struct that ncurses supplies is used instead. And as far as I know the content of that struct is not public, you're supposed to use the ncurses functions with a pointer to those windows.

[SeanMirrsen]

I guess it's easier to mvwputch a character of some kind into that window so it's marked as active as normal, since its contents aren't actually used. Its only purpose is to provide the dimensions of the black-out area.

[SeanMirrsen]

Well, I suppose this is all ready. I'm don't think anything else needs to be done here, unless some kind of bug crops up later.

[KA101]

OK, I haven't had a good hard code-read on this one so maybe I'm missing something. Is this SDL-only or does it apply to Curses builds too?

KA101 rereads

Never mind, this is within my scope. Queued.

[SeanMirrsen]

I don't know if the framebuffer even works in Curses (it might), but even if it does the only problems arose due to the difference in font sizes between terrain and the rest of the game in SDL. Since Curses is hard-locked to the one font, neither the framebuffer fix nor the overmap thing apply to the Curses builds. Nor indeed the window gaps, or anything else.

[KA101]

That's what I'd been thinking, but since this was being coded for Curses, I thought perhaps I was wrong.

[KA101]

Can verify that it doesn't seem to immediately break anything on Curses, at least. ;-)

[SeanMirrsen]

What changes I did for Curses was exactly so that it wouldn't break anything. :-P

[SeanMirrsen]

Bit of a problem here. I noticed intermittent glitches on the item pickup list when examining, and set up a debug trap to tell me if the window ID of the pickup window matched the window ID of the overmap.
Sure enough, after a few tries:

This is an issue because... well, because it happens at all. w_pickup is a temporary ID, it gets created anew every time with newwin(). w_overmap is part of the game class, stored as g->w_overmap. Whether it's the IDs spontaneously overlapping (often enough to be noticeable and catchable), or the == operator screwing up the comparison, it creates problems because curses_drawwindow() uses the == operator to check whether to draw the window in one or the other font. Or, with my blackspace window, whether to draw it at all. Theoretically, it could happen even with the terrain window ID, but for some reason it doesn't.

I'm considering adding an ID field to the WINDOW struct, so that the windows can be identified by given ID instead of an equality operator, but perhaps it's more of a problem with the new window IDs I've created? Somehow this never happens with the terrain window - i.e. the terrain window is a unique ID stored in g->w_terrain, but this glitch never happened to me before, and never happens with its font now, whereas I fairly often see the overmap font and the blackspace window instead of the standard font in the pickup window.

Can somebody advise? Is this a problem with the new window variables, or the WINDOW struct in general?

[BevapDin]

I don't see any IDs here. g->w_messages and the other w_ members are pointers, it's completely reasonable that after creating and destroying a window, the next window gets created on the very same memory as the first one and thereby having the same pointer value:

WINDOW *first = new WINDOW(); // newwin 
delete first; // delwin
// now the memory occupied by *first is free to be reused
WINDOW *second = new WINDOW();
if(first == second) {
    // This is a valid situation
} else {
    // this, too.
}

Btw. technically you can't even read a pointer after it has been deleted, so first == second in the example is actually invalid, or undefined behavior at best.

[SeanMirrsen]

I call them IDs because that's what I see them as. They're pointers in principle, but insofar as I'm referring to them by their variable name, I call them IDs.

And I would be very much fine with that example not working, but it does.

Yeah, I sort of suspected that was the reason. w_overmap and w_blackspace are deleted when you leave the overmap. (w_blackspace is actually deleted immediately after being created and drawn... there has to be a better way to blank a section of screen from outside the window-drawing functions) The only reason they're noticeable is because they're the two windows that have special drawing clauses that don't exist in memory when other windows are being created.

I'll work on a way to fix that. I'd consider adding an id parameter to WINDOW, but then I'd have to alter the window constructor calls to pass the id on to the windows. Maybe I can just keep those two in memory, and just werase them...

[kevingranade]

The pointer checking is intended to detect layout changes, and suppress the caching if it has changed, right? How about hooking into wrefresh() in wrefresh()->curses::draw_window()->font::drawwindow() to either set a flag or pass a parameter to tell drawwindow() to not use the cache?

[SeanMirrsen]

It's not just caching, curses_drawwindow checks against the window pointer to see what font to draw the window in, and whether to draw tiles. What happens is that g->w_overmap is assigned a pointer when the overmap window is created, but the pointer remains (as it's, what, just a memory address index?) when the window is deleted, and another window can end up in the same memory space, so the check against the g->w_overmap pointer and the win pointer passed to curses_drawwindow makes the function draw the window in the wrong font.

I'll make the changes to keep w_overmap and w_blackspace in memory, clearing them rather than deleting (who cares if an empty window remains in memory, it's never redrawn unless called on), as soon as my internet connection reacquires a semblance of decency.

[SeanMirrsen]

Okay, now it should be actually ready. There are no more holes that I can see, and no more bugs showed themselves after the last change.