Fix double space in messages like "X is standing over Y" - #800
Conversation
| restoreRNG; | ||
| return; |
There was a problem hiding this comment.
Why is this now an exit point? I can't see the relevance to the bug
There was a problem hiding this comment.
The way this code worked before was rather strange: it called sprintf with object uninitialized, then called describedItemName to initialize object, then (at the end of the function, outside the nested ifs) it had another identical call to sprintf. I'm guessing this was left over from an incomplete refactoring or similar. (Also note the similarly redundant sprintf that this PR removes, a few lines down. That part isn't strictly necessary for this PR, but it seemed worth fixing that one too.)
After this PR, this call to sprintf is now different from the one at the end of the function, so we have to exit early to stop that one from running.
There was a problem hiding this comment.
could you add a comment here explaining that? saying we can't fall through to the final sprintf because we need one fewer spaces (maybe with an explanation why that is)
There was a problem hiding this comment.
Done.
It looks like it might be possible to avoid the separate sprintf call (and maybe others; there's 9 in total in this function) with more work, but given how complicated the function is and how many cases would need to be tested, I'm not sure if it's worth the effort.
After a recent issue turned out to have been detectable with Valgrind (tmewett#817) and I found some uninitialized variables while working on a PR (tmewett#800), I used Valgrind to look for other similar issues. It found a potential one in `titleMenu()`, and although I'm not sure whether is actually causes issues, it seems worth fixing.
2c69103 to
c7a2b78
Compare
2 participants
No description provided.