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

Set new_game before loading JSON data #71806

Merged
merged 1 commit into from
Feb 18, 2024

Conversation

RenechCDDA
Copy link
Member

@RenechCDDA RenechCDDA commented Feb 17, 2024

Summary

None

Purpose of change

While loading core data, we do some adjustments to the (fake) map (yes before the game is started!). But we only set new_game after loading the json, but still before we actually start the game. The assumption being that loading json data is just loading json data, it doesn't modify the map or do anything besides moving data around. Well, it does. Camps generate a fake map, perform fake map updates to it, and in the process (about 20 functions down the callstack, in weather_manager::get_temperature) process any items with temperature that those fake updates generated. Since the real map isn't loaded and doesn't exist, it crashes.

Describe the solution

Before loading the JSON data, set new_game. This prevents the call in weather_manager::get_temperature and is what I personally would expect to be the case. new_game is still set to false at the same time(when the game is actually started), so this shouldn't impact gameplay. The only chance for unexpected interactions is if some function explicitly relied on new_game being false during loading(?!), which would be a baffling circumstance.

There are very few places where new_game is checked, so I am pretty confident this is a safe change.

Describe alternatives you've considered

I tried changing the default parameters for mapgendata to use a terrain type which wouldn't generate any items, but this wasn't successful. The byproducts of the fake camp update were still being generated. I tried again, using a completely null terrain which couldn't be loaded, same issue.

Testing

All tests besides localization/overmap test passed locally, both before and after the changes. Game compiles, loads, does not crash even with the linked issue's JSON loaded.

Additional context

@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 BasicBuildPassed This PR builds correctly, label assigned by github actions labels Feb 17, 2024
@juur
Copy link
Contributor

juur commented Feb 18, 2024

This resolves the block on #71804 when applied locally.

@Maleclypse Maleclypse merged commit 8fc7328 into CleverRaven:master Feb 18, 2024
25 of 29 checks passed
@RenechCDDA RenechCDDA deleted the newgame_during_init branch February 18, 2024 18:25
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Game crashes generating faction camp recipes when any of the components has_temperature()
3 participants