zturtleman / spearmint

Spearmint — an updated id Tech 3 engine for continuing the classics and creating new games.
https://clover.moe/spearmint
GNU General Public License v3.0
239 stars 27 forks source link

Looping target_speaker cannot be turned off and stays on outside PVS #300

Open zturtleman opened 4 years ago

zturtleman commented 4 years ago

target_speaker with spawnflags LOOP_ON or LOOP_OFF is suppose to toggle on/off when activated by another entity. Disabling loop sound does not work.

The issue is that trap_S_StopLoopingSound() is not called when target_speaker disables looping sound. It's necessary because looping target_speaker plays sound with trap_S_AddRealLoopingSound().

This feature probably use to work in Q3 1.1x when trap_S_AddLoopingSound() was used which automatically stops if sound isn't added this frame. trap_S_AddRealLoopingSound() and trap_S_StopLoopingSound() were added in Q3 1.2x (Team Arena update).

The change to use trap_S_AddRealLoopingSound() in Q3 1.2x also caused looping target_speaker entities to no longer turning off when not in the potentially visible set (PVS).

1. Revert to trap_S_AddLoopingSound()?

I haven't looked into the difference between add looping sound and add real looping sound.

What is the benefit of trap_S_AddRealLoopingSound()? Can looping speaker just be changed back to trap_S_AddLoopingSound()?

2. Call trap_S_StopLoopingSound()?

The suggested change at https://github.com/ioquake/ioq3/pull/180 causes a audio studdering issue when using OpenAL and playing railgun hum when local player is holding railgun. This can be fixed by limiting to ET_SPEAKER entities. It's also possible to stop looping speaker in Game VM using EV_STOPLOOPINGSOUND. See the Reference section below.

However both of these solutions have a problem. If a looping speaker leaves the player's PVS the sound continues to play and cgame won't get the loopSound change or event. Either the (turned off) looping speakers must be SVF_BROADCAST or CGame CG_TransitionSnapshot() needs to disable looping speakers when the entity leaves the snapshot.

target_speaker PVS bug

Using trap_S_AddLoopingSound() or properly calling trap_S_StopLoopingSound() leads to disabling target_speaker when it leaves the PVS. This could affect existing maps that don't even use toggle off looping speaker.

In the case of maps for Q3 1.1x, it fixes the looping speakers. For Q3 1.2x or later maps, it might be undesirable. I haven't looked at Team Arena or third party maps to see if this might be an issue.

It would be useful to at least add a cvar to enable the Q3 1.2x/1.32 bug of keeping looping speaker playing even after it leaves the PVS.

Reference

These only fix disabling looping speaker if the entity is in the player's PVS which isn't a full solution for using trap_S_AddRealLoopingSound(). (I haven't actually tested toggling looping target_speaker entities.)

To allow turning off speaker without breaking weapon looping sound when using OpenAL (which is just done in cgame, not entityState loopSound):

diff --git a/code/cgame/cg_ents.c b/code/cgame/cg_ents.c
index 5a02e58e..3c742999 100644
--- a/code/cgame/cg_ents.c
+++ b/code/cgame/cg_ents.c
@@ -273,6 +273,8 @@ static void CG_EntityEffects( centity_t *cent ) {
                        trap_S_AddRealLoopingSound( cent->currentState.number, cent->lerpOrigin, vec3_origin, 
                                cgs.gameSounds[ cent->currentState.loopSound ] );
                }
+       } else if ( cent->currentState.eType == ET_SPEAKER ) {
+               trap_S_StopLoopingSound( cent->currentState.number );
        }

Though ironically railgun hum loop sound prevents lava sizzle (entityState loopSound) from playing.

If you wanted to be compatible with vanilla Q3 CGame there is an (unused) event:

diff --git a/code/game/g_target.c b/code/game/g_target.c
index 1cf28c8b..1bfd422f 100644
--- a/code/game/g_target.c
+++ b/code/game/g_target.c
@@ -188,10 +188,12 @@ Multiple identical looping sounds will just increase volume without any speed co
 */
 void Use_Target_Speaker (gentity_t *ent, gentity_t *other, gentity_t *activator) {
        if (ent->spawnflags & 3) {      // looping sound toggles
-               if (ent->s.loopSound)
+               if (ent->s.loopSound) {
                        ent->s.loopSound = 0;   // turn it off
-               else
+                       G_AddEvent( ent, EV_STOPLOOPINGSOUND, 0 );
+               } else {
                        ent->s.loopSound = ent->noise_index;    // start it
+               }
        }else { // normal sound
                if ( ent->spawnflags & 8 ) {
                        G_AddEvent( activator, EV_GENERAL_SOUND, ent->noise_index );

Final Thoughts

For Spearmint (mint-arena) I think the solution would be to handle calling trap_S_StopLoopingSound() from CG_TransitionSnapshot() when ET_SPEAKER loopSound changes to 0 or entity leaves the PVS (no longer in snapshot).

Add a cvar to mint-arena to disable the calls to trap_S_StopLoopingSound() to use Q3 1.32 behavior. I guess it should technically a Game systeminfo cvar that is latched. Enabling Q3 1.32 loop sound mode client-side won't enable sounds outside of PVS and server operator might want it for a specific map.

These changes could be done in ioquake3 CGame but no one uses ioq3 CGame and it introduces different map behavior (vanilla CGame vs ioq3 CGame, which for me vary based on sv_pure) that no one can depend on being present and should not target.

For ioquake3 (or vanilla compatible project) to fix toggling looping speaker it might be better to fix it server-side in Game VM with EV_STOPLOOPINGSOUND and SVF_BROADCAST so that maps can use toggle without require CGame changes. Though using SVF_BROADCAST for this is kind of an ugly hack to disable target_speaker outside PVS and would still play looping sounds outside of the PVS which may or may not be desirable.

Personally I'm not really a fan of extending mapping features in ioquake3 unless that is a specific goal that will be documented (with code as GPL and Q3 SDK license) and supported by other projects as well. EntityPlus has additional entity features that would be a useful starting point for such a project.

zturtleman commented 4 years ago

@KuehnhammerTobias points out that looping target_speaker stops when it leaves PVS using the OpenAL backend but not the internal mixer (using SDL audio output).

You can test this on q3dm4. With SDL sound you won't hear the portal sound of the only existing Teleporter when you are under the RL bridge and you never entered the area where the teleporter is. If you enter the PVS area with the teleporter for the first time the portal01.wav speaker starts playing (as intended), next if you leave the teleporter area and go back beyond the Rocketlauncher bridge the portal speaker continues playing the sound. This means the PVS check fails, and the speaker doesn't stop playing. With OpenAL(soft) the PVS check works, this means with OpenAL enabled, the speaker will stop as soon as you leave the teleporter PVS area.