DEH music string replacement buggy in Boom-based ports?

[esselfortium]

I've uploaded a simple vanilla DEH patch that works in some ports but not in others:

http://esselfortium.net/wasd/musicbug.deh

 

In Chocolate Doom, Crispy Doom, and GZDoom, map31 will play the music track D_DOOM instead of its default D_EVIL.

 

In PrBoom-Plus and Eternity, the patch won't work, and map31 will play D_EVIL with no change.

 

I know dehacked string replacement is a bit hacky (heh) but it seems odd that this is broken in Boom-derived ports. I wonder how many years ago this broke and if it's been noticed before? (:

[JadingTsunami]

Nice find. This indeed looks like a bug to me. The DeHackEd handler checks for to and from text length of 4 and assumes that means it is a sprite reference.

 

  if (fromlen==4 && tolen==4)
    {
 /* JDS: handle as a sprite */
    }
  else
    /* JDS: check if sound or music */

I believe the correct code should be:

 

  if (fromlen==4 && tolen==4)
    /* JDS: check for sprite */
  
  if( !found )
    /* JDS: check for sound/music */

 

[esselfortium]

Aha, that indeed looks like it would be the culprit :) Nice sleuthing!

[Altazimuth]

@JadingTsunami Awesome work. Fixes EE for sure. How do you wanna be credited in the commit?

[fabian]

PR for PrBoom+ anyone?

[Altazimuth]

6 minutes ago, fabian said:

PR for PrBoom+ anyone?

Don't you have write access? :P

[JadingTsunami]

1 hour ago, Altazimuth said:

@JadingTsunami Awesome work. Fixes EE for sure. How do you wanna be credited in the commit?

 

Oh, thank you. Anything is fine really. I think the most credit goes to @esselfortium who found the issue and narrowed it down to a minimum failing case.

 

I checked and the bug is nearly 21 years old, present all the way back to the Boom 2.02 source release.

 

Quite a find!

[Altazimuth]

Just now, JadingTsunami said:

I checked and the bug is nearly 21 years old, present all the way back to the Boom 2.02 source release.

 

Quite a find! 

Figured as much. That was my guess, given Choco and GZ work but PRB+ and EE didn't.

[esselfortium]

That's amazing. I seem to have somehow become a magnet for ancient undiscovered bugs.

[Doomkid]

This baffles me as I’ve used the music renaming method in deh before. I guess by pure luck none of them were ever 4 character names!

[A.K.]

Next on Doomworld Mysteries!

[fabian]

8 hours ago, Altazimuth said:

Don't you have write access? :P

Sure, but I can't fix all the bugs in three source ports on my own. And if I know someone already has a fix, I'd rather request a PR.