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

Fix e2 soundcore & sound emitter from not playing soundscripts & sounds with sound characters. #3125

Merged
merged 5 commits into from
Sep 6, 2024

Conversation

unknao
Copy link
Contributor

@unknao unknao commented Aug 18, 2024

Fixes a design oversight introduced by #3122 not accounting for soundscripts & sound paths with sound characters in them.

@StrawWagen
Copy link

This should be merged, already ran into this bug lol

@unknao
Copy link
Contributor Author

unknao commented Aug 23, 2024

@thegrb93 if you are still concerned about the pattern not working, here's an image of it working.
image

@thegrb93
Copy link
Contributor

It's not removing those characters for the string that goes into the sound functions though, just the string that checks if the path exists.

@thegrb93
Copy link
Contributor

thegrb93 commented Aug 26, 2024

Also I'm pretty sure sure some non-alphanumerical characters are allowed for file paths. I'm not sure what removing them from the front of the string accomplishes

@unknao
Copy link
Contributor Author

unknao commented Aug 31, 2024

who names their sound directories in the way that would purposely break sound characters?

@thegrb93
Copy link
Contributor

Let me know if my commit works for ya, and can merge if so

@unknao
Copy link
Contributor Author

unknao commented Sep 2, 2024

Let me know if my commit works for ya, and can merge if so

Nope, sound characters don't work.

@thegrb93
Copy link
Contributor

thegrb93 commented Sep 2, 2024

Let me know if my commit works for ya, and can merge if so

Nope, sound characters don't work.

what sound character?

@unknao
Copy link
Contributor Author

unknao commented Sep 3, 2024

what sound character?

i tried both ^ and )

@thegrb93
Copy link
Contributor

thegrb93 commented Sep 3, 2024

and what about those doesn't work?

@thegrb93
Copy link
Contributor

thegrb93 commented Sep 3, 2024

What are sound characters? Some kind of syntax in the path?

@unknao
Copy link
Contributor Author

unknao commented Sep 6, 2024

What are sound characters? Some kind of syntax in the path?

https://developer.valvesoftware.com/wiki/Soundscripts See the sound characters section.

@thegrb93
Copy link
Contributor

thegrb93 commented Sep 6, 2024

Those aren't things the sound-emitter or e2 sound lib should care about.

@thegrb93 thegrb93 merged commit 5c5f3dd into wiremod:master Sep 6, 2024
1 check failed
@unknao
Copy link
Contributor Author

unknao commented Sep 6, 2024

Those aren't things the sound-emitter or e2 sound lib should care about.

And yet it does, why choose to break something that has always worked in e2 and sound emitter before?

@thegrb93
Copy link
Contributor

thegrb93 commented Sep 6, 2024

Those should be handled another way, not using the sound path. Valve are fucking morons for doing it that way. You can add some inputs or new logic somewhere that handles them, but wire users shouldn't have to manipulate the sound path like that for different effects.

@unknao
Copy link
Contributor Author

unknao commented Sep 6, 2024

Why change something that's worked for years and years with the justification that you can just bloat e2 later to fix the issue?
It's not impossible to use the e2 helper to inform the people who would benefit from such information instead of just removing the functionality altogether.

@thegrb93
Copy link
Contributor

thegrb93 commented Sep 6, 2024

Because it should be done in a better way and this incentivizes improving it.

@unknao
Copy link
Contributor Author

unknao commented Sep 6, 2024

This is the hand the source engine has dealt us, you don't need to fix what's not broken.

@thegrb93
Copy link
Contributor

thegrb93 commented Sep 6, 2024

What effects are actually needed?

@thegrb93
Copy link
Contributor

thegrb93 commented Sep 6, 2024

Is there no other way to do those effects in glua?

@thegrb93
Copy link
Contributor

thegrb93 commented Sep 6, 2024

It looks like the '?' char was already banned due to crashing or something. I'm surprised the other chars were even allowed.

@unknao
Copy link
Contributor Author

unknao commented Sep 6, 2024

So when i try to change something that's backwards compatible (like in #2693) i get told for it immediately but when you do it it's ok?

@unknao
Copy link
Contributor Author

unknao commented Sep 6, 2024

It's not as if some new method of controlling sound characters can't coexist with the way sound characters have been used for 10+ years now.

@unknao
Copy link
Contributor Author

unknao commented Sep 6, 2024

What effects are actually needed?

Everything that doesn't crash the game is well and good.

@thegrb93
Copy link
Contributor

thegrb93 commented Sep 6, 2024

So when i try to change something that's backwards compatible (like in #2693) i get told for it immediately but when you do it it's ok?

The file.Exists change broke it, not me. We need a fix that can verify both the flags and the path.

@unknao unknao deleted the soundcorefix branch December 24, 2024 08:08
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