Map 3d conversion WIP

[CIB]

Roadmap:

• Extend map to hold several z-levels at once, rather than just the current one
• Handle all z-levels in map::shift logic
• Update map API to support x/y/z instead of x/y coordinates in all accessors/functions
• Update caller chains of map API
• Optimize generation of pure sky/rock terrain

[BevapDin]

I think the comment is wrong, or at least misleading, NUMBER_Z_LAYERS is the total number of z-levels, it does not have to be symmetric, OVERMAP_DEPTH could 20 and OVERMAP_HEIGHT could be 4.

[CIB]

Sorry yes, that comment is from an earlier #define that I deleted. Forgot to remove the comment with it.

[BevapDin]

I think this (and the next line) should be done before calling shift_layer. Shifting triggers loading of new submaps which puts the vehicles of those new submaps into vehicle_list and updates the vehicle cache with the new vehicles.

Currently this does not do anything:
reset_vehicle_cache calls clear_vehicle_cache(), so the call here is not needed.
reset_vehicle_cache than iterates over vehicle_list which has just been cleared, so nothing happens here at all.

Also note the previous order of execution (before this commit):

clear_vehicle_cache();
vehicle_list.clear();
...
map::loadn
map::update_vehicle_list
...
reset_vehicle_cache

Now it's

map::loadn
map::update_vehicle_list
...
clear_vehicle_cache();
vehicle_list.clear();
reset_vehicle_cache

[CIB]

I'm.. not sure where that reset_vehicle_cache() comes from. What. Must be a copypaste error, I'm still getting used to vim.

[BevapDin]

reset_vehicle_cache Was already there in map::shift, it's the last line of that function.

vim

👍

[BevapDin]

You forgot to define MAPHEIGHT (-:

[BevapDin]

You're setting the absolute coordinates of the map in each call to this function, that means abs_sub ends up being completely wrong, set_abs_sub should be called exactly once from map::shift. You're calling it from map::shift, it should not be called here.

Also note that set_abs_sub is called in map::loadn when gridx==0 and gridy==0 (the submap at grid[0]). Now it's called several times (with gridx==0 and gridy) for each z-level.

Another thing: take a look at mapbuffer.cpp: mapbuffer::save clears all "unused" submaps. It considers submaps used when they are around the player. map_origin is the variable that defines the location of the top left submap in game::m, every submap this is outside of the main map is delete there. The check assumes that only submaps on the current z-level are used (it compares the z-coordinate of the submap with g->levz).

Result: when saving it will delete all submaps that are not on the current z-level. Save & quit should work fine as the submaps are not accessed anymore, but quicksave will lead to a crash.

[CIB]

You are amazing. I still don't know what you're saying, but it sounds like exactly the bug I've been hunting down for 3 hours today. I'll look into it tommorrow once I've had some rest.

[KA101]

Yeah, Kevin encountered something similar when we were working on digging up/down.

[Zireael07]

This is GREAT!

[CIB]

I spent several hours yesterday trying to figure out why my layers weren't aligned properly.. Until when I laid down in the evening and thought for a while, I realized that stairs aren't at all aligned between z-levels, and rather you just get teleported around if you take the staircase from evac shelter upper to evac shelter lower.

I think in the long run we'll want to fix this, as it creates all sorts of trouble(e.g. what happens if you dig down? How would this even work with looking down on the lower z-level, etc etc). However, for the time being, I'll simply ignore this and try to keep the teleport functionality working.

[acidia]

I know house basements are another culprit of this but if needed I can get a number of 'required' buildings jsonized with aligned stairs fairly quickly. I feel a bit disoriented by the way random house basements are handled but I'm not sure if other people would agree with basements being unique to houses so that they conform to the house floor-plan.

[CIB]

Well, we'd need some concept of connecting the stairways(i.e. mapgen X at rotation Y is only compatible with mapgen Z at rotation Y). I'd say that's something for the mapgen bounty, though, seeing as that bounty is huge anyway, and I guess problems like this are the reason.

[CIB]

Ironic, looking at the valgrind profiling right now, and the result is that apparently most of this doesn't have much of an impact. The only real issue is generating hundreds of layers of sky and rock every time you get to a new overmap tile.

[kevingranade]

Yes, making the stairs line up is a mapgen issue, and IS a big part of why that bounty is so big.

The more time I spend coding, and especially working on performance, the more I just trust the tools and don't worry about trying to figure out what will and won't have bad performance impact (that doesn't mean doing things in a dumb way, but it does mean implement things in a simple way and only complicate them if testing indicates that it's causing problems).

As for sky/rock, I think the best solution for that is having "platonic" sky/rock submaps that are shared by all the compatible locations.
Quick sketch: Add a "shared" flag to submaps. It's never set except on the special sky and rock submaps_. Anything that writes_ to a submap needs to check for that flag, if it's present, it needs to trigger generation of a new submap which is initially a copy of the shared map except the shared flag*. Then it needs to trigger swapping out the new submap for the shared map in both the active map*** and the mapbuffer. Then it can proceed to write to the new submap. Mapgen needs to grab a reference to shared submaps and stash them in the mapbuffer on generation of these terrain types instead of creating a new submap. Mapbuffer needs to have an override that saves/loads "this is a shared map of type bla" instead of writing/reading the whole data block

_Other shared submaps might exist, like "water", anything that's essentially featureless and we have a lot of.
*_Just as it sounds, this is what you call "copy on write".
***Any active map? getting hairy.

[kevingranade]

A note here, if this is invoked on a tinymap (which it pretty much always should be), my_MAPSIZE is going to be 2, so it is only going to be generating as many submaps as are needed.
Leaving the delete grid[... at the end is a bit of paranoia.

[CIB]

Generating is probably the wrong word, I mean allocating. I've actually profiled and with the new submap constructor/destructor, creating more submaps than needed has a significant performance impact.

[Snaaty]

Is there any way to contribute to the development of z-levels for me?
I know some rudimentary C++, but that's it.

[kevingranade]

I'm not aware of any simple tasks related to z-level development other than
making new 3-d content and possibly making things fall properly when they
end up on open space. The latter is complicated by the fact that the
reality bubble doesn't extend to other z levels yet.
Oh, I filed an issue about blood spattering on open space, that should be
fixable.

[CIB]

Sorry for the long silence, I was(and will be) busy trying not to fail exams. Anyway, I really thought somebody else would pick up this bounty if I don't deliver fast, but since that doesn't seem to be happening, I'll be working on it slowly as an on the side thing.

Just rebased work on top of latest master, seems to be working okayish. Only bug I got was a "trying to add monster at location where another monster already exists" bug, not sure if that's my fault, will investigate. Saving, loading, moving between z-levels etc. seems to work.

EDIT: Right, just remembered the bug I was dealing with when I decided to delay this and focus on studying instead. Vehicles poof whenever map::shift is invoked.

[CIB]

And that takes care of vehicles and traps. Everything seems to work fine now.

[Zireael07]

Woot! woot!

[KA101]

OK, I see that there are still points left, but I'm glad to see this move closer to the point where we break out the buckazoids. (I'd love to have an excuse to photoshop our faces onto the Briefcase-O-Cash scene from old-school X-COM. XD )

Anyway, I really thought somebody else would pick up this bounty if I don't deliver fast

DDA is CC-BY-SA, so someone could swoop in, finish building out this PR, and split the take with you. But the exams take priority.

[Snaaty]

Thanks for picking this up again, CIB!

[CIB]

I'm unsure how to proceed with shared submaps. Swapping out pointers in the "caller" map could be a real problem, i.e. there's currently tons of places that grab a submap pointer and keep it for the lifetime of the function.

I'd actually kinda like to have a workflow like this:

• Already know ahead of time whether you're going to modify the submap or not
• If yes, right at the beginning, convert it to a non-shared submap
• Do mass data access without expensive checks/indirection

So basically, "open map for reading" and "open map for writing", heh.

[kevingranade]

That way is potentially slightly faster, but way more fragile. How about safe and unsafe versions of the setters you can use when it matters, and code defaults to using the safe versions. Either way getters never care what kind of map it is.

[CIB]

So, use an extra layer of indirection, and have an accessor that bypasses that level of indirection(but you need to know what you're doing)? Would work I suppose.

[CIB]

Huh, I left this open? Whoops.