Skip to content

Fix issue #1170 - #1172

Merged
Headline merged 4 commits into
alliedmodders:masterfrom
naydef:master
Feb 6, 2020
Merged

Fix issue #1170#1172
Headline merged 4 commits into
alliedmodders:masterfrom
naydef:master

Conversation

@naydef

@naydef naydef commented Feb 3, 2020

Copy link
Copy Markdown
Contributor

After doing some testing with a debugger, I noticed that engsound->IsSoundPrecached(waveName) returns true, even though the sound is not precached. Because I don't have the patience to compile Sourcemod, I replaced the jnz instruction with nops and now sounds are successfully precached.
For this reason, I removed IsSoundPrecached from PrecacheSoundScript and GetGameSoundParameters

I think it'll be a good idea to add a note to IsSoundPrecached warning that this function may not work correctly(always returning true)

Fixes issue #1170

and removed IsSoundPrecached checks for GetGameSoundParams, too

@asherkin asherkin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice find!

I think it'll be a good idea to add a note to IsSoundPrecached warning that this function may not work correctly (always returning true)

Might as well do that as part of this change (and mark it as #deprecated too).

Comment thread extensions/sdktools/vsound.cpp
@naydef
naydef requested a review from asherkin February 3, 2020 16:26
@naydef

naydef commented Feb 3, 2020

Copy link
Copy Markdown
Contributor Author

I'm not sure what else is required here.

@asherkin asherkin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nothing else is required, this all looks good.

I'll pull it when I'm at a proper computer and can merge it into 1.10 at the same time.

@naydef

naydef commented Feb 3, 2020

Copy link
Copy Markdown
Contributor Author

Alright!

@Headline Headline left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, I'll go ahead and take this now

@Headline
Headline merged commit fba71ed into alliedmodders:master Feb 6, 2020
Headline pushed a commit that referenced this pull request Feb 6, 2020
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.

3 participants