Hi, On Sun, Oct 14, 2012 at 12:23:45AM +0200, Sylvain wrote: > Hi Bas, > > Thanks a great lot for all your effort on this nasty bug, despite my > lack of responsiveness.
No problem, bug fixing is fun. :-) > I took a few hours to investigate what exactly happens (adding a few > code comments in the process), and I see that in your testcase dmod, > the warp is an invisible sprite which is merged in the background and > thus never found by find_sprite(). Yes, that is indeed essential to trigger the bug. > If spr[0] is clean, the warp continues despite not finding the warp > sprite; if spr[0] is modified, then we hit the bug, as the code > assumes the warp animation is not finished. Yes. And because Seth didn't like index 0, the animation of spr[0] doesn't actually play (he uses for (crap = 1; ...)), so it will never finish. > I'm not sure in what way an unclean spr[0] will affect the game, but > it might, and cleaning it would affect compatibility with the original > Dink. I don't think it would. Seth doesn't actually use it. He just skipped it because he wants to start his indices at 1. However, I don't suggest it should be cleaned. That would only hide other bugs. > Consequently I used and documented your original fix from > http://www.dinknetwork.com/forum.cgi?MID=168476#168476 > > http://git.savannah.gnu.org/cgit/freedink.git/commit/?id=fff4b7cb8d6d2bf84482fdc83c2a21fd9d0379e1 > http://git.savannah.gnu.org/cgit/freedink.git/commit/?id=402bf38f69cd7e5c47322b8087ba535d6f823283 > http://git.savannah.gnu.org/cgit/freedink.git/commit/?id=5d692b447eb7a8b3c0f1128390641992a4dff484 > > WDYT? Renaming "prop" to "is_warp" is a very good idea; it makes it all much better understandable. :-) As for the bug, it really is several bugs. You have fixed the warp problem by ignoring memory corruption, which is good. Still, I would suggest that the memory should not get corrupted in the first place. That is, if the engine tries to create a sprite, but there are no free slots for it, it should also detect this and refuse to write into spr[0]. I have found two places where the engine creates sprites (add_random_blood and the creation of flying duck heads), and that may be all, but I didn't check. In both cases, there is no check if the sprite creation actually worked, and there should be. > (Btw, I don't have the same line numbers in my source files, possibly > you added test code around? ;)) I don't think I did, but I may be using an old copy. Thanks, Bas
signature.asc
Description: Digital signature
