fabiangreffrath / crispy-doom

Crispy Doom is a limit-removing enhanced-resolution Doom source port based on Chocolate Doom.
https://fabiangreffrath.github.io/crispy-homepage
GNU General Public License v2.0
805 stars 132 forks source link

Demo desync in MAP33 caused by the Wolf SS present in that map #292

Closed SoDOOManiac closed 6 years ago

SoDOOManiac commented 6 years ago

Background

Version of Crispy Doom: 17.04.2018 Git https://github.com/fabiangreffrath/crispy-doom/commit/9b692d6780e1f7c4a72a6322cd306d5e42b30074

Operating System and version: Win 7

Game: Doom 2 BFG Edition

Observed behavior: I hear the Wolf SS alert sound in this location doom0036 but the graphics that enemy uses are those for the zombieman. While recording the demo I completed the level, but in the demo played back that Arch-Vile kills me. BTW, the Arch-Vile resurrected that WereWolf SS.

Expected behavior: Wolf SS properly replaced by a zombieman and demo sync maintained, like in MAP31 and MAP32.

Demo: ss_test.zip No mods have been used.

fabiangreffrath commented 6 years ago

but the graphics that enemy uses are those for the zombieman.

Hm, this should be impossible when recording a demo. The whole WolfSS replacement code is covered by a if crispy->singleplayer conditional. Also, the whole enemy class would be replaced, not only their sprites. But, I know MAP33 and there are some Zombiemen around the place where the WolfSS is, so maybe you mixed this up.

While recording the demo I completed the level, but in the demo played back that Arch-Vile kills me. BTW, the Arch-Vile resurrected that WereWolf SS.

Are you playing with the BlackOps or Smoothed mod again?

SoDOOManiac commented 6 years ago

Are you playing with the BlackOps or Smoothed mod again?

I said no mods had been used, to make a clean experiment. Just watch the demo ;)

fabiangreffrath commented 6 years ago

BTW, the Arch-Vile resurrected that WereWolf SS.

Hm, how do you know it resurrected the WolfSS if you cannot see it?

Just watch the demo ;)

Will have to wait until I am at home.

SoDOOManiac commented 6 years ago

Hm, how do you know it resurrected the WolfSS if you cannot see it?

The Arch-vile did a resurrection animation. Another update: when recording the demo, the Wolf SS is invisible but audible (!) doom0033 and when playing back the demo he looks and sounds like a zombieman. doom0039 In MAP31 and MAP32 there's no such bug, the SS are just replaced by zombiemen both during demo recording and playback: ss_test_31.zip

fabiangreffrath commented 6 years ago

Oh crap! Could you probably do a test and play the demo with the Vanilla DOOM2.WAD loaded as a PWAD?

SoDOOManiac commented 6 years ago

Oh yeah, with vanilla DOOM2.WAD loaded as a PWAD start crispy-doom -iwad doom2(2).wad -merge doom2.wad -playdemo ss_test the demo keeps sync! doom0041

fabiangreffrath commented 6 years ago

Thank you very much for spotting this bug! The demo should play back now with either DOOM2.WAD loaded or not, but never show an actual WolfSS sprite.

fabiangreffrath commented 6 years ago

Sorry, yesterday's fix was moot, because G_InitNew() unconditionally sets demoplayback = false before calling G_DoLoadLevel() which in turn calls P_SetupLevel(). I'll have to come up with something different...

fabiangreffrath commented 6 years ago

This might work:

--- a/src/doom/g_game.c
+++ b/src/doom/g_game.c
@@ -2239,7 +2239,7 @@ G_InitNew

     usergame = true;                // will be set false if a demo
     paused = false;
-    demoplayback = false;
+//  demoplayback = false;
     automapactive = false;
     viewactive = true;
     gameepisode = episode;
@@ -2605,6 +2605,9 @@ void G_DoPlayDemo (void)
     int demoversion;
     int lumplength; // [crispy]

+    // [crispy] demo playback is never singleplayer-safe
+    CheckCrispySingleplayer(!(demoplayback = true));
+
     lumpnum = W_GetNumForName(defdemoname);
     gameaction = ga_nothing;
     demobuffer = W_CacheLumpNum(lumpnum, PU_STATIC);
@@ -2614,7 +2617,6 @@ void G_DoPlayDemo (void)
     lumplength = W_LumpLength(lumpnum);
     if (lumplength < 0xd)
     {
-       demoplayback = true;
        G_CheckDemoStatus();
        return;
     }
@@ -2650,7 +2652,6 @@ void G_DoPlayDemo (void)
         fprintf(stderr, message, demoversion, G_VanillaVersionCode(),
                          DemoVersionDescription(demoversion));
        fprintf(stderr, "\n");
-       demoplayback = true;
        G_CheckDemoStatus();
        return;
         }
@@ -2693,8 +2694,6 @@ void G_DoPlayDemo (void)

     usergame = false;
     demoplayback = true;
-    // [crispy] update the "singleplayer" variable
-    CheckCrispySingleplayer(!demorecording && !demoplayback && !netgame);

     // [crispy] demo progress bar
     {

Critial cases to test are, e.g.

SoDOOManiac commented 6 years ago

Critial cases to test are, e.g. loading a game while a demo is running in the demo loop playing back a demo from a savegame starting a new game while a demo is running in the demo loop

All of the above test cases show the fix working perfectly! BTW: how did I get 102% kills, did the Arch-Vile's resurrections count? And Par time for MAP33 SUCKS, that is a map bug, there's no need to do anything about it, it's even fun XD

fabiangreffrath commented 6 years ago

All of the above test cases show the fix working perfectly!

I hope you mean the fix I committed, not the one I posted above?

did the Arch-Vile's resurrections count?

I think so, yes. The monster's flags are reset when it gets resurrected, so the MF_COUNTKILL flags is restored: https://github.com/fabiangreffrath/crispy-doom/blob/master/src/doom/p_enemy.c#L1230

And Par time for MAP33 SUCKS, that is a map bug, there's no need to do anything about it, it's even fun XD

There is a tool available for Choco that checks if a demo desyncs by comparing the Kill/Items/Secrets/Time and Par Time counts at the end of each map with the values from Vanilla. I don't want to confuse this tool by changing any of these values when demos are played back, and this includes MAP33 (although not originally supported by either Choco nor Vanilla)

fabiangreffrath commented 6 years ago

Thanks!

fabiangreffrath commented 6 years ago

@Zodomaniac Why do you remove your valuable comments?

SoDOOManiac commented 6 years ago

@fabiangreffrath Sorry for the confusion) When watching that demo I noticed that in the map there were 93 monsters+3 resurrections, and the counter was 95 kills. I was puzzled if one kill wasn't counted (I suspected that because one of the monsters had been resurrected twice), removed the previous comment and wanted to post it again with an addition, but then I loaded the game, used IDDT and discovered that I had really missed one enemy. Anyway, your commit fixed the issue perfectly, and the port behaves correctly in all the critical cases :)

fabiangreffrath commented 6 years ago

Cool, thanks again! Do you use a pwad to show the wolfss sprite or do you just live with it missing?

SoDOOManiac commented 6 years ago

I recorded that demo without the PWAD fix and then played it back both without and with the PWAD, the demo keeps sync so the PWAD is safe to use ;)