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

replaced boost::shared_ptr with std::shared_ptr #836

Merged
merged 1 commit into from Apr 23, 2016

Conversation

Projects
None yet
7 participants
@Speciesx
Contributor

Speciesx commented Apr 21, 2016

Since everyone use a newer compiler, i think we could replace boost::shared_ptr with std::shared_ptr.

@tritonas00

This comment has been minimized.

Show comment
Hide comment
@tritonas00

tritonas00 Apr 21, 2016

Collaborator

This means that we no longer need boost as dependency?

Collaborator

tritonas00 commented Apr 21, 2016

This means that we no longer need boost as dependency?

@Speciesx

This comment has been minimized.

Show comment
Hide comment
@Speciesx

Speciesx Apr 21, 2016

Contributor

no, there is still boost::regex, boost::lexical and boost::optional.

Contributor

Speciesx commented Apr 21, 2016

no, there is still boost::regex, boost::lexical and boost::optional.

@tritonas00

This comment has been minimized.

Show comment
Hide comment
@tritonas00

tritonas00 Apr 21, 2016

Collaborator

And those can't be replaced?

Collaborator

tritonas00 commented Apr 21, 2016

And those can't be replaced?

@Speciesx

This comment has been minimized.

Show comment
Hide comment
@Speciesx

Speciesx Apr 21, 2016

Contributor

I tried to replaceboost::regex, boost::smatch andboost::ssub_match, but ror doesn't start after this change.

Contributor

Speciesx commented Apr 21, 2016

I tried to replaceboost::regex, boost::smatch andboost::ssub_match, but ror doesn't start after this change.

@only-a-ptr

This comment has been minimized.

Show comment
Hide comment
@only-a-ptr

only-a-ptr Apr 22, 2016

Member

there is still boost::regex, boost::lexical and boost::optional.

Whaaat? 😄 Who put that in there? Can we throw it out? Please investigate.

One note: Quite a few edited files belong to RigEditor, which is obsolete. I won't continue it, I want the editor be written in Python (or some other scripting lang) rather than C++. Could anyone please remove all RigEditor code? Things to look into:

  • Main menu (GUI button + startup logic)
  • GUI panels (source/main/gui/panels/GUI_RigEditor*)
  • Logic (source/main/rig_editor)
Member

only-a-ptr commented Apr 22, 2016

there is still boost::regex, boost::lexical and boost::optional.

Whaaat? 😄 Who put that in there? Can we throw it out? Please investigate.

One note: Quite a few edited files belong to RigEditor, which is obsolete. I won't continue it, I want the editor be written in Python (or some other scripting lang) rather than C++. Could anyone please remove all RigEditor code? Things to look into:

  • Main menu (GUI button + startup logic)
  • GUI panels (source/main/gui/panels/GUI_RigEditor*)
  • Logic (source/main/rig_editor)
@tritonas00

This comment has been minimized.

Show comment
Hide comment
@tritonas00

tritonas00 Apr 22, 2016

Collaborator

I remember i compiled 0.38 without boost and caleum by removing a few lines. Game worked fine.

But caleum needs boost i think

https://github.com/RigsOfRods/ogre-caelum/blob/master/CMakeDependenciesConfig.txt#L17

Collaborator

tritonas00 commented Apr 22, 2016

I remember i compiled 0.38 without boost and caleum by removing a few lines. Game worked fine.

But caleum needs boost i think

https://github.com/RigsOfRods/ogre-caelum/blob/master/CMakeDependenciesConfig.txt#L17

@mikadou

This comment has been minimized.

Show comment
Hide comment
@mikadou

mikadou Apr 22, 2016

Contributor

Whaaat? 😄 Who put that in there? Can we throw it out? Please investigate.

I have introduced boost::optional in the collision code. Replacing boost::shared_ptr with std::shared_ptr makes sense as it is now available in the official standard. For other stuff not (yet) in the standard using Boost is fine in my opinion.

Contributor

mikadou commented Apr 22, 2016

