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

Bugfix: don't play theme if music off #42

Closed
wants to merge 1 commit into from

Conversation

damianut
Copy link
Contributor

@damianut damianut commented Jul 2, 2024

Otherwise, it causes Access Violation error.

Otherwise, it causes Access Violation error.
Copy link
Member

@piotrmacha piotrmacha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. We should check if the music system is active, but the current code causes a compilation error because s_musicSystemDisabled is an undeclared identifier. I will be able to help with that in the evening today, but if you manage to fix that beforehand, the PR would be ready to merge.

@@ -6,6 +6,11 @@ namespace GOTHIC_NAMESPACE
{
int BassMusic_Play()
{
if (s_musicSystemDisabled)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compilation error

D:\a\zBassMusic\zBassMusic\src\Gothic/Externals.hpp(9): error C2065: 's_musicSystemDisabled': undeclared identifier
D:\a\zBassMusic\zBassMusic\src\Gothic/Externals.hpp(9): error C2065: 's_musicSystemDisabled': undeclared identifier
D:\a\zBassMusic\zBassMusic\src\Gothic/Externals.hpp(9): error C2065: 's_musicSystemDisabled': undeclared identifier
D:\a\zBassMusic\zBassMusic\src\Gothic/Externals.hpp(9): error C2065: 's_musicSystemDisabled': undeclared identifier

Copy link
Member

@piotrmacha piotrmacha Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@damianut, please make the following changes:

  1. Replace s_musicSystemDisabled with zCMusicSystem::s_musicSystemDisabled - this symbol is a static member of zCMusicSystem class, but in external we are outside the class context.
  2. Move your if below parser->GetParameter(id); - in externals, we must always pop every parameter from the stack before early return. Otherwise, it would lead to memory leaks or even a crash.

The final code should look like

    int BassMusic_Play()
    {
        zSTRING id;
        parser->GetParameter(id);
        if (zCMusicSystem::s_musicSystemDisabled)
        {
            log->Error("Music system disabled.");
            return 0;
        }
        zCMusicTheme* theme = zmusic->LoadThemeByScript(id);
        zmusic->PlayTheme(theme, zMUS_THEME_VOL_DEFAULT, zMUS_TR_DEFAULT, zMUS_TRSUB_DEFAULT);
        return 0;
    }

@piotrmacha
Copy link
Member

As requested by the PR author @damianut, the fixes were made by the project maintainers in #43

@piotrmacha piotrmacha closed this Jul 7, 2024
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants