-
Notifications
You must be signed in to change notification settings - Fork 5k
Return stereo for music/ambient #89470
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't you literally removing sound reverb and echo here?
code/game/sound.dm
Outdated
@@ -1,5 +1,7 @@ | |||
|
|||
///Default override for echo | |||
// TFF CHANGE START - FIX STEREO-AUDIO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uuuh, did you PR this from a modular codebase?
code/game/sound.dm
Outdated
@@ -176,11 +180,13 @@ | |||
else | |||
var/area/A = get_area(src) | |||
sound_to_use.environment = A.sound_environment | |||
|
|||
// TFF CHANGE START - FIX STEREO-AUDIO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here?
Not really - without this override, the byond uses a standard environment (probably "generic") that has echo and reverb in it. And no, this change is not made solely for the sake of the modular codebase. It's just that I... I've seen that when modifying core-files they do that, that's why I put “TFF CHANGE”, but it seems to be only misleading ._." |
// X CHANGE is a modular codebase thing, we don't use it - if you remove code, delete it, not comment it out |
For some reason, this actually has been tested and works, making sounds play in stereo instead of mono. Thanks BYOND. EDIT: echo being defined for a sound causes it to downmix to mono. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Ok my understanding of this was completely wrong
As it turns out modifying echo OR environment downmixes to mono in any situation. I guess that makes sense |
Assuming this code is combined with the PR's existing code, this will do nothing because it is erroring, as |
Please remove [FIX] from your PR title. The Changelog makes that apparent. Also update your branch with master to restart icon diff bot |
## About The Pull Request Returns stereo sound for music and ambient, which was changed here => tgstation#55333 Due to the override environment (probably) absolutely ALL music was downmixed to mono, such as music via global sound, direct sound to mob and megafauna themes. In addition to this, the sound from musical instruments at certain positioning was sent exclusively to one ear, which made it extremely uncomfortable to listen to with headphones. Commenting of 183-189 lines was done to make proc local_sound work correctly, otherwise an error occurs. Video from the tests is attached, old - what we have at the moment; new - what we get when commenting on the lines. https://drive.google.com/file/d/1wSZ2I5XE74nePOTfMmaj-xqmonf3g58O/view (too much weight, had to upload it on google-drive, I apologize for that.) ## Why It's Good For The Game Fixes the bug? Based on the PR with override environment, downmixing music in mono was not planned. Makes working with music and listening to it more comfortable ## Changelog :cl: fix: remove downmix stereo=>mono where it is not required /:cl:
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may not be viewable. --> <!-- You can view Contributing.MD for a detailed description of the pull request process. --> ## About The Pull Request <!-- Describe The Pull Request. Please be sure every change is documented or this can delay review and even discourage maintainers from merging your PR! --> Ports the following TG station PRs: - [x] tgstation/tgstation#59667 - [x] tgstation/tgstation#61573 - [x] tgstation/tgstation#67455 - [x] tgstation/tgstation#72805 - [x] tgstation/tgstation#74170 - [x] tgstation/tgstation#74650 - [x] tgstation/tgstation#75691 - [x] tgstation/tgstation#76453 - [x] tgstation/tgstation#76751 - [x] tgstation/tgstation#81135 - [x] tgstation/tgstation#81923 - [x] tgstation/tgstation#82152 - [x] tgstation/tgstation#85953 - [x] tgstation/tgstation#85967 - [x] tgstation/tgstation#89470 - [x] tgstation/tgstation#89777 - [x] tgstation/tgstation#90330 ## Why It's Good For The Game <!-- Argue for the merits of your changes and how they benefit the game, especially if they are controversial and/or far reaching. If you can't actually explain WHY what you are doing will improve the game, then it probably isn't good for the game in the first place. --> jukebox fixes + audio stuff is good... ## Testing Photographs and Procedure <!-- Include any screenshots/videos/debugging steps of the modified code functioning successfully, ideally including edge cases. --> <!-- You can uncomment line 1 @ _maps/_basemap.dm to boot up a test map that loads much faster. --> <details> <summary>Screenshots&Videos</summary> https://github.com/user-attachments/assets/c5ef5f9c-0f8b-4b16-a9aa-e6a36c04443f </details> ## Changelog <!-- If your PR modifies aspects of the game that can be concretely observed by players or admins you should add a changelog. If your change does NOT meet this description, remove this section. Be sure to properly mark your PRs to prevent unnecessary GBP loss. You can read up on GBP and its effects on PRs in the tgstation guides for contributors. Please note that maintainers freely reserve the right to remove and add tags should they deem it appropriate. You can attempt to finagle the system all you want, but it's best to shoot for clear communication right off the bat. --> :cl: XeonMations, JJRcop add: Added a new verb that players can request audio from admins with add: Admins can now play internet links. config: More config changes related to roundstart and roundend music. /:cl: <!-- Both :cl:'s are required for the changelog to work! You can put your name to the right of the first :cl: if you want to overwrite your GitHub username as author ingame. --> <!-- You can use multiple of the same prefix (they're only used for the icon ingame) and delete the unneeded ones. Despite some of the tags, changelogs should generally represent how a player might be affected by the changes rather than a summary of the PR's contents. --> --------- Co-authored-by: Jonathan Rubenstein <jrubcop@gmail.com> Co-authored-by: san7890 <the@san7890.com> Co-authored-by: Kylerace <kylerlumpkin1@gmail.com> Co-authored-by: SomeRandomOwl <2568378+SomeRandomOwl@users.noreply.github.com> Co-authored-by: Fikou <23585223+Fikou@users.noreply.github.com> Co-authored-by: Aleksej Komarov <stylemistake@gmail.com> Co-authored-by: Watermelon914 <37270891+Watermelon914@users.noreply.github.com> Co-authored-by: MrMelbert <51863163+MrMelbert@users.noreply.github.com> Co-authored-by: John Willard <53777086+JohnFulpWillard@users.noreply.github.com> Co-authored-by: PeriodicChaos <44913068+PeriodicChaos@users.noreply.github.com> Co-authored-by: EnnyDaiz <160051896+EnnyDaiz@users.noreply.github.com> Co-authored-by: harry <me@harryob.live> Co-authored-by: harryob <55142896+harryob@users.noreply.github.com>
About The Pull Request
Returns stereo sound for music and ambient, which was changed here =>
#55333
Due to the override environment (probably) absolutely ALL music was downmixed to mono, such as music via global sound, direct sound to mob and megafauna themes.
In addition to this, the sound from musical instruments at certain positioning was sent exclusively to one ear, which made it extremely uncomfortable to listen to with headphones.
Commenting of 183-189 lines was done to make proc local_sound work correctly, otherwise an error occurs.
Video from the tests is attached, old - what we have at the moment; new - what we get when commenting on the lines.
https://drive.google.com/file/d/1wSZ2I5XE74nePOTfMmaj-xqmonf3g58O/view
(too much weight, had to upload it on google-drive, I apologize for that.)
Why It's Good For The Game
Fixes the bug? Based on the PR with override environment, downmixing music in mono was not planned.
Makes working with music and listening to it more comfortable
Changelog
🆑
fix: remove downmix stereo=>mono where it is not required
/:cl: