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

Fix the hidden lab basement mapgen #49230

Merged
merged 6 commits into from Jun 14, 2021
Merged

Conversation

eltank
Copy link
Contributor

@eltank eltank commented Jun 8, 2021

Summary

Bugfixes "Fix the hidden lab basement mapgen"

Purpose of change

Fixes #49106
Fixes #27618

As discussed in #49106 (comment) the hidden lab that's supposed to spawn occasionally under certain city buildings (with a basement entrance that requires a science ID to enter) stopped showing up in games after #39198 in April 2020. This soft-breaks one of the refugee center missions (the mission can still be completed due to another bug, as described in #49106).

Describe the solution

  1. Implement a map special that has a chance to replace the "basement" OMT (only found under "house_31" and "house_32") with "basement_hidden_lab_stairs". This is done by validating the randomly chosen location of the special by using a "connection" to point [0,0,-1] (1 tile underground) with terrain "basement" and "existing" bit set to true.

When trying to place specials the mapgen only tries 10 random points in each sector (15x15 map tile region), which means that a suitable location has ~4% chance to be randomly selected. Fortunately there are ~5-15 suitable locations per overmap, but the overall chance of finding one of them is still low (20-50%). For this reason I flagged the special as "UNIQUE" and gave it the maximum chance to spawn. There will be at most one instance per overmap.

Since the special modifies the same tile as the connection point [0,0,-1] and there was a C++ validation check asserting that special terrain doesn't overlap connection points, I relaxed that check for connections that have "existing=true" (since those connections aren't supposed to modify terrain, so devs should be allowed to overwrite them).

  1. Add basement stairs to house_31 and house_32. Right now the basements are inaccessible because at the time that the houses were added (houses for legacy basements #37939 February 2020) there was C++ code to generate stairs in these houses, so the house JSON was written without any stairs.

  2. Fix a couple of bugs in one of the 2 lab basement entrance layouts. The card reader had been replaced by a non-interactive console and the turret was sitting on a terrain tile inconsistent with the surroundings.

Describe alternatives you've considered

Change house_31 and/or house_32 to always have basement_hidden_lab_stairs under them; this would likely cause too many hidden labs to spawn. I'd like to limit the number of such labs to 1 per overmap.

Instead of tagging the special UNIQUE I experimented with "occurrences": [1,1] and "occurrences": [0,1]. The former is more-or-less equivalent (forces 1 per overmap if mapgen can find a location), but doesn't allow reducing the chance to spawn. The latter results in no spawns.

I also tried setting a connection point of [0,0,0] to either of the 2 house OMTs. But the connection would only let me match one terrain and I couldn't get it to work for both.

Testing

Modify the game JSON to make the map symbol for basement_hidden_lab_stairs stand out (e.g. "color": "i_pink"). Can also give it a searchable name (instead of "basement").

Generate a new world. Debug -> Reveal map. Open the map, go to z=-1 and look for the highlighted symbol. If found also check lower levels for lab tiles. If not found teleport to a different overmap and repeat or just generate a new world.

Also follow repro instructions from #49106 and verify that the 3rd mission from the Refugee Center Doctor actually sends you to a hidden lab with a basement entrance, that there's a console in the lab somewhere near the map marker and that interacting with the console gives you the data needed to complete the mission.

Additional context

If in the future someone decides to change the definitions of "house_31" and "house_32" and replace "basement" with something else this will break again. [Added some comments to guard against this]

Screenshots:
House 31
house31

House 32
house32

Lab basement 1
swipe
one_down

Lab basement 2
other_down

Non-lab basement under house 31/32:
normal_basement

Hmm, looks like the "normal" basement under house 31/32 has a rock floor, which is not consistent with other basements (concrete floor). Should probably fix that, but not in this PR.

@anothersimulacrum anothersimulacrum added <Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON labels Jun 9, 2021
@eltank eltank changed the title [CR] Fix the hidden lab basement mapgen [WIP] Fix the hidden lab basement mapgen Jun 9, 2021
@eltank
Copy link
Contributor Author

eltank commented Jun 9, 2021

This is working now. I just have one more issue to solve. One of the 2 layouts for "basement_hidden_lab_stairs" (from basement_lab_stairs.json) uses the "lab_palette" palette, in which "6" means "console", so the science card reader is replaced with a console that can't be interacted with.
The easiest solution would be to just remove that layout, but I kinda want to preserve it for variety's sake.
[fixed]

@eltank eltank changed the title [WIP] Fix the hidden lab basement mapgen Fix the hidden lab basement mapgen Jun 9, 2021
@eltank eltank marked this pull request as ready for review June 9, 2021 07:05
@Venera3
Copy link
Contributor

Venera3 commented Jun 9, 2021

It's a good solution for stable, at the very least. A few points:

How often do they spawn? There are no mandatory specials anymore, and I maintain the opinion that there shouldn't be (barring the number of map specials exploding, but that will probably take a while). The mission not being able to place it complicates that, of course, so I'd aim for every other/third overmap as a compromise.

How big of a mess do they make out of the subway system? generate_sub has a serious tunnel vision problem, so I could imagine some fun configurations.

Disabling the special validation check for overwriting the connection terrain disables a helpful debug message (for when you accidentally overwrite the connection), so do include that change in the overmap documentation - append something like "if true you can overwrite the connection itself" to the existing line.

The basement itself has "NO_ROTATE", did you check if the stairs line up in all possible rotations of the houses? I'd also include a comment in the overmap_terrain entry of the basement to warn against messing with it.

@actual-nh
Copy link
Contributor

If in the future someone decides to change the definitions of "house_31" and "house_32" and replace "basement" with something else this will break again.

Perhaps a CI test for this?

@eltank
Copy link
Contributor Author

eltank commented Jun 9, 2021

How often do they spawn? There are no mandatory specials anymore, and I maintain the opinion that there shouldn't be (barring the number of map specials exploding, but that will probably take a while). The mission not being able to place it complicates that, of course, so I'd aim for every other/third overmap as a compromise.

I don't have a benchmark, but anecdotally based on playtesting it seems to spawn approximately every other overmap. This can be tuned of course if it becomes a problem.

How big of a mess do they make out of the subway system? generate_sub has a serious tunnel vision problem, so I could imagine some fun configurations.

I haven't noticed any issues, it looks similar to how it used to be a couple of year ago when it was working normally. If there's any problem with the subway systems these days it's the abundance of the so-called micro labs. Here are some screenshots of the subway layers (-2 and -4) in an overmap that includes a hidden basement lab:
z2
z4

Disabling the special validation check for overwriting the connection terrain disables a helpful debug message (for when you accidentally overwrite the connection), so do include that change in the overmap documentation - append something like "if true you can overwrite the connection itself" to the existing line.

By "overmap documentation" do you mean OVERMAP.md? The structs in omdata.h have no existing documentation/comments that I could find.
I'm not sure what you're referring to when you say "the existing line".

The basement itself has "NO_ROTATE", did you check if the stairs line up in all possible rotations of the houses?

I did not make any attempt to line up the stairs, it's impossible to make that work for all possible rotations of the house. But the game plays just fine without stair alignment. BTW, subway stations have the same issue. The z=0 map gets rotated while z=-1 does not so the stairs don't line up most of the time. But then neither do z=-1 and -2 and those are both fixed.

I'd also include a comment in the overmap_terrain entry of the basement to warn against messing with it.

I added comments to the city_building definitions of house 31 and 32. It's fine to mess with the "basement" mapgen, they're just normal basements.

@eltank
Copy link
Contributor Author

eltank commented Jun 10, 2021

If in the future someone decides to change the definitions of "house_31" and "house_32" and replace "basement" with something else this will break again.

Perhaps a CI test for this?

I was thinking it would be nice to have a test that generates 3x3=9 overmaps and checks for the presence of all map specials needed by quests. But that seems non-trivial to implement, tricky to maintain (the list of quest-related specials needs to be updated manually) and may be flaky due to RNG.

@Venera3
Copy link
Contributor

Venera3 commented Jun 10, 2021

The test isn't really necessary imho - the only missions that can't actually spawn their destinations are the old hardcoded ones that should be deprecated one of these days, and labs themselves will probably see some major changes in the coming dev cycle.

The subway looks fine on the overmap, and any map-scale overlaps could happen with other labs as well, so all's well. They might have a bit of a problem generating train depots, but such is life.

Either OVERMAP.md or the guide to beginning/intermediate mapgen has a section about the connections array.

I'll try to whip up some tests to get at a large sample size. The microlab/normal lab ratio is on purpose, old-style labs are horrible from a game balance standpoint (that's also the reason they got rare), but there should be a way for you to get at the midgame sciencey goodies.

@eltank
Copy link
Contributor Author

eltank commented Jun 11, 2021

Either OVERMAP.md or the guide to beginning/intermediate mapgen has a section about the connections array.

I've added some more information to OVERMAP.md, the only md file that talks about overmap connections in any detail (I checked all of them).

@ZhilkinSerg ZhilkinSerg merged commit e2ef8b8 into CleverRaven:master Jun 14, 2021
@eltank eltank deleted the hiddenlab branch June 15, 2021 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON
Projects
None yet
5 participants