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

Improve/simplify SoundEffects.cpp example #3083

Conversation

vittorioromeo
Copy link
Member

Same principle as #3062 and #3071, basically:

  • Remove unnecessary layers of abstraction
  • Pass dependencies of effects explicitly rather than relying on global state or initialization in constructors
  • Avoid unnecessary usage of std::optional for music resources
  • Minor readability improvements

Note that I still get segfaults in this example, with or without this PR. See #3079 for context.

@vittorioromeo vittorioromeo force-pushed the feature/improve_soundeffects_example branch from 0489699 to 58944d6 Compare June 11, 2024 12:31
@coveralls
Copy link
Collaborator

coveralls commented Jun 11, 2024

Pull Request Test Coverage Report for Build 9465573843

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.5%) to 54.24%

Totals Coverage Status
Change from base Build 9463943212: -1.5%
Covered Lines: 8837
Relevant Lines: 15465

💛 - Coveralls

@SFML SFML deleted a comment from vittorioromeo Jun 12, 2024
@SFML SFML deleted a comment from vittorioromeo Jun 18, 2024
auto-merge was automatically disabled June 23, 2024 23:02

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Planned
Development

Successfully merging this pull request may close these issues.

None yet

2 participants