axmolengine / axmol

Axmol Engine – A Multi-platform Engine for Desktop, XBOX (UWP) and Mobile games. (A fork of Cocos2d-x-4.0)
https://axmol.dev
MIT License
868 stars 195 forks source link

Axmol extension's includes conflict with project's includes #1645

Closed smilediver closed 7 months ago

smilediver commented 8 months ago

Axmol's core/CMakeLists.txt has this:

target_include_directories(${_AX_CORE_LIB}
    ...
    PUBLIC ${_AX_ROOT}/extensions
    ...
    INTERFACE ${_AX_ROOT}/extensions

This exposes all files in extensions folders to the project even if those extensions are disabled. The problem with this setup is that it can conflict with the custom libraries inside the project or with the project itself.

For example, we have a custom separate ImGui and Spine libraries integrated into our projects, and including spine/SkeletonAnimation.h leads to problems. Since ${_AX_ROOT}/extensions is added to the include paths, the project ends up including ${_AX_ROOT}/extensions/spine/SkeletonAnimation.h header and not ours.

I think ${_AX_ROOT}/extensions should be removed from include paths, and each extension should add its own include paths, and only if the extension is enabled. Headers like ${_AX_ROOT}/extensions/spine/SkeletonAnimation.h should be moved to something like ${_AX_ROOT}/extensions/spine/includes/spine/SkeletonAnimation.h, and ${_AX_ROOT}/extensions/spine/includes path should be added to the project only if Spine extension is turned on.

Note that this applies to all extensions, not just Spine.

rh101 commented 8 months ago

I agree with the issue raised here, as I've faced it a few times before, and while I've so far managed to change the folder structure in my own project to avoid it, fixing it in the engine source would be a more appropriate solution.

@halx99 I have some time today to take a look at resolving this if it would help.

rh101 commented 7 months ago

Will have a PR up soon with the changes.

rh101 commented 7 months ago

I ran into a few issues, some of which I think I've managed to sort out, but one that I'm confused about, which is regarding the usage of __has_include:

#if __has_include("DrawNodeExTest/DrawNodeExTest.h") and #if __has_include("EffekseerForCocos2d-x.h")

__has_include will search for the file, and if it exists, the result is true, so in this case, it's always true, whether or not that extension is enabled. Going by the information here, the current usage of __has_include for Effekseer and DrawNodeEx is not correct. Pre-processor defines would be better suited for this, ones which are defined if and only if those extensions are enabled.

halx99 commented 7 months ago

I ran into a few issues, some of which I think I've managed to sort out, but one that I'm confused about, which is regarding the usage of __has_include:

#if __has_include("DrawNodeExTest/DrawNodeExTest.h") and #if __has_include("EffekseerForCocos2d-x.h")

__has_include will search for the file, and if it exists, the result is true, so in this case, it's always true, whether or not that extension is enabled. Going by the information here, the current usage of __has_include for Effekseer and DrawNodeEx is not correct. Pre-processor defines would be better suited for this, ones which are defined if and only if those extensions are enabled.

absolutely agree

rh101 commented 7 months ago

@halx99 What are the steps required to generate the LUA bindings? I need to check if the auto-generated code is correct.

EDIT: Never mind! I figured it out.