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

Changed deprecated auto_ptr in AudioDevice to unique_ptr #1463

Closed
wants to merge 1 commit into from
Closed

Changed deprecated auto_ptr in AudioDevice to unique_ptr #1463

wants to merge 1 commit into from

Conversation

JinLisek
Copy link

@JinLisek JinLisek commented Aug 1, 2018

  • Has this change been discussed on the forum or in an issue before?
    Yes, here: https://en.sfml-dev.org/forums/index.php?topic=24313.0
  • Does the code follow the SFML Code Style Guide?
    I guess so.
  • Have you provided some example/test code for your changes?
    None, I've just changed auto_ptr to unique_ptr. I haven't found any unit tests in the repo. I ran cmake with ninja and it compiled.
  • If you have additional steps which need to be performed list them as tasks!
    None

Description

I removed deprecated auto_ptr in AudioDevice, so it no longer produces warnings when compiling with -Werror. The fix is tiny, shouldn't change anything, also the pointer is not even really used, at least in my understanding.

Tasks

  • Tested on Linux
    It compiles on Linux, I haven't tested it on different platforms
  • Tested on Windows
  • Tested on macOS
  • Tested on iOS
  • Tested on Android

How to test this PR?

I don't think it's possible to test, as the pointer is not even used.

@eXpl0it3r
Copy link
Member

eXpl0it3r commented Aug 2, 2018

SFML 2.x is built around C++03 and for backwards compatibility we like to keep it that way.
Since std::unique_ptr was introduced with C++11, we unfortunately can't accept this PR at this point.

Additionally, we've already started to implement some changes for SFML 3, which among others, already replaced std::auto_ptr, see the unstable branch. 😉

Thanks for your contribution! 🙂

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

Successfully merging this pull request may close these issues.

None yet

2 participants