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 Disconsolate Merc crash #62362

Merged
merged 1 commit into from Nov 24, 2022

Conversation

mqrause
Copy link
Contributor

@mqrause mqrause commented Nov 24, 2022

Summary

Bugfixes "Fix Disconsolate Merc crash"

Purpose of change

Partially fixes #60938

Describe the solution

The crash is caused by outdated info on visible monsters when the forced redraw option is enabled and the compass widget tries to access the already removed npc. This adds a call to update the monster info before redrawing.

Describe alternatives you've considered

Figure out which order all the stuff happens in and find a way to shuffle things around without adding another call. That's probably not possible, though.
Or remove the force redraw option because it also causes other isses (#58795). But that obviously has more implications.

Testing

Load the save and walk north. To cause a crash both the "force redraw" option and the compass widget have to be enabled. With this fix there won't be a crash even with them enabled.

Additional context

http://files.catbox.moe/oevx9t.zip
Thanks @RenechCDDA for providing the save.

@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 Nov 24, 2022
@I-am-Erk
Copy link
Contributor

Do we still get the crash if you give him a debug invisibility trait, prior to running the EOC?

@mqrause
Copy link
Contributor Author

mqrause commented Nov 24, 2022

Assuming that'd happen at the same point during turn processing, I don't think that would change anything. Plus it would mean actually figuring out the involved json and/or EOC code and fiddling around with it.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Nov 24, 2022
@I-am-Erk
Copy link
Contributor

I would still like it tested but I am okay with merging this to fix the issue. THe underlying code still needs a fix at some point so I think we should call this a partial fix, not complete.

@I-am-Erk I-am-Erk merged commit 18761d7 into CleverRaven:master Nov 24, 2022
@I-am-Erk I-am-Erk added this to Eponymous PRs. in PRs to revert after G-stable Nov 24, 2022
@mqrause mqrause deleted the disconsolate_merc_crash_fix branch November 24, 2022 21:47
andrei8l added a commit to andrei8l/Cataclysm-DDA that referenced this pull request Nov 25, 2022
@dseguin dseguin removed this from Eponymous PRs. in PRs to revert after G-stable Jan 30, 2023
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.

Disconsolate Merc crashes game.
2 participants