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.

Sign up for GitHub

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

Jump to bottom

Initial Sound Implementation #1325

Merged
merged 20 commits into from
May 19, 2022
Merged

Initial Sound Implementation #1325

merged 20 commits into from
May 19, 2022

Conversation

Ziemas
Copy link
Contributor

@Ziemas Ziemas commented Apr 20, 2022 •
edited
Loading

TODO

  • Fix up naming in 989snd stuff to match rest of project
  • SFX's with Flag bit 1 need to stop previous instances
  • Make bank unloading more robust, it needs to delete all the voices associated with the bank. (Currently trips asan due to use after free)
  • setvolpan impl for midi

Probably more that i forgot.

maybe

  • RE effect system (pitchbend/pitchmod) (seems unused though?)
  • separate out the audio callback from 989snd (probably belongs in common code)

@Ziemas Ziemas force-pushed the sound branch 7 times, most recently from f9d9fc7 to 1354177 Compare April 25, 2022 03:50
Ziemas added 16 commits May 7, 2022 20:49
@xTVaser xTVaser marked this pull request as ready for review May 19, 2022 20:41
@xTVaser xTVaser changed the title wip Sound Initial Sound Implementation May 19, 2022
xTVaser
xTVaser approved these changes May 19, 2022
View reviewed changes
Copy link
Member

@xTVaser xTVaser left a comment

Choose a reason for hiding this comment

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

Looks good to me on Windows. Game booted properly and sound/music works.

The only issue I found was this #1362 but it's a problem on master as well.

CI is passing, codacy can be ignored / cleaned up later.

@xTVaser xTVaser merged commit 07cc0dd into open-goal:master May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xTVaser xTVaser xTVaser approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Ziemas @xTVaser