-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 mapgen special build errors #77468
Conversation
data/mods/TEST_DATA/overmap_terrain_coverage_test/overmap_terrain_coverage_whitelist.json
Outdated
Show resolved
Hide resolved
Assuming this passes the test you'll want to update the summary notes. |
Why are you removing the existing road requirement? These are supposed to place on preexisting roads to look more natural, it doesn't make sense to have a motel at the end of a long road to nowhere, they also have priority -1 to ensure they place after all the other roads are in |
It's a different idea that @Maleclypse had for the same issue. My personal concern is to get the mapgen to a place where it can reliably run. I don't have a strong opinion on this solution. |
I don't really see why a 6 month old change needs a bandaid fix, I've just opened up a new world to check nothing derped recently and in 3 OMs there's 3 successful wind turbine spawns and 6 varying motel spawns, with at least 1 of each. Either the test fail wants changing so it doesn't just accumulate failed minimum occurrence attempts regardless of successful spawns in between resulting in it failing over large areas unless I've misremembered how it works or the special placement code wants improving to be more reliable. I also have no clue why you think this would speed up mapgen |
Agreed, I don't think this solution will speed up mapgen. I've removed that |
Closing for now. Happy to reopen if there is interest in the future. |
Summary
Bugfixes "fix mapgen special build errors"
Purpose of change
Fix build
Describe the solution
Removing existing road requirements from new map specials
Describe alternatives you've considered
Set the minimum occurrences of these non-unique map specials:
...to zero as stated in the relevant doc. See https://github.com/CleverRaven/Cataclysm-DDA/blob/master/doc/OVERMAP.md
Add related terrains to overmap_terrain_coverage_whitelist
Testing
WIP
Additional context
Thanks to @Maleclypse for the idea