[LoneFox]

This happens every time I try to complete a level by hitting a nuke button:

eduke32: src/engine.c:7016: dorotatesprite: Assertion `!(bad&1)' failed.
./eduke32[0x537774]
/lib/libc.so.6(+0x34e50)[0x7fd2f3884e50]
/lib/libc.so.6(gsignal+0x3e)[0x7fd2f3884d7e]
/lib/libc.so.6(abort+0x185)[0x7fd2f38862a5]
/lib/libc.so.6(+0x2d960)[0x7fd2f387d960]
/lib/libc.so.6(+0x2da02)[0x7fd2f387da02]
./eduke32[0x4d04f3]
./eduke32[0x4e18b0]
./eduke32[0x49a2a8]
./eduke32[0x41996e]
./eduke32[0x42485b]
/lib/libc.so.6(__libc_start_main+0xed)[0x7fd2f38711ed]
./eduke32[0x40726d]

This is on Linux x86_64, software mode.
Revision 2498 works, anything after that fails.

[Plagman]

Ugh, assertion failures shouldn't abort; if we're going to have them, it should only spew.

[Helixhorned]

Nice, I had suspected that my code was broken somehow but never (even now) hit the assertion failure myself. It's fixed in r2529.

Plagman, why not? Assertions are a way of communicating certain programmer's assumptions at a given point. When they fail, the result is undefined behavior in most cases, i.e. the program has no meaning. Terminating it is the most sensible course of action then.

Basically, these two articles constitute my, er, philosophy about programming in C:
A Guide to Undefined Behavior in C and C++ by John Regehr, for a general write-up and
What Every C Programmer Should Know About Undefined Behavior by Chris Lattner, for an account from a practical perspective.

Therefore, strictness == GOOD, early crashes == GOOD.

LoneFox, I'm glad that you're running debug builds, it contributes greatly to making EDuke32 more stable overall. If you don't mind the performance penalty, you could even try out a CLANG=1 RELEASE=0 DEBUGANYWAY=1 build, which will catch some more errors.

[Plagman]

I'm all for having assertions in the code, but it doesn't make sense to have them abort everything. Having them spew the line number and prompt people to post their log would be better overall. I'd rather have a rotatesprite that looks slightly wrong for someone than him not being to use debug builds at all, which provides more coverage by potentially hitting other asserts, trying to get a backtrace for another crash entirely, etc. If we crash just after the assertion failure, fine, can't be avoided. If we don't, then either we're fine or in some small amount of cases we'll get side-effects that keep manifesting throughout the program that the user will complain about, and the assertion failure will be there in his log. Matching these symptoms with the assert condition will make for a lot easier debugging than just the condition on its own with no context. More information is always better than no information.

[Helixhorned]

Maybe, but errors that are merely written out to the log are ignored by most users.

[Plagman]

Yes, and I agree that spewing without prompting isn't sufficient. I think we should be spewing without aborting during gameplay, and on exit prompt the user to post their log. Since they're already going out of their way to run a debug build, they should react in most cases. Even better, the prompt could be to automagically upload the crashlog to my server. I think we already have code to do HTTP requests somewhere for the auto-update stuff, so it probably wouldn't be too hard.

[Danukem]

Helixhorned, on 25 March 2012 - 07:47 AM, said:

Maybe, but errors that are merely written out to the log are ignored by most users.

Yes, and also our volume of users is pretty small to begin with, so it's possible for that kind of error to go unnoticed and undocumented for months.

Maybe what this shows is that there needs to be a distinction between official releases and development snapshots. In an official release, stability is very important because you don't want to drive away users with nonfunctional builds, so for those releases assertion errors should not abort. But for development snapshots, it's more important to get information about bugs, and aborting makes that much more likely.

EDIT: Nevermind Plagman's suggestion is way better.