JNechaevsky / international-doom

Small, functional and simple source ports, created with help, efforts and bits of code from people around the world.
https://jnechaevsky.github.io/inter-doom/
GNU General Public License v2.0
30 stars 2 forks source link

Sound fixes for Heretic's "flipped levels" and "disable render when minimized" #42

Closed kitchen-ace closed 10 months ago

kitchen-ace commented 10 months ago

Using (255 - angle) for the flipped sound angle seems right, and lets you return to using int for the sound angle.

Moving the display update back to being after the sound update avoids many clicks where the first tic of the sound is played at angle 0. This is a bug introduced with commit 862475c.

For an example of the above, try this map without the fix. Move back and fire the hellstaff, many of the shots will click in the left channel only briefly, before going back to the centre channel. Happens more frequently with uncapped framerates.

hsnd.zip

I've played a few episodes with this applied now and it fixes most of the sound issues. There is still the occasional click towards angle 0, so a more robust fix might be needed at some time.

kitchen-ace commented 10 months ago

Also note that vanilla Heretic has a few bugs of its own. One I noticed is that long sounds, such as ambient sounds, don't get their position updated while there are lots of enemies present. A good example is E5M2, the waterfall at the start won't update it's angle until the sound restarts. However type massacre and then the waterfall sounds like it should... I don't have a clue about why this is, just something to be aware of when testing sounds, to make sure you're not conflating bugs.

P.S. No worries on going on an introvert coding binge! :D

JNechaevsky commented 10 months ago

Moving D_Display() was deliberately, unfortunately. If this function will be moved back, then volume will be still zero-ed if "Mute inactive sound" is on, and windows looses it's focus, then minimized (while not focused) and restored back. 😕

Probably I should move S_MuteUnmuteSound() to somewhere in upper level code, like in HandleWindowEvent at i_video.c, need to experiment. And thanks for the idea!

kitchen-ace commented 10 months ago

Moving D_Display() was deliberately, unfortunately.

Ah, I see. Feel free to close this, then.

I don't know enough about what's going on to know why sounds are sometimes played before their angle is updated, but it is a noticeable problem.

JNechaevsky commented 10 months ago

Actually... No need to close! I'm wrong, even with current implementation it is possible to accidentally get 0 volumes. Should be fixed like this, i.e. no need to touch original snd_MaxVolume and snd_MusicVolume:

--- C:/GitHub/international-doom/src/heretic/s_sound.c  Wed Jan 10 21:55:45 2024
+++ C:/msys64/home/Julia/inter-doom/src/heretic/s_sound.c   Wed Jan 10 21:49:47 2024
@@ -657,6 +657,14 @@
     }
 }

+void S_SetMinVolume(void)
+{
+    for (int i = 0; i < MAX_SND_DIST; i++)
+    {
+        soundCurve[i] = 0;
+    }
+}
+
 static boolean musicPaused;
 void S_SetMusicVolume(void)
 {
@@ -688,19 +696,8 @@

 void S_MuteUnmuteSound (boolean mute)
 {
-    static int snd_MaxVolume_old;
-    static int snd_MusicVolume_old;
-
     if (mute)
     {
-        // Remember current sound and music volume.
-        snd_MaxVolume_old = snd_MaxVolume;
-        snd_MusicVolume_old = snd_MusicVolume;
-
-        // Set volume variables to zero.
-        snd_MaxVolume = 0;
-        snd_MusicVolume = 0;
-
         // Stop all sounds and clear sfx channels.
         for (int i = 0 ; i < snd_Channels ; i++)
         {
@@ -713,14 +710,10 @@

         // Set actual volume to zero.
         I_SetMusicVolume(0);
-        S_SetMaxVolume(true);
+        S_SetMinVolume();
     }
     else
     {
-        // Restore volume variables.
-        snd_MaxVolume = snd_MaxVolume_old;
-        snd_MusicVolume = snd_MusicVolume_old;
-
         // Set actual volume to restored values.
         I_SetMusicVolume(snd_MusicVolume * 8);
         S_SetMaxVolume(true);

I'll experiment a bit more and then gladly merge your correction 👍

JNechaevsky commented 10 months ago

All done here, thank you very much!

JNechaevsky commented 10 months ago

Yeah, just checked E5M2. It feels like an ambient sound is updating stereo separation not every game tic, but only after it's finished and then started again. My old solution, slightly cleaned, but I'm really not sure is it optimal for all possible cases and safe to use at all.

On E5M2, right after waterfall will play it's first sound in sequence, next sequences will be properly updated on stereo separation. You can just stand at starting point and rotate camera left-right to check out.

s_sound.c

--- C:/GitHub/international-doom/src/heretic/s_sound.c  Wed Jan 10 22:20:38 2024
+++ C:/msys64/home/Julia/inter-doom/src/heretic/s_sound.c   Wed Jan 10 23:52:38 2024
@@ -340,6 +340,101 @@
     }
 }

+void S_StartSoundAmbient(void *_origin, int sound_id)
+{
+    mobj_t *origin = _origin;
+    mobj_t *listener;
+    int dist, vol;
+    int i;
+    int sep;
+    int angle;
+    int64_t absx;
+    int64_t absy;
+    int64_t absz;  // [JN] Z-axis sfx distance
+
+    // [JN] Do not play sound while demo-warp.
+    if (nodrawers)
+    {
+        return;
+    }
+    if (sound_id == 0 || snd_MaxVolume == 0 || (nodrawers && singletics))
+    {
+        return;
+    }
+
+    listener = GetSoundListener();
+
+    if (origin == NULL)
+    {
+        origin = listener;
+    }
+
+    if (origin == listener || snd_monosfx)
+    {
+        sep = 128;
+    }
+    else
+    {
+        angle = R_PointToAngle2(listener->x, listener->y, origin->x, origin->y);
+        angle = (angle - viewangle) >> 24;
+        if (gp_flip_levels)
+            angle = 255 - angle;
+        sep = angle * 2 - 128;
+        if (sep < 64)
+            sep = -sep;
+        if (sep > 192)
+            sep = 512 - sep;
+    }
+
+    // [JN] Calculate the distance.
+    absx = llabs(origin->x - listener->x);
+    absy = llabs(origin->y - listener->y);
+    absz = aud_z_axis_sfx ?
+           llabs(origin->z - listener->z) : 0;
+    dist = S_ApproxDistanceZ(absx, absy, absz);
+    dist >>= FRACBITS;
+
+    if (dist >= MAX_SND_DIST)
+    {
+        return;                 //sound is beyond the hearing range...
+    }
+    if (dist < 0)
+    {
+        dist = 0;
+    }
+
+    // [JN] Calculate the volume based upon the distance from the sound origin.
+    vol = soundCurve[dist];
+    
+    // [JN] No priority checking.
+    for (i = 0; i < snd_Channels; i++)
+    {
+        if (channel[i].mo == NULL)
+        {
+            break;
+        }
+    }
+
+    if (i >= snd_Channels)
+    {
+        return;
+    }
+
+    channel[i].pitch = (byte) (NORM_PITCH + (M_Random() & 7) - (M_Random() & 7));
+    channel[i].handle = I_StartSound(&S_sfx[sound_id], i, vol, sep, channel[i].pitch);
+    channel[i].mo = origin;
+    channel[i].sound_id = sound_id;
+
+    if (S_sfx[sound_id].usefulness == -1)
+    {
+        S_sfx[sound_id].usefulness = 1;
+    }
+    else
+    {
+        S_sfx[sound_id].usefulness++;
+    }
+}
+
 void S_StartSoundAtVolume(void *_origin, int sound_id, int volume)
 {
     mobj_t *origin = _origin;

p_enemy.c

--- C:/GitHub/international-doom/src/heretic/p_enemy.c  Sun Jan  7 00:06:30 2024
+++ C:/msys64/home/Julia/inter-doom/src/heretic/p_enemy.c   Wed Jan 10 23:47:08 2024
@@ -2407,6 +2407,7 @@
 void A_ESound(mobj_t *mo, player_t *player, pspdef_t *psp)
 {
     int sound = sfx_None;
+    extern void S_StartSoundAmbient(void *_origin, int sound_id);

     switch (mo->type)
     {
@@ -2419,7 +2420,8 @@
         default:
             break;
     }
-    S_StartSound(mo, sound);
+    S_StartSoundAmbient(mo, sound);
+    //S_StartSound(mo, sound);
 }
kitchen-ace commented 10 months ago

Confirming that the patch above works for ambient sounds and seems demo-compatible on a quick test. Actually the very first time the waterfall sound plays in E5M2 it has the old behaviour of not adjusting its angle, after that it works as you'd expect. I think it's still a big improvement.

JNechaevsky commented 10 months ago

This will do the trick. Please don't ask anything, I have no idea how this magic works. 😀

    if (dist >= MAX_SND_DIST)
    {
        //return;                 //sound is beyond the hearing range...
        dist = MAX_SND_DIST - 1; // ta-da!
    }

Such replacement is all-around safe for demos, as despite of being called by playstate, it doesn't hits P_Random or affects playstate somehow. Hitting M_Random is always safe, as it's not used for playstate/demo/multiplayer.

I've heard some talks that "Heretic's random is always too random", and that's probably because of this line, which is hitting global playstate random every odd tic. Absolute evil.