openframeworks
Replace macros / constants in examples (but with what?)
Related to #5624, but I believe we need to decide what the correct solution is on the user side for our PI, TWO_PI, constants as the feedback for something like glm::pi<float> indicates a need for something simpler.
See also #5163 and #1007 for previous discussions.
@bakercp the PR looks good to me - going to leave one comment in there - but otherwise good to merge.
I think it is also okay to use glm::pi
@ofZach
I think the PR could get merged and we could work on a better user facing solution? ( I don't think #defines for PI etc in a global namespace are tenable ).
float ofPi(); Could be an obvious one? which would just return glm::pi<float>()
sure -- it def feels like we could use a friendly user facing option -- for sure this
glm::pi<float>()
is going to make a lot of beginner oriented code feel a lot less friendly.
I also think this will break alot of old code so we should think about the friendliest solution
one quick thought, I don't know if this reasonable, could we keep these constants in the ofApp class / ofApp namespace as consts, constexpressions etc -- so example and older sample code can stay as is? Then elsewhere, say in another class, you could write ofApp::PI ? The goal would be to not have code break as much as possible and keep things simple. I don't know if this is simpler, or even could work, but just thinking out loud. happy to take a look at this...
Jumping back to the namespace thoughts
Constants in the of namespace
of::Pi
of::HalfPi
of::TwoPi
More nested
of::Math::Pi
of::Math::HalfPi
of::Math::TwoPi
of::Math::Lerp(src, dst, 0.2)
Unity style ( note this is sort of weird, but would mimic the Unity approach, also not sure this can be done in C++ in a non hackey way )
of::Math.Pi
of::Math.TwoPi
of::Math.Lerp(src, dst, 0.2)
@ofZach Either way I think it is safer to just keep the current defines/macros for now ( with a deprecation warning ), but switch over to using the newer api for OF core and examples?
The ofApp solution is clever but it wouldn't account for classes in people's projects or addons and probably better to have an of::Pi ( or whatever we choose ) and warn about PI instead of having ofApp::PI which doesn't fix all use cases and also sort of splits the API in a weird way.
Anyway, it is a great excuse to circle back to the namespace discussion 🙂
my 2 cents, In terms of friendliness PI wins above all. But then, of::Pi or of::PI feels the closest. if using a namespace you can even have in the ofApp.cpp something like using namespace of and you would still able to use PI
Good point @roymacdonald !
Wondering though if of::PI prevents us from holding onto the macros at the same time?
ie: will of::PI and PI clash if we don't remove the #define PI at the same time as adding of::PI and doing using namespace of; ?
If we want to keep PI around for a bit ( to avoid breaking legacy code ), it might make sense to not mirror the old API but do it the way we want:
ie: of::Pi could exist at the same time as PI.
Will try this out though and do a PR for people to test with.
C++20 define std::numbers::pi (float,double and long double). It does not have 2pi.
can we have something more like of::numbers::pi ? So we can have something like :
#if __cplusplus >= 202002L
using namespace std::numbers;
#else
using namespace of::numbers;
#endif
perimeter = 2 * pi * radius;
And that does not prevent using namespace of; elsewhere
now we are using namespace of for of::filesystem maybe it is a good place to have constants too
This PR is meant to minimize the use of Math constants in the core https://github.com/openframeworks/openFrameworks/pull/7413 so some of them can be removed in the future