Fattorino / ImNodeFlow

Node based editor/blueprints for ImGui
MIT License
119 stars 15 forks source link

CPP20 #16

Closed aiekick closed 3 months ago

aiekick commented 3 months ago

Hello,

please add in your documentation than your lib is requiring c++20 at least.

and maybe other minimal requirements

Fattorino commented 3 months ago

Closed with commit d45ad31

Fattorino commented 3 months ago

C20 is actually just needed for std::erase_if, should I just do it manually and go back to requiring C17 if not lower?

aiekick commented 3 months ago

i seen the std::erase_if.

its like you want.

in my case all my libs are guarantying a minimal of cpp11 (else absolutely needed like for the api filesystem of cpp17), because i noticed than many of my users have project running only on cpp11.

but its like you want, since you are the creator you can do what you want :)

its just appear to me, than all node libs i can find are not exactly what i need, in features, in dependencies, of logics. So i decided to create my own. based on your work, and the ones of @CedricGuillemet , @Nelarius and @Thedmd (i was using this last from 4 years now, and abandonned because tired to merge manually the stacklayout api used by it in imgui myself..)

Fattorino commented 3 months ago

well, thanks for the feedback

aiekick commented 3 months ago

I think about what i said, and i think the true reason for why i support at least cpp11 is : its important for a lib based on and for Dear ImGui to support at least the same standard as ImGui himself. Here the fist not compatible func you are suing is std::erase_if, except that, the rest is ok for cpp11

Fattorino commented 3 months ago

then it's decided, I'll remove the rather unnecessary std::erase_if

Fattorino commented 3 months ago

reopened just for the todo list

Fattorino commented 3 months ago

closed once again with commit 0ed793d1991df938a97b4fbfa5b66877cfc69783

Fattorino commented 3 months ago

Idk where to ask so I'll do it here. @aiekick I'm pretty new to CMake, so I'd appreciate a quick feedback on what I can improve about the CMakeList of this project, thanks

aiekick commented 3 months ago

you CMakeLists.txt is ok to me.

btw i notices than some users want to compile imgui libs as shared, and in this case you need to have some modification like :

ex :

in your CMakeLists.txt


if (BUILD_SHARED_LIBS)
    target_compile_definitions(ImNodeFlow INTERFACE BUILD_IM_NODE_FLOW_SHARED_LIBS)
endif()

if you do that cmake will produce a define :

if you like you can specify that explictly like i done for some old libs ( ex: glad) :

target_compile_definitions(glad PUBLIC IM_NODE_FLOW_EXPORT PRIVATE BUILD_IM_NODE_FLOW_SHARED_LIBS)

if you do that cmake will produce a define :

indeed you can name that as you want :)

in you h you need a specific code just for windows. ex what i do in my code (can be simplified)

#if defined(__WIN32__) || defined(WIN32) || defined(_WIN32) || defined(__WIN64__) || defined(WIN64) || defined(_WIN64) || defined(_MSC_VER)
#if defined(ImNodeFlow_EXPORTS)
  #define IM_NODE_FLOW_API __declspec(dllexport)
#elif defined(BUILD_IM_NODE_FLOW_SHARED_LIBS)
  #define IM_NODE_FLOW_API __declspec(dllimport)
#else
  #define IM_NODE_FLOW_API
#endif
#else
  #define IM_NODE_FLOW_API
#endif

btw i know there is an equivalent code for gcc, but in my tests i have not noticed any requirement to have some declspec(dllexport) orr declspec(dllimport) equivalent code, so maybe for old version of linux :)

Also some interesting trick i used for manage my workflow :

theses methods will send your runtime output to a dedicated dir regarding the config

set_target_properties(${PROJECT} PROPERTIES RUNTIME_OUTPUT_DIRECTORY_DEBUG "${FINAL_BIN_DIR_DEBUG}")
set_target_properties(${PROJECT} PROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELEASE "${FINAL_BIN_DIR_RELEASE}")
set_target_properties(${PROJECT} PROPERTIES RUNTIME_OUTPUT_DIRECTORY_MINSIZEREL "${FINAL_BIN_DIR_DEBUG_MINSIZEREL}")
set_target_properties(${PROJECT} PROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELWITHDEBINFO "${FINAL_BIN_DIR_DEBUG_RELWITHDEBINFO}")

you have that for RUNTIME or LIBRARY

and you can also name your output in the same way :


set_target_properties(${PROJECT} PROPERTIES OUTPUT_NAME_DEBUG "${PROJECT}_${CMAKE_SYSTEM_NAME}_Debug")
set_target_properties(${PROJECT} PROPERTIES OUTPUT_NAME_RELEASE "${PROJECT}_${CMAKE_SYSTEM_NAME}_Release")
set_target_properties(${PROJECT} PROPERTIES OUTPUT_NAME_MINSIZEREL "${PROJECT}_${CMAKE_SYSTEM_NAME}_MinSizeRel")
set_target_properties(${PROJECT} PROPERTIES OUTPUT_NAME_RELWITHDEBINFO "${PROJECT}_${CMAKE_SYSTEM_NAME}_RelWithDebInfo")

in this case CMAKE_SYSTEM_NAME is containing the os name

you can do many more but its just the recent discovering who was helpfull to me :)

Fattorino commented 3 months ago

Thanks a lot, I'll be looking to improve the configuration in the future then