Whaaat? 😄 Who put that in there? Can we throw it out? Please investigate.

I have introduced boost::optional in the collision code. Replacing boost::shared_ptr with std::shared_ptr makes sense as it is now available in the official standard. For other stuff not (yet) in the standard using Boost is fine in my opinion.

@Speciesx

This comment has been minimized.

Show comment
Hide comment
@Speciesx

Speciesx Apr 22, 2016

Contributor

Replacing boost::shared_ptr with std::shared_ptr makes sense as it is now available in the official standard. For other stuff not (yet) in the standard using Boost is fine in my opinion.

regex is also available as standard.

Contributor

Speciesx commented Apr 22, 2016

Replacing boost::shared_ptr with std::shared_ptr makes sense as it is now available in the official standard. For other stuff not (yet) in the standard using Boost is fine in my opinion.

regex is also available as standard.

@Hiradur

This comment has been minimized.

Show comment
Hide comment
@Hiradur

Hiradur Apr 23, 2016

Contributor

@Speciesx Can you upload your diff to the master branch for your changes not in this pull request so we can take a look?

Contributor

Hiradur commented Apr 23, 2016

@Speciesx Can you upload your diff to the master branch for your changes not in this pull request so we can take a look?

@monwarez

This comment has been minimized.

Show comment
Hide comment
@monwarez

monwarez Apr 23, 2016

Contributor

By the way, do c++1y (a.k.a c++14) is too young or not ? So that we can replace for example auto def = std::shared_ptr<RigDef::File>(new RigDef::File()); by auto def = std::make_shared<RigDef::File>();

Contributor

monwarez commented Apr 23, 2016

By the way, do c++1y (a.k.a c++14) is too young or not ? So that we can replace for example auto def = std::shared_ptr<RigDef::File>(new RigDef::File()); by auto def = std::make_shared<RigDef::File>();

@Hiradur

This comment has been minimized.

Show comment
Hide comment
@Hiradur

Hiradur Apr 23, 2016

Contributor

@monwarez Switching to C++14 is too early IMO. I think a good measure is if the default compiler in Debian stable (gcc 4.9) has full support for the standard. That is not the case (gcc 5 has full support for C++14 [1]).
There are also the older Ubuntu LTS versions 12.04 (gcc 4.6, but this one dropped out already since it doesn't support C++11 fully) and 14.04 (gcc 4.8). At least the LTS release before the current LTS release from Ubuntu should be supported.

[1] https://gcc.gnu.org/projects/cxx-status.html#cxx14

Contributor

Hiradur commented Apr 23, 2016

@monwarez Switching to C++14 is too early IMO. I think a good measure is if the default compiler in Debian stable (gcc 4.9) has full support for the standard. That is not the case (gcc 5 has full support for C++14 [1]).
There are also the older Ubuntu LTS versions 12.04 (gcc 4.6, but this one dropped out already since it doesn't support C++11 fully) and 14.04 (gcc 4.8). At least the LTS release before the current LTS release from Ubuntu should be supported.

[1] https://gcc.gnu.org/projects/cxx-status.html#cxx14

@only-a-ptr

This comment has been minimized.

Show comment
Hide comment
@only-a-ptr

only-a-ptr Apr 23, 2016

Member

Hmm, this makes me want to write C++ usage guidelines for RoR, such as these: http://llvm.org/docs/CodingStandards.html. I'll create an issue ticket on it.

@mikadou Please don't introduce any more boost.

EDIT: Please discuss here: #839

Member

only-a-ptr commented Apr 23, 2016

Hmm, this makes me want to write C++ usage guidelines for RoR, such as these: http://llvm.org/docs/CodingStandards.html. I'll create an issue ticket on it.

@mikadou Please don't introduce any more boost.

EDIT: Please discuss here: #839

@ulteq ulteq merged commit 14eb7e9 into RigsOfRods:master Apr 23, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Speciesx Speciesx deleted the Speciesx:boost-to-std branch May 1, 2016

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