Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trigger flip effects don't work if sound devices are disabled #583

Closed
rr- opened this issue Jun 5, 2022 · 10 comments · Fixed by #713
Closed

Trigger flip effects don't work if sound devices are disabled #583

rr- opened this issue Jun 5, 2022 · 10 comments · Fixed by #713
Assignees
Labels
OG bug A bug in original game
Milestone

Comments

@rr-
Copy link
Collaborator

rr- commented Jun 5, 2022

This is a bit esoteric, but if all sound output devices are disabled in Windows, the lights never turn on at the beginning of Atlantis.

@rr- rr- added the TR1X bug A bug with TR1X label Jun 5, 2022
@rr- rr- added this to the 2.10 milestone Jun 5, 2022
@oziphantom
Copy link
Collaborator

is this because it is waiting for a sound to end that will never end?

@walkawayy
Copy link
Collaborator

Hm FX_Flicker seems fine:

void FX_Flicker(ITEM_INFO *item)
{
    if (g_FlipTimer > 125) {
        Room_FlipMap();
        g_FlipEffect = -1;
    } else if (
        g_FlipTimer == 90 || g_FlipTimer == 92 || g_FlipTimer == 105
        || g_FlipTimer == 107) {
        Room_FlipMap();
    }
    g_FlipTimer++;
}

@rr-
Copy link
Collaborator Author

rr- commented Jun 30, 2022

I think it's a music trigger that can't fire and thus prevents other triggers from working?

@rr- rr- removed this from the 2.10 milestone Jul 25, 2022
@walkawayy
Copy link
Collaborator

walkawayy commented Jan 17, 2023

@rr- I think this is because of the effect routines running in sound.c.
https://github.com/rr-/Tomb1Main/blob/4432f86bdb2b6cf1935bc76ba7b78804a06cc46b/src/game/sound.c#L220

before it's checked:

    if (!m_SoundIsActive) {
        return;
    }

The bug affects flip effect routines caused by triggers:
https://github.com/rr-/Tomb1Main/blob/7afb2b9db068f074a2ab556a9bd4e1adedcf71d9/src/game/room.c#L906

I also tested Flip Effect 5 (earthquake) in Palace Midas and it does not bounce the camera with sound devices disabled, so this bug is not limited to the Atlantis flicker. I think the flip effects were called in sound.c because a lot of them play sound effects? I'm not sure where we would move this to.

Also I went to test if this happens in TombATI, and the game doesn't even start if sound devices are disabled. So I'd say this is an OG bug.

@walkawayy walkawayy changed the title T1M bug: Atlantis flicker doesn't trigger if there are no sound devices Flip effects don't work if sound devices are disabled Jan 17, 2023
@walkawayy walkawayy changed the title Flip effects don't work if sound devices are disabled Trigger flip effects don't work if sound devices are disabled Jan 17, 2023
@rr-
Copy link
Collaborator Author

rr- commented Jan 17, 2023

I'd say we should move this code:

    if (g_FlipEffect != -1) {
        g_EffectRoutines[g_FlipEffect](NULL);
    }

to Effect_Control() and remove it from Sound_UpdateEffects.

Something like this:

diff --git a/src/game/effects.c b/src/game/effects.c
index ff6f4bc4..306cc389 100644
--- a/src/game/effects.c
+++ b/src/game/effects.c
@@ -34,6 +34,11 @@ void Effect_Control(void)
         }
         fx_num = fx->next_active;
     }
+
+    // XXX: Some of the FX routines rely on the item to be not null!
+    if (g_FlipEffect != -1) {
+        g_EffectRoutines[g_FlipEffect](NULL);
+    }
 }
 
 int16_t Effect_Create(int16_t room_num)
diff --git a/src/game/sound.c b/src/game/sound.c
index 6f16c30f..0ca557de 100644
--- a/src/game/sound.c
+++ b/src/game/sound.c
@@ -217,12 +217,6 @@ void Sound_UpdateEffects(void)
         }
     }
 
-    // XXX: Why are we firing this here? Some of the FX routines rely on the
-    // item to be not null!
-    if (g_FlipEffect != -1) {
-        g_EffectRoutines[g_FlipEffect](NULL);
-    }
-
     for (int i = 0; i < MAX_PLAYING_FX; i++) {
         SOUND_SLOT *slot = &m_SFXPlaying[i];
         if (!(slot->flags & SOUND_FLAG_USED)) {

We're already calling Effect_Control near Sound_UpdateEffects in game.c, what would be backwards incompatible is that currently inventory.c calls Sound_UpdateEffects but does not call Effects_Control. In my opinion this is good and shouldn't cause problems.

@walkawayy
Copy link
Collaborator

Ok. And call it / check it where?

@rr-
Copy link
Collaborator Author

rr- commented Jan 17, 2023

See my edit

@walkawayy
Copy link
Collaborator

Ok I'll ask around if that will break anything since it moves where the effect routines are called in Game_Control:

        Item_Control();
        Effect_Control(); // g_EffectRoutines now here

        Lara_Control();
        Lara_Hair_Control(false);

        Camera_Update();
        Sound_UpdateEffects(); // g_EffectRoutines used to be here
        g_GameInfo.current[g_CurrentLevel].stats.timer++;
        Overlay_BarHealthTimerTick();

@walkawayy walkawayy self-assigned this Jan 17, 2023
@walkawayy walkawayy added this to the 2.13 milestone Jan 17, 2023
@walkawayy
Copy link
Collaborator

walkawayy commented Jan 17, 2023

I discussed this a bit with ChocolateFan. What about separating it into its own function but call it just before the sound code? I think moving it into Effect_Control before Lara_Control could potentially break things for a small benefit since most people are playing with sound devices and don't see this bug. It makes sense to move it since it's not really related to sound code, but we don't want to introduce bugs.

        Item_Control();
        Effect_Control();

        Lara_Control();
        Lara_Hair_Control(false);

        Camera_Update();
        Effect_Routines();  // g_EffectRoutines now here
        Sound_UpdateEffects(); // g_EffectRoutines used to be here
        g_GameInfo.current[g_CurrentLevel].stats.timer++;
        Overlay_BarHealthTimerTick();

@walkawayy walkawayy added OG bug A bug in original game and removed TR1X bug A bug with TR1X labels Jan 17, 2023
@rr-
Copy link
Collaborator Author

rr- commented Jan 18, 2023

Sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OG bug A bug in original game
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants