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

Add #undef PI #336

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add #undef PI #336

wants to merge 1 commit into from

Conversation

codereptile
Copy link

Some other libraries define PI, which creates a conflict. This will implicitly resolve this problem. (It can be resolved explicitly, by adding this line of code before include, but that's extremely annoying)

Some other libraries define PI, which creates a conflict. This will implicitly resolve this problem. (It can be resolved explicitly, by adding this line of code before include, but that's extremely annoying)
@codereptile
Copy link
Author

Just wanted to say that this came from Unreal Engine, where it defines PI too

@chiphogg
Copy link

I feel the pain here --- we recently hit the exact same problem in Au. This conversation thread may be relevant for considering the options for nholthaus/units.

Unfortunately, as that thread points out, even if this landed it would be a fragile solution, because it would depend on the order of includes. As far as I can tell, if you're using a library that defines a macro for PI (such as Unreal Engine, or the Arduino core library), then there's just no good solution for defining the symbol PI anywhere in your library. We ended up deprecating it, and we'll remove it after it's been deprecated for a full minor release cycle.

It's certainly frustrating that people would define short-named macros like this in more modern and enlightened times, but if we want to support those libraries, I guess we're stuck with it!

@codereptile
Copy link
Author

@chiphogg Well, this wasn't much pain for me, a simple undef kind of solved it, but in the end I still switched to Au. By the way, I've started making a wrapper for UE with it, and so far it looks REALLY good. Especially since Unreal Engine uses centimeters as base unit for length, which is a pain in any physics formula. Being able to call methods and get results in any units turned out to be extremely useful. What do you think about this idea?)

@chiphogg
Copy link

Wow, really glad to hear Au's working well for you!

What do you think about this idea?

(Assuming "this idea" means making a wrapper for UE that uses Au)

If UE uses cm throughout for length, but has raw numeric types in its interfaces... oof. In that case, yes, I think a set of "wrapper" APIs, which let users pass Quantity types (or receive them as return values), and then simply forward the underlying values to/from the UE APIs, would be the way to go --- I think it's what I'd do too.

@codereptile
Copy link
Author

@chiphogg Yea "oof" kind of sums it up. I was wondering if we could advertise that as one of the examples for Au usage.

@chiphogg
Copy link

Yeah, that sounds like a really compelling use case!

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

Successfully merging this pull request may close these issues.

2 participants