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 identical omts spawning over themselves #68324

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

akrieger
Copy link
Member

@akrieger akrieger commented Sep 25, 2023

Summary

Bugfixes "Fix overload of zeds and vehicles with older saves"

Purpose of change

Possibly fixes #67837. One fix led to another bug, which leads to this fix. There's a migration safety issue in mapgen where old saves serialized 'bad data' that was resolved at runtime. The runtime fixup was removed, but the saves were not migrated. This is an attempt to implement that save migration.

Describe the solution

I don't really know the exact terminology, but the laymans answer is things like roads were running mapgen 'over themselves' repeatedly. This resulted in things like dozen car pileups and hundreds of zeds in cities. The patch that introduced this (#67773) implemented logic at worldgen? mapgen? time which prevents these redundant roads being added to a 'predecessors' list. I ported that logic into the savegame deserializer to try to handle this at load time so older saves migrate forward successfully. The only thing I'm unsure of is the base condition of zero to 1 predecessors, but I'm assuming that any predecessor should be allowed compared to a base case of I'm guessing null terrain?

Describe alternatives you've considered

Testing

Created a save in a game compiled just prior to #67773. Debug map revealed and teleported to a city extremely far away. No problem.
Loaded that save on ##67773. Teleported to the same tile. Hundreds of zeds.
before

Applied my fix. Teleported to the tile. Not hundreds of zeds.

after

Roads don't have infinite predecessors anymore. A before comparison:

current_bad

Created a new world. Inspected 4 way intersections in cities. Saw there are still some that have a manhole omt as a predecessor. Manholes appear to not be linear, and adding the LINEAR flag is apparently mutually exclusive with NO_ROTATE, so that's not going to change. I don't know if this means city intersections have a higher chance of double spawns of somethings or not.
after

Additional context

0.G didn't have this redundant manhole under crossroad stuff. But JSONizing roads is radically different than what was before. No clue how to get there.

og_manhole

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Sep 25, 2023
@akrieger
Copy link
Member Author

I'm not sure this is a perfect fix. There's still larger pileups happening than should be expected. I'm not sure how far back to go to get to 'true legacy code' to compare against.

@akrieger akrieger force-pushed the redundancy_elimination branch from d39588f to 84b9403 Compare September 25, 2023 18:37
@akrieger
Copy link
Member Author

It looks like the behavior is intentional if I'm reading #67223 right, it just seems like spawn rates on roads in town are still elevated. I wonder if just removing REQUIRES_PREDECESSOR from the road json definition would resolve it, but I don't understand the implications of doing that. I will try it though.

@akrieger
Copy link
Member Author

akrieger commented Sep 27, 2023

I think I got it. I think the right answer is to not push a linear predecessor if the incoming type is itself linear, unless predecessors is empty. That way a road spawning over a field pushes the field predecessor, if the road is then refined to a more specialized road, it does not push the base road spawn as a predecessor.

@akrieger akrieger force-pushed the redundancy_elimination branch from 84b9403 to 820e036 Compare September 28, 2023 00:38
@github-actions github-actions bot added the Map / Mapgen Overmap, Mapgen, Map extras, Map display label Sep 28, 2023
@akrieger
Copy link
Member Author

Well I think this approach is plausible. What do people think?

@ZhilkinSerg ZhilkinSerg merged commit 608376f into CleverRaven:master Sep 28, 2023
26 checks passed
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Sep 28, 2023
@akrieger akrieger deleted the redundancy_elimination branch May 31, 2024 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions Map / Mapgen Overmap, Mapgen, Map extras, Map display
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Towns have too many zombies and wrecked vehicles
2 participants