fabiangreffrath / woof

Woof! is a continuation of the Boom/MBF bloodline of Doom source ports.
GNU General Public License v2.0
208 stars 34 forks source link

Rekkr Sunken Land still not functioning properly #721

Closed rzhxiii closed 2 years ago

rzhxiii commented 2 years ago

Apologies in advance for starting this whole mess and for any frustrations caused by it. Now, if rekkrsl.wad is run as an IWAD, Woof crashes the moment a new game is started, regardless of level, with this error message: error If it's run as a PWAD for Doom 1, the game loads and works fine, but of course, the colored blood remains the same as in Doom.

I'm guessing that part of this is caused by the fact that standalone free Rekkr and the paid Sunken Land version either used different processes to be turned into IWADs, with the former being used and tested for vanilla Doom, while the latter launched with GZDoom, so this might actually be a WAD issue, but I don't know yet. I think that Sunken Land's WAD might also lack some files that it didn't use, but Doom did (in which case, those files were not changed, but since they're copyrighted they got removed from Sunken Land). Not sure about all this though, so I'm going to try to contact Revae as well (since he made Rekkr, he should be able to confirm/ deny if these are the reasons for which it doesn't work properly).

fabiangreffrath commented 2 years ago

Hm, I somehow got Crispy Doom to load it as an IWAD just yesterday evening and was sure that it also worked with Woof. 🤔

fabiangreffrath commented 2 years ago

You still have the extension changed to .wad, right?

rzhxiii commented 2 years ago

Yes, I tried it with a fresh install as well (for both Woof and Sunken Land, all I did was rename the extension). Woof crashes with the same error message attached. Edit: Crispy works

fabiangreffrath commented 2 years ago

We seem to have different copies of REKKRSL.wad. 🤷

fabiangreffrath commented 2 years ago

Could you try without loading the DEH patch on the command line?

rzhxiii commented 2 years ago

Actually scratch that, I'm stupid and forgot to take the Doom sprite fixes and widescreen assets out of the Crispy autoload. There's no chance of Doom assets being present if neither the port or the WAD supplies them, and I should've realized that. Crispy works fine.

rzhxiii commented 2 years ago

Woof still prints the same error, with or without the .deh. Nothing in doom-all, doom.wad or any rekkr autoload folders. I'm going to try a few other things.

rzhxiii commented 2 years ago

Drag and drop also gives the same error.

fabiangreffrath commented 2 years ago

Could be a sprite loading issue. I have a lot of security guards for this in Crispy since I implemented 16 sprite rotations there. Will have a closer look this evening.

fabiangreffrath commented 2 years ago

Blind chance, but could you please try the following patch? It seems that Windows' POSIX read() implementation does not like a NULL pointer for the second parameter, but this is what it gets from Z_Malloc() for an empty lump in W_CacheLumpNum().

--- a/src/w_wad.c
+++ b/src/w_wad.c
@@ -436,7 +436,7 @@ void W_ReadLump(int lump, void *dest)

   if (l->data)     // killough 1/31/98: predefined lump data
     memcpy(dest, l->data, l->size);
-  else
+  else if (l->size)
     {
       int c;
rzhxiii commented 2 years ago

I'll check it out this evening, just got off work but still have some stuff to do before I get a chance.

fabiangreffrath commented 2 years ago

Thank you! I have updated the patch a bit. Ignoring a NULL buffer might hide an actual bug, but we shouldn't even try to read a zero-size lump from anywhere.

rzhxiii commented 2 years ago

Sorry for delaying, I was too tired last night. I tried the patch. Good news, -iwad rekkrsl.wad now works perfectly out of the box. Bad news, colored blood is still broken.

fabiangreffrath commented 2 years ago

Good news, -iwad rekkrsl.wad now works perfectly out of the box.

Great, thanks for checking!

Bad news, colored blood is still broken.

Hm, I have added the autoload/rekkrsl.wad/bloodcolor.deh file in one of the last commits which should get autoloaded to explicitly disable colored blood. Could you make sure this file is available and gets loaded?

rzhxiii commented 2 years ago

Nope, still the same. Maybe the .deh patch isn't working as intended? Or it could be an order thing. If dehacked monsters don't explicitly set a blood color, but the .deh file comes after bloodcolor.deh in the load order, is it possible that Woof auto-defaults to Doom values?

rzhxiii commented 2 years ago

Actually, let me try doom-all, doom.wad, rekkr.wad and rekkrsa.wad folders as well.

rzhxiii commented 2 years ago

None of the folders work, that's really strange. Just to confirm, this is the correct .deh, right?

Thing 15 (Cacodemon) Blood color = 0

Thing 16 (Baron of Hell) Blood color = 0

fabiangreffrath commented 2 years ago

Yes, right.

I think we need some more information about your system. How do you build your binaries? What's the path called. Could you paste the entire terminal output that gets printed when you start your own build?

rfomin commented 2 years ago

It seems we called D_SetBloodColor() in wrong place. Maybe we should move G_ReloadDefaults():

diff --git a/src/d_main.c b/src/d_main.c
index b5c69e0..e194028 100644
--- a/src/d_main.c
+++ b/src/d_main.c
@@ -2362,10 +2362,6 @@ void D_DoomMain(void)
   // Check for wolf levels
   haswolflevels = (W_CheckNumForName("map31") >= 0);

-  // Moved after WAD initialization because we are checking the COMPLVL lump
-  G_ReloadDefaults();    // killough 3/4/98: set defaults just loaded.
-  // jff 3/24/98 this sets startskill if it was -1
-
   putchar('\n');     // killough 3/6/98: add a newline, by popular demand :)

   // process deh in IWAD
@@ -2387,6 +2383,10 @@ void D_DoomMain(void)

   PostProcessDeh();

+  // Moved after WAD initialization because we are checking the COMPLVL lump
+  G_ReloadDefaults();    // killough 3/4/98: set defaults just loaded.
+  // jff 3/24/98 this sets startskill if it was -1
+
   // Check for -file in shareware
   if (modifiedgame)
     {
fabiangreffrath commented 2 years ago

We only set deh_set_blood_color = TRUE if the blood was set to anything non-zero by DEHACKED, which the REKKR patches just don't do:

https://github.com/fabiangreffrath/woof/blob/master/src/d_deh.c#L2035-L2036

fabiangreffrath commented 2 years ago

@rzhxiii could you please try this patch as well?

--- a/src/d_deh.c
+++ b/src/d_deh.c
@@ -2031,9 +2031,7 @@ void deh_procThing(DEHFILE *fpin, FILE* fpout, char *line)
                     return;
                   }
                   mi->bloodcolor = (int)(value);
-
-                  if (mi->bloodcolor)
-                    deh_set_blood_color = TRUE;
+                  deh_set_blood_color = TRUE;
                 }
                 break;
fabiangreffrath commented 2 years ago

However, I think @rfomin's patch is also required, because G_ReloadDefaults() is only called a second time in G_DoNewGame(). So, it makes a difference if you start the game from the menu or e.g. with -warp from the command line.

rzhxiii commented 2 years ago

Just applied both patches and I can confirm that everything works as it should. The only thing that I did not expect was the fact that colored blood cannot be toggled anymore. (I'm assuming this is by design?) It is turned on however and Rekkr's monsters now bleed red.

rzhxiii commented 2 years ago

Ok, here are some screenshots and some observations:

rzhxiii commented 2 years ago

Please let me know if you need me to test anything else/ if you need more information. I can't thank both of you enough for dealing with this headache.

fabiangreffrath commented 2 years ago

Just applied both patches and I can confirm that everything works as it should. The only

Thanks again for checking!

thing that I did not expect was the fact that colored blood cannot be toggled anymore. (I'm assuming this is by design?) It is turned on however and Rekkr's monsters now bleed red.

Yes, this is intended. Dehacked has precedence, and if Dehacked sets (or unsets) colored blood, the user has no choice anymore.

* bloodcolor.deh is required, otherwise you get blue/ green blood where you shouldn't have it (no surprise here)

Indeed, no surprise. By default, the Doom flavors all have colored blood (if the users chooses so), so we have to explicitly disable it for the non-Doom IWADs.

* Before, the messages all displayed in Rekkr's default message color (I'm assuming because the font got changed, and color settings did not override custom fonts?). Now they get affected by local message settings (I have it set to gray). - this is not an issue, just an observation, in fact I quite like the change

Yes, I have extended the color translation ranges for the three non-Doom IWADs. Though that's not new. What's new is that REKKRSL.wad is recognized to set gamemission to pack_rekkr.

* Edit: the messages only change in Rekkr's case from what I can see, Doom PWADs that override the message color settings display as they did previously.

It also works for Chex and Hacx, but not PWADs. These are expected to provide color translation lumps like e.g. CRGREEN, CRBLUE, etc. themselves.

@rfomin Are we sure about the patches?

rfomin commented 2 years ago

@rfomin Are we sure about the patches?

Yes, I think it's all right.

fabiangreffrath commented 2 years ago

https://github.com/fabiangreffrath/woof/commit/a7f35f0d84ec481748df2de4f7ee0ad71d3a1a3e https://github.com/fabiangreffrath/woof/commit/c5bcf935f04942f6c9ae5b6926a79e813f7cc4f1 https://github.com/fabiangreffrath/woof/commit/ad74f830b9d74e11179a2a59857ff1420c88a1b9

Thanks @rzhxiii for insisting!