LAGonauta / MetaAudio

GoldSrc engine plugin for 3D sound
GNU General Public License v2.0
101 stars 13 forks source link

[Azure Sheep] Game may crash randomly #2

Open LAGonauta opened 5 years ago

LAGonauta commented 5 years ago

Symptom: Game may crash randomly, however there seems to be one reproducible crash after Kate is saved. She starts walking by herself and eventually stops at a corner. If the player keeps pressing the USE button on her the game might crash.

Where: Visual Studio seems to think the crash comes from "hl.dll", however we do not have the symbols to debug.

Probably an unexpected behaviour from MetaAudio is making the mod confused.

Save file near crashing area: savefile.zip

Mod patched up to malortie changes: https://www.moddb.com/mods/azure-sheep/downloads/azure-sheep-steampipe-patch-v11

malortie commented 5 years ago

@LAGonauta

Thanks for writing.

In the first place, I would like to thank you for having taken the time to write a detailed bug report, and in addition to this, to have provided a game save file to ease diagnosis.

Secondly, as requested, I uploaded a Visual Studio solution along with two projects to allow yourself to compile the mod. They should compile and output both client.dll and hl.dll out of the box. This could have been done before, but due to my lack of motivation for doing such task, this was never made.

https://github.com/malortie/halflife/tree/mod-hl-asheep/projects/vs2017

Finally, in regards to the issue you reported, You are correct. The crash occurs in the following block of code, in talkmonster.cpp:

BOOL CTalkMonster::CanFollow( void )
{
    if ( m_MonsterState == MONSTERSTATE_SCRIPT )
    {
        if ( !m_pCine->CanInterrupt() )
            return FALSE;
    }

    if ( !IsAlive() )
        return FALSE;

    return !IsFollowing();
}

The cause of the crash is due to m_pCine being NULL, while m_MonsterState is still evaluated at MONSTERSTATE_SCRIPT. Of course, this does not explain the reason behind why this bug exists.

When I found out about the location of the crash, I did check the locations around this block of code to see if I had changed anything. After having inspected all my previous commits and compared my working copy against the 'master' branch from Valve's official repository, I found no difference between this part of the code, and the one from my working copy. Still willing to make additional inquiries, I decided to check an older version of the Half-Life SDK. I believe the original Azure Sheep (2001) release was built using the Half-Life SDK 2.1 version so I did a check with this source code.

https://github.com/FaucetDC/hldc-sdk/blob/master/OldSourceCode/dlls/talkmonster.cpp

Once again, comparing the two files revealed no difference. This led me to believe that this bug could also be present in the original Azure Sheep release. I launched the original Azure Sheep and I managed to 'crash' the game by applying the same steps you provided in your bug report. In short, this bug was already present in older versions of the Half-Life SDK, so this is not a bug that was introduced with my patch.

For a quick fix, one could change the code of the block for the following:

BOOL CTalkMonster::CanFollow( void )
{
    if ( m_pCine && !m_pCine->CanInterrupt() )
        return FALSE;

    if ( !IsAlive() )
        return FALSE;

    return !IsFollowing();
}

However, I am not sure that this would be a good way of solving this issue. If the developers intended m_pCine to always be a valid pointer when m_MonsterState is evaluated at MONSTERSTATE_SCRIPT, this suggests that the root cause of this bug is located elsewhere in the code. I do believe the best way to solve this issue, would be to submit a pull request on Valve's official Half-Life repository with the 'right' fix applied. Please let me know what you think. What do you think would work best?