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

Fixed map generate bug #75270

Merged
merged 2 commits into from
Jul 28, 2024
Merged

Conversation

PatrikLundell
Copy link
Contributor

@PatrikLundell PatrikLundell commented Jul 27, 2024

Summary

None

Purpose of change

Fix #75265, fix #75262, fix #75256, fix #75258, i.e. map featured generated again when they should remain unchanged, including generation of duplicate NPCs.

Describe the solution

  • Restore a lost negation (lost when optimizing the code). This is what caused the re-generation and duplicate characters.
  • Tracked down a bizarre failure of .emplace to work properly, where placement at at least one other index cleared one set earlier. This led to a crash on loading of my save after fixing the "real" bug. Replaced emplace with old school indexing (which the style enforcer probably will scream about).
  • Swapped a couple of boolean checks around for a negligible performance gain: normal game play should be the optimal path, with editmap stuff being allowed to cost more.

Describe alternatives you've considered

Testing

  • Before fixing the bugs:
    • Walked to a car and drove to a Mr Lapin (a lot of them have been generated all over the place), saw him in a single instance (I've not visited his place before), verified via the overmap (only a single character listed for that tile).
    • Drove away a bit and noted a levitating wasp nest roof (which is what causes the shadow in one bug report), and saved when 10 or so overmap tiles away from Mr. Lapin.
    • Loaded the save and drove to Mr. Lapin, and found two copies of him.
  • After fixing the bugs:
    • Repeating the above, without finding any levitating wasp nest roof (or any nest at all, for that matter). When returning to Mr. Lapin he hadn't been duplicated (again, verified it via the overmap).

Additional context

I don't like the 'emplace' failure one bit.
I tracked it down using debug output to debug.log, and that showed that generate emplace at begin+52 was set to true and there was indeed a submap at the corresponding coordinates.
However, 11 iterations later, when the data for grid_pos 21 was emplaced, the data at grid_pos 52 was suddenly false (grid_pos 21 corresponds to pos (1, 0, -10), so it's the start of the next stack).

@github-actions github-actions bot added Map / Mapgen Overmap, Mapgen, Map extras, Map display [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 Jul 27, 2024
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jul 27, 2024
@kevingranade kevingranade merged commit bfeb1ff into CleverRaven:master Jul 28, 2024
25 of 27 checks passed
@PatrikLundell PatrikLundell deleted the generate branch July 28, 2024 06:57
@akrieger
Copy link
Member

emplace inserts into a vector, [] = overwrites. If the indexing ever went 'backwards' then you'd shift the generated bits over by 1.

@akrieger
Copy link
Member

image

@PatrikLundell
Copy link
Contributor Author

Oh, that definitely explains it, as the indices jump based on how coordinates are translated into indices. I thought emplace was equivalent to [] but using approved "modern" syntax.

@akrieger
Copy link
Member

For 1:1 maps it behaves like you expect. For 1:many maps it will append.

@qtquazar
Copy link

I'm still getting some phantom wasp nests casting shadows from higher levels in game post-fix, although other map notes now seem accurate.

@PatrikLundell
Copy link
Contributor Author

The bug fix does not deal with past damages, so if the nest roofs were generated before this fix was applied they'd still remain, unfortunately. Thus, I hope this is a case of lingering damage rather than freshly produced issues.

@AlStrangelove
Copy link

AlStrangelove commented Jul 28, 2024

Hi, long time player, first time commenter. It might not be a problem with new games, but i've noticed a bunch of Mapgen issues with the random encounters since updating with this patch. Some examples are the wasp nest roof thing, as mentioned above (which i don't recall having encountered previously in this save), as well as several erroneous entries for other things such as supply drops and the like that don't show up where the autonotes put them.

@PatrikLundell
Copy link
Contributor Author

Yes, lingering issues are not addressed. Levitating roofs shouldn't cause any problems beyond being weird. I've never used auto notes, but my guess is that ones representing overwritten features can simply be deleted manually?

Really bad things would be if something overwrote something that's unique, but I don't think that can happen, because I think major features would just be overwritten by the same or similar features (like one shop replacing another), while things like wasp nests and supply drops are minor "decorations" that can be spawned on any field tile, so if the tile is generated again the feature may not show up (and things may show up where there wasn't anything previously).

@qtquazar
Copy link

The bug fix does not deal with past damages, so if the nest roofs were generated before this fix was applied they'd still remain, unfortunately. Thus, I hope this is a case of lingering damage rather than freshly produced issues.

If I understand you correctly: no, unfortunately, I just observed the wasp nest issue happening in a fresh game post fix.

@PatrikLundell
Copy link
Contributor Author

Are you sure it actually happened after the fix, rather than you finding it after the fix (because you did only get close enough to actually see it later)?
I just went of a fairly long road driving journey retreading known territory after having enabled auto notes, and failed to get any such notes or see anything that appeared to be newly generated.

@qtquazar
Copy link

Positive. I play a very difficult start with world wipes each time, and frequently update experimentals, so I cycle through a lot of worlds.

That said, the invisible floating wasp nest is the only issue of its kind I've run into, and only once now since the fix. Others have generated properly. I'll capture it if I see it again.

@PatrikLundell
Copy link
Contributor Author

PatrikLundell commented Jul 29, 2024

Unfortunately, I don't think capturing it helps. The problem is that all the info we'd have is that the info on that tile has been wiped and replaced, but nothing to indicate why or how.

I'll try to investigate further, but I'm not too optimistic about it at this stage.

Edit:
I've instrumented the code (locally on my computer) to print a message if a map extra (stuff like wasp nests that are added on top of some kind of basic terrain) exists when the code reaches the place where these are added. I also hacked the rate at which wasp nests spawn on fields to a very high number. After that, I loaded a compressed save (the type used for bug reports) and drove around for half an hour or so, looking for anomalies. A fair number were reported for terrain that had once been seen but then been "forgotten" by the save compression process. I drove back and forth among the myriad wasp nests generated, saved and reloaded (since that causes driving by things to load from file rather than memory) without either seeing any levitating roof or any report of trying to check for the generation of a map extra where there had previously been a wasp nest. I then drove to a "forgotten" known wasp nest and got the report that it had indeed been examined for a map extra anew. However the result was an open field crawling with wasps and wasp larvae, but no roof.
I then quit and loaded the "real" save and drove around for an additional fair amount of time, passing by the same wasp nest that had been "forgotten" in the compressed save, and it appeared normal. When I passed out into fields previously not seen new wasp nests appeared en masse, as expected.
Thus, so far I haven't seen anything indicating map extras being evaluated for placement when they shouldn't be. That's no proof, but it certainly doesn't provide any leads on what might cause trouble.

@qtquazar
Copy link

Thank you for the extra effort. I'll report if I see it again. Having been tech support in a past life, I fully appreciate one anecdotal event post-fix is not a good indicator of any kind of recurring problem :)

(However, given I used to be tech support, I am absolutely sure of what I saw, and that it was indeed post-fix, given I know how annoying this kind of report can be.)

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
5 participants