Better stack traces #38
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements better stack traces into the game, for when it crashes. This uses a modified version of StackTracePlus, that I modified so it would show the name and id for mods (compared to just
[string "--- STEAMODDED HEADER..."]), and also shows some more info for internal Love2D unctions.It also adds a function
injectStackTracewhich loads stack trace plus, and overwrites the game's crash handler to, debug print the message and stack trace and show the custom stack trace on the screen.I made this originally as a mod, because I found often times people were having a hard time figuring out what mod was involved with a crash (even with debug mode giving stack traces). This was meant to resolve that, by giving more useful info on these crashes.
However, I have decided to make this PR to add it as a core Steamodded feature. This is mostly because a couple people suggested it on the discord, but also because I realized that the mod is unable to show crash info for mods that crash before the mod is initialized, which is less than optimal. I also figured this features would be useful for most people.
There are some design decisions I am not sure about, and would like if I could get a review on.
First off is the init.
injectStackTraceis currently called ininitSteamodded, which I figured would be decent as it is something that is being initialized, and it then is properly loaded before mods. However, I could call the function right under its definition, which might make more sense, as it could catch errors sooner (not sure how much logic is actually ran sooner), and is also entirely self contained.Another decision I made is to kind of hack the built in error handler, to always show a stack trace. The code is fine, but I could probably re-implement the entire handler, making it a bit nicer to work with, as well as do things like disable the sending of crash reports to localthunk, a bit more consistency with the stack if debug mode is on or not, and also would let us add additional functionality to the error screen in the future easier (maybe the ability to copy the error to the clipboard, or restart with mods disabled, for example).
I was also unsure if I should keep the core StackTracePlus code and my inject code in the same or different files. I kept them in the same for now, as I figured they are tightly related, and there's no reason to separate them, but maybe you have a different way of designing things.
Otherwise, thank you very much for the review.