Change overmap reveal to depend on LOS more than light level?

[sparr]

During the daytime, the overmap reveals in a radius around the player, including being able to see things deep in the forest. This seems like too much. Maybe we could use LOS calculations, and declare that certain overmap types block LOS, like normal map tiles do?

At night, as soon as you step on a single square of a new submap, the overmap tells you what kind of submap it is. This seems like too much. Maybe you should have to reach the middle X% of the submap to have it revealed, or have LOS to more than Y% of it?

At night, when you have a flashlight or headlights illuminating some, most, or even all of a submap, it doesn't get revealed on the overmap until you step onto it. This seems like too little. If I drive around at night with a vehicle covered in headlights, I should get the overmap revealed one or two tiles around me, as the headlights reach.

I can probably implement this; I'm looking for feedback, balance concerns, reasons it's done the way it is, etc.

[BevapDin]

Maybe we could use LOS calculations, and declare that certain overmap types block LOS, like normal map tiles do?

Look at player::overmap_los, for example.

The remaining issues are valid.

[sparr]

aha, oter_t.see_cost limits visibility on the overmap. thanks for pointing that out. if I make changes, I'll be sure to accommodate that.

[KA101]

Tough part with getting a square mapped is that sometimes the player can tell on seeing it. I see a small fenced area, probably an automated gas station. Wooden fence? I'm gonna go with house. Barbed wire? OK, that's either a farm or a FEMA, depending on whether there are zeds around.

So I can understand the desire to be a little less generous; conversely, I'm personally in favor of erring on the side of giving the info rather than hiding it. (Areas that are secret within DDA's world, such as the Necropolis, are another story.)

[sparr]

@KA101 this issue was strongly inspired by not getting enough info at night with a flashlight/headlight. I plan to give far more there than I take away in the other case(s).

[KA101]

The map-ranging differential during the day is in part to compensate for the limited range of the reality bubble (roughly: "what you can see on the minimap" is the farthest possible to see in the main-game window, given unlimited monitor size). Flashlights/headlights should only give about one tile's worth of map radius, maybe two at absolute most, I'd think.

And I'm subscribed to the repo, no need to ping me. ;-)

[sparr]

I was contemplating actually calculating LOS to visible tiles in the submap, but a simpler way would be to just give one or tiles of vision when a light of sufficient range is turned on. That might be acceptably simplified.

[KA101]

Yeah. IRL you can see much farther than the main game window can show--that's why guns feel rather short-ranged--so relying on actually-visible would nerf daylight recon way harder than it should. One tile-radius (mapping your map square and the adjacent ones) of mapping for (say) 10-20 consecutive turns of having a light on makes sense at night.

Turn-count is so people don't complain that the PC sees that whole area in 6 seconds.

[sparr]

Am I missing something or do game::update_overmap_seen() and player::overmap_los() do mostly the same job?

[sparr]

Just revealing adjacent submaps after you've had a light on for some turns seems overly simplistic. I could see maybe falling back to that, but I'd like to try to be smarter about it if possible.

I think that what I want to do is hook on to the end of map::build_seen_cache() and calculated how much of each nearby submap is lit. Then I could reveal highly-seen submaps on the overmap, or possibly keep a tally of how long I've seen them to assuage your flashlighton-flashlightoff concern. However, I'm having trouble figuring out all the function calls and math necessary to do all the coordinate conversion, to figure out which entries in the seen_cache correspond to which submaps.

[BevapDin]

You don't want submaps, you want overmap terrain coordinates because you want to make overmap terrain visible.

// Local map squares, e.g. suitable for g->m.ter_at() but invalid after a map shift
// Almost all map functions use this.
const auto pos = g->u.pos();
// Same coordinates system, but absolute (not relative to the location of the main map) and therefor global coordinates.
const auto abs_pos = g->m.getabs( pos );
// Overmap terrain coordinates (still absolute)
const auto abs_omt = overmapbuffer::sm_to_omt_copy( abs_pos );
// Overmap terrain at that place e.g. house, road, lab, crater, etc.
// Needs a z-level!
const auto &omt = overmap_buffer.ter( abs_omt.x, abs_omt.y, g->get_levz() );

Coordinate systems are explained in overmapbuffer.h

[sparr]

@BevapDin I'm confused. Isn't one character on the overmap equal to one submap?

Also, the problem is that I don't want to call those functions thousands of times. I've got seen_cache[][] which is a boolean array of squares the player can see, in g->u.pos() coordinate system. I don't just need to convert a single position to omt coords. I need a quick arithmetical approach to figuring out which blocks of coordinates in seen_cache correspond to which omt coords.

I'm going to see how far down the rabbit hole I can get by following the functions you've mentioned. As someone new to the source, I can say that the frequency with which we use auto around here makes it really hard to figure this stuff out. If I knew the type of pos, I'd be able to find m.getabs() because it takes that type. If I knew the type of abs_pos, I'd be able to find sm_to_omt_copy because it takes that type. etc.

[sparr]

It looks like I was on the wrong track. seen_cache is not the final arbiter of what the player can see, being true for a large number of tiles that the player can't actually see at night.

It looks like the logic I need might actually be in map::draw()? In which case, I'm going to need to break that out, run it for a larger area than the current view window, and [re]use those results when actually drawing the map, to avoid doing it all twice.

[kevingranade]

My first guess at that specific error is you need to '''#include "overmapbuffer.h"''' I'm not a fan of a lot of the auto either, some people seem to think it's the best thing ever, but all the arguments for using it everywhere I've seen have completely ignored trying to read a foreign codebase.

[sparr]

I'm also seeing a lot of duplicate logic between map::draw() and cata_tiles::light_at(), which is the same logic that I need for what I'm trying to do here. That definitely looks like a candidate to de-duplicate. I guess I need to do that first, before I continue trying to tackle this idea.

[kevingranade]

run for an area larger than the current view window.

This is not going to work, submap data outside that area isn't even in memory. Something you might be able to do is augment the overmap cell data with information about the submaps that make it up once they're generated (possibly even generating them early, shouldn't be that big a deal). In general, if you're operating at overmap scale, you're going to want to operate on overmap data, it's simply the way the game is structured, and ignoring that is going to be painful. Answering an earlier question, each overmap tile is made up of 4 submaps.

[sparr]

I thought all submap data in the reality bubble would be in memory? I have to operate at both scales, because I care which squares are visible.

[kevingranade]

Yes, but the reality bubble is precisely how far you can see in the regular view, which only extends about 2-3 squares (radius) on the overmap.

[sparr]

map::draw() only calculates for the squares that fit in the current UI window, even if you're playing in 80x24 and that's just ~50x20 squares.

getting the data calculated for the whole reality bubble (11x11 submaps, 132x132 squares, right?) is what I'm talking about doing.

[kevingranade]

Ok, I'm not sure what you're doing then, identifying overmap squares based on dynamic lighting and LOS?

[BevapDin]

Isn't one character on the overmap equal to one submap?

Nope, that's why you should read the text in overmapbuffer.h. If that text does not explain it all, ask and add the missing information there, so the next reader gets it.

I need a quick arithmetical approach to figuring out which blocks of coordinates in seen_cache correspond to which omt coords.

Read the documentation. Quick and dirty: each overmap terrain is (SEEX_2)x(SEEY_2) map squares big.

I can say that the frequency with which we use auto around here makes it really hard to figure this stuff out.

This is not about auto, the type of a position is always a point (or a tripoint, but that's the same thing only with additional data, it should not behave differently). The "X_to_Y" functions of the overmapbuffer preserve the dimension of the input point: they return a point if the input is a point and return a tripoint if the input is a tripoint.

If I knew the type of pos, I'd be able to find m.getabs() because it takes that type.

Uhm, does your editor not have a "search in file" function? There are three versions of getabs, they all do the very same, only taking its input in different formats, but calling getabs(x,y) is exactly the same as getabs(point(x,y)). Search for the function name. The type of the input is completely irrelavnt, it's only for simpler usage that it support a tripoint:

const tripoint pos = ...
const auto abs_pos = m.getabs( pos );
// even shorter, don't use a temporary pos
const auto abs_pos = m.getabs( ... );
// Same as, but this is obviousel not very readable and way to long:
const point abs_pos_without_z = m.getabs( pos.x, pos.y );
const tripoint abs_pos( abs_pos_without_z.x, abs_pos_without_z.y, pos.z );

[sparr]

@BevapDin "search in file" doesn't help if I don't know the name of the function I'm searching for. If, however, I knew the type of your pos then I could find functions in map that take that type as a parameter, and getabs would have been on that list. This is a spot I've gotten stuck a number of times in the cata source, when I am looking at an object and I know I need a function that does something to/with that object, but it's declared as type auto so I don't know what it is.

[sparr]

@kevingranade precisely

However, it looks like #11790 is in the way, unless I want to do this all twice. So I'm gonna pursue fixing that first.

[sparr]

Nope, that's why you should read the text in overmapbuffer.h. If that text does not explain it all, ask and add the missing information there, so the next reader gets it.

Thanks for the pointer. It looks like the comments in overmapbuffer.h answer the questions I have on that front.

[BevapDin]

If, however, I knew the type of your pos then I could find functions in map that take that type as a parameter, and getabs would have been on that list

You know how many function take a point as parameter, nearly all of the map functions do. Do you really want to look of all of them, how does that help you? And even if you knew it's a point (and not a tripoint), there are now overloads for many of them, so you would have nearly the same list for tripoints as for points. And now look at all the functions outside of the map class that also take a point/tripoint.

I think, you're going at this from the wrong side. The point/tripoint classes are simple storage classes, they have no meaning on their own. They can store any kind of point. They could store your position, or the position of something else on the overmap/the map/a submap/etc. in any coordinate system they like. They could store the location of a letter on the screen (in the curses interface), completely disconnected from the game world itself. They could store the pixel location of something on the screen etc. Meaning is only introduced when their values get used in some way.

---

auto is deductible (the compiler does it, so you can do it, too):

const auto pos = g->u.global_square_location();
const auto abs_pos = m.getabs( pos );

Look up what the function returns, there is only one global_square_location in the player class and it returns a tripoint, therefor pos is a tripoint. Sometimes there are overloaded versions with different parameters (getabs), look which one matches the call here and, again, you'll see what that function returns - getabs(tripoint) returns a tripoint.

Also, you can safely assume that "pos" or similarly named variables are either a point or tripoint, what else could they be?

[sparr]

I'd rather not continue discussion in this thread about coding practices and deduction. I'll be over in #11790 trying to de-duplicate and make consistent the "is this square visible" code, then back here to get my overmap LOS idea working once that's fixed.

[sparr]

OK, #11790 is coming along. Now, back to this idea. To make this work, I'm going to need to check the visibility of all the squares in the reality bubble. For users with tiny fonts, huge screens, or zoomed out tiles, that check is already getting done in map::draw() or cata_tiles::draw(). For everyone else, some of it is already getting done.

I don't want to do it all twice. I'm thinking that what I need to do is make a lit_level visibility_cache[MAPSIZE*SEEX][MAPSIZE*SEEY] and populate it, then have *::draw() as well as my overmap visibility checker refer to that. How does that sound?

[sparr]

visibility_cache would be about 75kB, assuming the enum gets packed as a 4-byte int

[sparr]

I guess visibility_cache could be bools, like seen_cache. I wonder if it could re-use seen_cache...

nope, that makes it unusable for the normal map render, since that uses all the values.

[sparr]

What if I abstract the x/y loop out of the draw functions, so there's just one in map, and loop it across the whole reality bubble instead of the screen so I can do the overmap LOS update there, and give that a callback to a function in map or cata_tiles that actually draws the things that fit on the screen?

[BevapDin]

Actually, as you bought it up, what do you not like about seen_cache? It's content is directly exposed via map::pl_sees, which in turn is used in Creature::sees to determine whether the player can see a specific location. It's also used in map::draw to determine the visibility (or it's at last part of that decision).

[sparr]

@BevapDin it's just part of the decision. pl_sees and seen_cache handle line of sight and blocking terrain and light sources. they don't handle the player's sight range, time of day, blindness, boomer, etc. I need to work at a point after all that's been calculated, which currently happens in ::draw()

[sparr]

@BevapDin in your example code above I think there's a step missing?

const auto pos = g->u.pos(); // squares
const auto abs_pos = g->m.getabs( pos ); // squares?
const auto abs_omt = overmapbuffer::sm_to_omt_copy( abs_pos ); // omt coords

where did the squares coordinate get converted to a submap coordinate?

[sparr]

I'm at a point where I have submap coordinates ranging from 0,0 to MAPSIZE-1,MAPSIZE-1. From what I have found so far it looks like I need to convert that relative-submap coord to a relative-square coord, then that to an absolute-square coord, then that to an omt coord?

[BevapDin]

You found the bug, that intentionally (really, absolutely not by accident) left in (-:

It should be ms_to_omt_copy, "ms" as in "map squares", that's why I need to test more.

[sparr]

Here's what I have so far:

        if ( sm_squares_seen[x][y] > 0 ) {
            const point pos(x*SEEX,y*SEEY);
            const auto abs_pos = g->m.getabs(pos);
            const auto abs_omt = overmapbuffer::ms_to_omt_copy( abs_pos );
            set_seen( abs_omt.x, abs_omt.y, g->get_levz(), true);
        }

[BevapDin]

const point pos(x_SEEX,y_SEEY);

Why this? x, y are local map squares, there is no more detailed coordinate system than that. A point(x_SEEX,y_SEEY) indicates there are other points in the range ((x-1)_SEEX,(y-1)_SEEX). e.g if x=1 and y=2, pos would be (12, 24), next iteration (y=3), pos would be (12, 36), what about the point between?

[sparr]

In that context, x and y are submaps. sm_squares_seen is an 11x11 array of ints counting how many squares in each submap can be seen. I need to turn relative submap coordinates into relative square coordinates so that I can use getabs and then ms_to_omt on them.

The code is actually working. Should I submit a PR so we can discuss the whole change instead of little snippets, even though it depends on another PR that hasn't landed yet?

[BevapDin]

In that context, x and y are submaps.

Then the code is fine.

However, consider using overmapbuffer::sm_to_ms_copy instead of that "* SEEX", using the function makes it clear why this is done (to convert the point into another coordinate system).

Btw., you can bypass the conversion to map square coordinate system: You already have (relative) submap coordinates, just add map::abs_sub to them and it will be absolute submap coordinates. abs_sub is the submap coordinate of the upper left submap in the reality bubble, that is the submap with the relative coordinates (0,0).

[sparr]

ahh, being able to add map::abs_sub was a trick I didn't see. thanks again.

[sparr]

Driving around and having the overmap revealed ahead of me as my headlights illuminate things... so wonderful :)