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

Change: [Win32] unify the crashlog handler with the other OSes #11236

Merged
merged 1 commit into from
Aug 27, 2023

Conversation

TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Aug 27, 2023

Motivation / Problem

Windows didn't use the base crashlog handler for most part, and does everything itself. This is a bit annoying when we want to change things, as you have to remember to change it in two places. So while working on #11232 this was a real pita.

So, in this PR I set out to fix that discrepancy, and make sure Windows is much more like the rest.

Description

The main difference is that Windows delayed creating a crash.sav. This is the only OS that does so, both MacOS and Linux create a crash.sav immediately. Otherwise, other PRs before this PR already made sure Windows is much more like the rest.

While at it, make the crash text a bit more readable, and sync this with MacOS. It just read a bit awkward.

Limitations

There is an argument to be made that a crash.sav should not be made when a crashlog is generated, but only on demand. As it is not unlikely creating a savegame causes other problems. After all, we are in a broken state. But we also noticed this often leads to users not having a crash.sav to add to a ticket, as they simply didn't read the dialog, or didn't understand what it was saying.

In the long run, I rather focus on something JGRPP did, and detect if we crash during the crash handler, and handle that case more proper. That will be done in a follow-up PR, for all OSes (and not only for Windows).

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR touches english.txt or translations? Check the guidelines
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, game_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

While at it, make the crash text a bit more readable, and sync
this with MacOS.
@TrueBrain TrueBrain merged commit 29a37c2 into OpenTTD:master Aug 27, 2023
20 checks passed
@TrueBrain TrueBrain deleted the crashlog-unify-windows branch August 27, 2023 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants