Better debugging for mutable special placement #51504
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
When placement of mutable overmap specials fails, it's hard to figure out why. The small amount of context previously given in the debugmsg wasn't enough. I did have some further debug printfs in non-compiled code behind a #if, but that's no use to people who can't compile the game themselves, nor does it help debug failures in CI. Reformulate the code so that these more detailed messages are always saved (rather than printed) and when a placement error occurs, it includes a full history of the placement to allow for debugging.
jbytheway
force-pushed
the
anthill_spawn_failure
branch
from
8c845bf to
cc93cab
Compare
|
#51497 was merged, so rebased this and marked ready for review. |
actual-nh
added
[C++]
|
I won the bug lottery and had this trigger in one of my PRs. Here is the log: |
|
Thanks for letting me know. I've downloaded it and will take a look when I get the chance. |
|
I believe I found the problem and will PR a putative fix shortly. Still not sure why it was being non-deterministic locally, though. I might have to do some digging into that at some point... |
Venera3
pushed a commit
to Venera3/Cataclysm-DDA
that referenced
this pull request
Sep 21, 2021
When placement of mutable overmap specials fails, it's hard to figure out why. The small amount of context previously given in the debugmsg wasn't enough. I did have some further debug printfs in non-compiled code behind a #if, but that's no use to people who can't compile the game themselves, nor does it help debug failures in CI. Reformulate the code so that these more detailed messages are always saved (rather than printed) and when a placement error occurs, it includes a full history of the placement to allow for debugging.
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.
Summary
None
Purpose of change
When placement of mutable overmap specials fails, it's hard to figure out why. The small amount of context previously given in the
debugmsgwasn't enough. I did have some further debugprintfs in non-compiled code behind a#if, but that's no use to people who can't compile the game themselves, nor does it help debug failures in CI, such as happened in #51494 here.Describe the solution
Reformulate the code so that these more detailed messages are always saved (rather than printed) and when a placement error occurs, it includes a full history of the placement to allow for debugging.
I also optimized the placement slightly; now it automatically moves to the next phase of placement when one phase is exhausted.
Describe alternatives you've considered
Rather than formatting the string always, but only using it in the event of an error, I considered saving all the info needed to format the string and doing it later, but that would be messier code, and I don't think any of this string formatting is particularly expensive. In particular, there are no translations involved.
Testing
I tweaked the anthill special definition to force a placement error and got the detailed debugmsg as expected.
Additional context
I've been unable to easily reproduce the placement failures locally (I did so once, but it wasn't repeatable), so I'm hoping that by PRing this I can debug the issue via any further CI failures that occur.
Unfortunately this change is going to conflict with the fixes in #51497 and that one is more important, so I'm going to mark this as draft until that's merged and I can rebase this atop it.Now rebased.