Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(port): mutable specials and unhardcored labs and anthills #3520

Merged
merged 67 commits into from
Nov 13, 2023

Conversation

Vollch
Copy link
Contributor

@Vollch Vollch commented Oct 28, 2023

Summary

SUMMARY: Infrastructure "Ported mutable specials from DDA, and unhardcored labs and anthills"

Purpose of change

Allows porting mutable specials from DDA, adding new ones, and also solves bunch of issues caused by hardcoded anthills\labs, such as type mismatch or building over subways.

Describe the solution

This PR contains both ported things, and new one. As i'm not fully satisfied with current DDA implementation to leave original changes as is.
First, ported PRs:

What was made on top of that:

  • Mapgens of anthills and lab was changed to intersect where those two specials meets, imitating ants labs. Restored "Lab with Anthill".
  • Mutable crater is cut off in favor of old blob. Two reasons: mutable one looks more bland, and its only distinguish feature - explosions - does not supported by other parts of BN code anyway.
  • Changed specials IDs reverted to original. No need to multiply legacy and clutter code without necessity, plus i fixed bug with copy-from'ing mutables, so they're more forgiving to mods trying to change anthills.
  • Mutables should be fully compatible with current DDA implementation, and on top of that i added few new options to JSONs which i needed to unhardcode labs. All those things are listed in documentation.
  • Added ability to nesting specials, pretty much like nested mapgens does.
  • Fully unhardcoded all labs. Central and Ice ones are individual specials, regular lab implemented as 1x1 surface specials + nested specials for basement. It does that to also spawn nested underground labs under research facility and prison.
  • Implemented p2p type of overmap connections, which links specials without hooking everything to city centers. Currently used by subways.
  • Last three(pre-review) commits are not directly related to mutables, that's more tweaks for general specials placement rework. Adding spacing option, and improving distribution. But It's built upon code overhauled by mutables, so i'll throw it here.

Describe alternatives you've considered

Specials should get native nesting support to fully get rid of hardcode in labs. Done.
Connections could be improved to support different layouts on top of hooking everything to city positions. Done.
Rules of when central lab generate subways z-4 might need to be tweaked, as current implementation doesn't fully replicated old nuanced behavior.

Testing

Generated bunch of overmaps, ran mutable generation and endgame spawning tests, looks stable.

Additional context

Nothing to showcase really. Most of efforts were focused on making it look as similar to current labs\anthills as possible.

Vollch and others added 30 commits October 26, 2023 03:39
Previously this would fail with an error which didn't explain where the
problem was.
Linear terrain (roads, ant tunnels, sewers, etc.) was not previously
possible to define in JSON.  This was because the rotation wasn't
correctly calculated for linear terrain.  Make it be.
Co-authored-by: John Bytheway <[email protected]>
When placement of mutable overmap specials fails, it's hard to figure
out why.  The small amount of context previously given in the debugmsg
wasn't enough.  I did have some further debug printfs in non-compiled
code behind a #if, but that's no use to people who can't compile the
game themselves, nor does it help debug failures in CI.

Reformulate the code so that these more detailed messages are always
saved (rather than printed) and when a placement error occurs, it
includes a full history of the placement to allow for debugging.
I think I figured out why mutable special placement was sometimes
failing.  Some joins could be postponed but others could appear
pointing at the same tile and be satisfied in the absence of the
postponed joins.  Then, when the postponed joins are restored they
cannot be satisfied.

Avoid this by keeping track of the tiles with postponed joins and
ensuring that they new joins pointing at the same tiles are added
directly to the postponed list rather than the unresolved list.
The previous attempt to fix the rare anthill placement errors did not
work.  I now know this was due to a simple typo, but tracking down the
issue took a lot of refactoring.  That refactoring improved the code, so
I'm including it here.

Changes include:
* The previous fix was that new unresolved joins that might conflict
  with postponed joins would themselves be automatically postponed.  We
  have now reversed this logic so the postponed joins are restored.
  This allows another chance for the current phase to satisfy the joins,
  now that might be possible.
* Postponed joins now need to have their positions indexed for efficient
  access, so I factored out that indexing capability into a new struct
  indexed_joins.
* Removed the list of orphaned joins.  They should no longer ever occur.
* Added a bunch of calls to a consistency_check function that verifies
  the class invariants (except it's disabled for now, with the code left
  in case it's useful for future debugging).
* The order of unresolved joins used to depend on their addresses in
  memory, which made the tests non-reproducible.  Change that to be
  deterministic.
* Add unit test which places ~100k anthills to more easily observe
  placement errors.
* Improve the debugging output for failed placements to make it easier
  to see what happened (add phase boundary markers and capitalize
  FAILED).
Rather than always having a fixed number of overmaps for a particular
rule during placement, allow a random number.  Currently this can only
be a poisson-distributed number (or a fixed value, as was already
supported).
Explain how to specify that the max number of instances should be drawn
from a Poisson distribution.
To add more flexibility to the layout of mutable specials it is
convenient to allow some joins for particular overmaps to be optional.
This helps in particular with the final phase when you want to clean up
all the leftover unresolved joins.

Generalize the definition of joins to allow this in the JSON, and handle
appropriately in the code.

Convert craters to use this feature.

Extend the unit tests to spawn a test crater to exercise this feature.
Sometimes it's important for the two sides of a join to be different,
for example a hallway_to_room join should not join two hallways.  It is
also be important if the set of locations is different for the two
sides.

Add support for this type of asymmetry, and use it to allow anthills to
have multiple entrances.

Update test anthill also.
The Windows stack size limit is smaller than Linux, so need to tweak
this test to avoid a stack overflow.
To be able to use these types in regular mapgen they need to be promoted
to headers.
When a mutable special is placed, keep a record of every join used and
store that in the overmap.  Display this information in the overmap
editor to aid with debugging.
Similarly to neighbourhood checks in chunk placement, add join checks to
chunk placement.
* Acid Anthills

* Update ants.json

* Update ants.json
When placement fails for a mutable overmap special, add a note to the
overmap with some details to assist debugging.
There used to be a function called direction_XY that converted a
direction to a point.  This didn't match the naming scheme used for
analogous functions for om_direction and cube_direction.

Rename it to displace_XY and also create a tripoint version displace.
Previously the mapgen feature of placing chunks based on neighbours used
the absolute cardinal directions on the map.  This is unhelpful when the
map is rotated.

Rotate the neighbours to match the rotation of the map so that these
checks make sense for the mapgen.
For symmetry with the - overload.
Previously these checks were based on absolute map directions.  After
this change they are instead rotated with the map, so that they are
easier to use logically within the map.
It can be useful to know the rotation of a particular tile for debugging
purposes.  This may not be obvious for linear terrain.  Add it to the
debugging output in the overmap editor.
It will help a lot with mutable overmap special design if a rule can
place multiple overmaps together in a chunk.

This attempts to implement that.  It entails a significant rewrite of
the guts of overmap special placement.

Add a test_microlab special to test this new feature.
src/mapgen.cpp Outdated
Comment on lines 1829 to 1836
/* TODO: Throws errors on central lab depot, presumably due to loading order
for( const std::set<oter_str_id> &p : neighbors ) {
for( const oter_str_id &id : p ) {
if( !id.is_valid() ) {
debugmsg( "Invalid oter_str_id '%s' in %s", id.str(), oter_name );
}
}
}*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite the load order issue; this check happens at the "check consistency" stage and by then all JSONs have been loaded and all copy-froms have been resolved.

This legitimately triggers on subway because subway is not an overmap terrain, but rather overmap terrain type. The overmap terrains belonging to this type will have suffixes like _ns, _esw, _nesw, etc.

The issue comes from how this class misuses oter_str_id in the first place. In the test method below, it doesn't use the id as an id, it casts it back to std::string for pattern matching, which voids the whole reason behind having it be typesafe id. This should either all be std::strings, or be switched to oter_type_str_id and the code in test should check if terrain belongs to this type (e.g. by passing ot_match_type::type to is_ot_match).

Copy link
Contributor Author

@Vollch Vollch Nov 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. That's actually my derp, original PR changed oter_str_id to oter_type_str_id, but mapgen.cpp have so many conflict all around(due do missing parametric mapgens) that it slipped by me during resolving. And the fact that DDA eventually removed that check() hinted me in wrong direction. I just confirmed it works at all(thanks to test()'s string casting) and called it a day :)

Copy link
Contributor Author

@Vollch Vollch Nov 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out that it can't really check for ot_match_type::type - It breaks oter checks in central labs, as they share same mapgens with regular labs, but have different types

Copy link
Contributor

@olanti-p olanti-p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestions here are mostly me hunting for typos and opportunities to elaborate on the documentation.

From the testing, the system is quite robust, the unique specials like the refugee center or the endgame lab are placed even with wacky density/spacing settings, so people running with 9999 mods should be fine.

The subways have changed somewhat, on z=-4 the labs often can't quite connect to each other, but other times it properly generates the usual "central lab <-> secondary labs" network. There's also that single rail line that ends up in a dead end under a random city. But then again, subway gen has many other glaring issues (tunnels opening into lake, rails not matching on diagonals, sharp turns making vehicles ram the walls...)

Something's weird seems to be going on with the lake specials, on default settings and on x5 density I get maybe a lighthouse and a marine on overmaps with substantial lakes.
image

doc/src/content/docs/en/mod/json/reference/json_flags.md Outdated Show resolved Hide resolved
src/overmap.cpp Outdated Show resolved Hide resolved
src/overmap.cpp Outdated Show resolved Hide resolved
Comment on lines +74 to +75
std::poisson_distribution<int> dist( mean );
return string_format( "Poisson(%.0f)", dist.mean() );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Suggested change
std::poisson_distribution<int> dist( mean );
return string_format( "Poisson(%.0f)", dist.mean() );
return string_format( "Poisson(%.0f)", mean );

Copy link
Contributor Author

@Vollch Vollch Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::poisson_distribution::mean
double mean() const;
https://en.cppreference.com/w/cpp/numeric/random/poisson_distribution/mean

data/json/overmap/overmap_mutable/lab.json Outdated Show resolved Hide resolved
doc/src/content/docs/en/mod/json/reference/map/overmap.md Outdated Show resolved Hide resolved
doc/src/content/docs/en/mod/json/reference/map/overmap.md Outdated Show resolved Hide resolved
doc/src/content/docs/en/mod/json/reference/map/overmap.md Outdated Show resolved Hide resolved
doc/src/content/docs/en/mod/json/reference/map/overmap.md Outdated Show resolved Hide resolved
@Vollch
Copy link
Contributor Author

Vollch commented Nov 8, 2023

Something's weird seems to be going on with the lake specials

lakes always was pretty empty, and normalization to lake\land ratio reduced occurrences even lower. Plus, lake specials for some weird reason depends on cities, even islands. That restricts their placement, and even more with current distribution of specials between different cities: at x5 density 0-1 lighthouse becomes 0-5, but then there's 5+ cities on overmap, and mapgen tries to put one lighthouse per city, yet not all cities have a lake around them to actually place it.
Lake things could be hardcoded to ignore that, but right way to do that is via json: increase occurrences, and untie from cities. "Luckily" we have just a few of lake specials, so rebalancing it won't be difficult. I have some drafts already with json tweaks, but didn't planned to touch it in this PR.

@Vollch
Copy link
Contributor Author

Vollch commented Nov 8, 2023

As for subways. z-4 central labs are intendedly allowed to spawn with no depot. It can be changed to json side, if needed. Like i mentioned old connection rules was pretty nuanced, and while it's possible to replicate it'd be pretty messy - involving om_pos checks on all regular labs, and i'm not really sure whether i need to port all those convoluted nuances or not. For now it just spawns "poisson": 1.5 depots, which can end with zero. But could be changed to e.g. flat 1 plus poisson(0.5). Or just flat 1. Or [1,3] range, whatever.

For subways in general, those dead ends due to how subways linked to each others. Specials S1 and S2 links to city C1, then S3 links to C2, then C1 links to C2. Now we have everything in one global network, but those C1 and C2 may have a dead ends if they reused old tracks. I'm not perfectly happy with that solution, but it just works, and i didn't came up with something more elegant so far.
S1, S2, S3 are specials. C2 point is seamlessly mended in network. C1 is dead end:
image

Edit: Finally got my hands to rework subways. No more dead ends.

@Vollch
Copy link
Contributor Author

Vollch commented Nov 8, 2023

Regarding disconnected things. Well, it shouldn't happen, obviously :) Rechecking generation i noticed that i missed one old connector under city mall, fixed now. And stumbled on microlabs spawned right under city center(c1 and c2 points as per pic above), preventing to link there - this one also should get better, by reusing road's fallback for no cities. But still no guarantees, due to limited search range.
I guess i could add a check to overmap::can_place_special() to prevent placing anything under city centers, but still undecided - in grand total that'll be a lot of checks on course of generation, and i'm a bit worried about performance tax. Or maybe changing oters under city centers would do the same with no extra cost. Or maybe i'll eventually rework subways to not hook to cities, and this issue will be obsoleted.

Edit: ...and yeah. This problem is gone now, after adding new connections type for subways.

@Vollch
Copy link
Contributor Author

Vollch commented Nov 8, 2023

And also fixed one of those old "glaring issues" with rails connections. Chunks should check for actual connection, not just any subway.
Before:
image

After:
image

Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshots

Cataclysm: Bright Nights - 20176b75a143_01
Cataclysm: Bright Nights - 20176b75a143_02
Cataclysm: Bright Nights - 20176b75a143_03
Cataclysm: Bright Nights - 20176b75a143_04
Cataclysm: Bright Nights - 20176b75a143_05
Cataclysm: Bright Nights - 20176b75a143_06
Cataclysm: Bright Nights - 20176b75a143_07

@scarf005 scarf005 added this pull request to the merge queue Nov 12, 2023
Merged via the queue into cataclysmbnteam:upload with commit 07735ca Nov 13, 2023
13 of 17 checks passed
@Vollch Vollch deleted the mutable_specials branch November 25, 2023 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING-JSON-CHANGES Changes to JSON data format or features JSON related to game datas in JSON format. mods PR changes related to mods. src changes related to source code. tests changes related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants