PR#95 namespace collision

Nelarius / imnodes

A small, dependency-free node editor for dear imgui

MIT License

2.01k stars 244 forks source link

PR#95 namespace collision #97

Closed sonoro1234 closed 3 years ago

sonoro1234 commented 3 years ago

Sorry for not realizing this earlier: namespace ImNodes will collide with https://github.com/rokups/ImNodes/blob/02b085dace8bdd815374d6440efb1f70d8a9ebaf/ImNodes.h#L35

Nelarius commented 3 years ago

Drat 😰 Is this preventing you from using the library in a project? Assuming you're using both libraries in the same project? Or is this preventing code generation in cimgui?

sonoro1234 commented 3 years ago

LuaJIT-ImGui is including cimnodes for Nelarius/imnodes and cimnodes_r for rokups/ImNodes (with namespace ImNodes). I have not tried but I guess that cimnodes generation will succed but using both libraries (as LuaJIT-ImGui does) is open to namespace clashes.

Nelarius commented 3 years ago

Oh right, I hadn't thought about the LuaJIT-ImGui use case at all 🤔 Wondering what the options here are.

Doing nothing and keeping things the way they are opens the door to problems in LuaJIT-ImGui
Reverting the namespace back to imnodes. It's inconsistent with the enum constants though, and I really like the way enum constant usage is consistent with ImGui now
Something a bit silly, like changing the namespace to Imnodes in order to be consistent with the CamelCase naming used elsewhere

A bit unfortunate that our two projects have the exact same name 😂 I think @rokups was first, so maybe I should call this ImNodes2 or something 😄

rokups commented 3 years ago

Ehm but why is this a problem? I doubt anyone is going to use both libraries in same codebase.

sonoro1234 commented 3 years ago

Ehm but why is this a problem? I doubt anyone is going to use both libraries in same codebase.

I am doing it in LuaJIT-ImGui

sonoro1234 commented 3 years ago

ImNodes2 or something

ImNodesN ?

Nelarius commented 3 years ago

As unfortunate as the namespace collision is, I'll keep things as they currently are 😞 I never really intended this to be used in the same codebase as other node editors.

sonoro1234 commented 3 years ago

@Nelarius Please, consider taking a look at this solution done by ImGuizmo: https://github.com/CedricGuillemet/ImGuizmo/commit/e1049a960a1ab450d578196f938e73502b91ebaa