search

syoyo / tinygltf

Header only C++11 tiny glTF 2.0 library
MIT License
2.01k stars 409 forks source link

Added detail namespace to prevent json namespace conflicts #407

Closed DavidSM64 closed 1 year ago

DavidSM64 commented 1 year ago

Fixes issue #406

Ran parser & unit tests, and it all passed.

Don't even need to rename json in this case. For some weird reason it has to use ::detail::json instead of detail::json. I have no idea why.

syoyo commented 1 year ago

@DavidSM64 Thanks! But it fails to compile with rapidjson backend.

Also, let me give some to resolve ::detail namespace scope issue. We should be able to make it details::

DavidSM64 commented 1 year ago

RapidJSON should be fixed now hopefully. Going to look into the ::detail issue.

Edit: Ah, found the issue. Forgot to add namespace tinygltf { above a namespace detail {. This should be good to go now if you don't have any other issues with it.

syoyo commented 1 year ago

@DavidSM64 Thanks!

RapidJSON should be fixed now hopefully.

Awesome!

Forgot to add namespace tinygltf { above a namespace detail {

Oh, some JSON stuff was defined in global unnamed namespace scope and not wrapped by tinygltf namespace 😅

https://en.cppreference.com/w/cpp/language/namespace#Unnamed_namespaces

Your PR makes it better.

syoyo commented 1 year ago

@DavidSM64 Confirmed CI build passes, so merged! Thank you for the contribution!