8000 Consider using `nlohmann::json` instead of just `json` · Issue #406 · syoyo/tinygltf · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Consider using nlohmann::json instead of just json #406

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.

Sign up for GitHub

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

Jump to bottom
Closed
DavidSM64 opened this issue Feb 17, 2023 · 2 comments
Closed

Consider using nlohmann::json instead of just json #406

DavidSM64 opened this issue Feb 17, 2023 · 2 comments
Labels
enhancement

Comments

@DavidSM64
Copy link
Contributor

Describe the issue

In my project I have been using the SimpleJSON library (https://github.com/nbsdx/SimpleJSON) for creating/loading json files, which uses the namespace json for its classes. When trying to use the tinygltf library I get ambiguity errors because you are also using json. I solved this issue in my own project by changing json to nlohmann::json in tiny_gltf.h

To Reproduce

  • OS: Ubuntu (Under WSL2)
  • Compiler: g++

Expected behaviour

Changing json to nlohmann::json in the nlohmann implementation fixes the problem. I know it would be annoying, but it should help resolve future conflicts.
@syoyo
Copy link
Owner
syoyo commented Feb 17, 2023

Oh, I see!

json namespace is not exposed to tinygltf public API, so it should be safe to rename it(for example, json_backend)

Also, we'd be better to wrap some implementation code with detail(or internal) namespace(instead of anonymous namespace

tinygltf/tiny_gltf.h

Line 1708 in 6dba104

namespace {
) to avoid potential namespace conflicts issue further. i.e.

namespace tinygltf {
namespace detail {

using json_backend = nlohnann::json;

...

void JsonParse(...) {
  doc = json_backend::parse(...)
}

} // detail
} // tinygltf

PR is much appreciated!

@DavidSM64 DavidSM64 mentioned this issue Feb 18, 2023
Merged
@syoyo
Copy link
Owner
syoyo commented Feb 18, 2023

🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@syoyo @DavidSM64
0