Skip to content
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.

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

Change extension folder structure to reduce file name conflicts #1647

Merged
merged 6 commits into from
Feb 3, 2024

Conversation

rh101
Copy link
Contributor

@rh101 rh101 commented Feb 2, 2024

Which branch your pull-request should merge into?

  • 2.1: BugFixes and Improvements for Current LTS branch

Describe your changes

Moved each extension into its own "src" folder.
Each extension CMakeList.txt now sets up the include paths for that extension.
LUA bindings generator updated, but this needs to be checked.
Fix issue with detecting Effekseer and DrawNodeEx extensions in cpp-tests.

Note that I manually updated the auto-generated lua bindings, so I haven't yet tested the changes to the LUA binding generator to see if it does generate the correct output. Hopefully that is the case, but if not, then any help with that specific part would be appreciated.

It seems that a few existing issues surfaced when the extension folder structure was changed, such as header files to inactive extensions being included when they shouldn't be.

FYI, the changes in this PR shouldn't break any existing code. If anything, it would be a few include statements that would need to be fixed up in a project, assuming it uses extensions.

Issue ticket number and link

#1645

Checklist before requesting a review

For each PR

  • Add Copyright if it missed:
    - "Copyright (c) 2019-present Axmol Engine contributors (see AUTHORS.md)."

  • I have performed a self-review of my code.

    Optional:

    • I have checked readme and add important infos to this PR.
    • I have added/adapted some tests too.

For core/new feature PR

  • I have checked readme and add important infos to this PR.
  • I have added thorough tests.

@rh101 rh101 marked this pull request as draft February 2, 2024 09:22
@rh101
Copy link
Contributor Author

rh101 commented Feb 2, 2024

There are issues with the LUA bindings generator, so attempting to fix that in order to complete this PR.

@rh101 rh101 marked this pull request as ready for review February 2, 2024 10:17
@rh101
Copy link
Contributor Author

rh101 commented Feb 2, 2024

The LUA binding generator should be working now. If there are any concerns or suggestions, then I'll get around to having a look at them in 2 days time.

@halx99 halx99 linked an issue Feb 2, 2024 that may be closed by this pull request
@halx99
Copy link
Collaborator

halx99 commented Feb 2, 2024

lgtm

@rh101
Copy link
Contributor Author

rh101 commented Feb 2, 2024

Hold off on any merging. One last thing to do, which is fixing the pre-built paths in AXLinkHelpers.cmake.

EDIT: All done. Commit d894c54 should also fix the issue with yasio that has been reported on the Discord server (missing header search path and linker error with pre-builts).

Set correct paths to extensions for prebuilt option
@halx99 halx99 added the enhancement New feature or request label Feb 3, 2024
@halx99 halx99 added this to the 2.1.2 milestone Feb 3, 2024
@halx99 halx99 merged commit 9f8962d into axmolengine:2.1 Feb 3, 2024
8 checks passed
@halx99
Copy link
Collaborator

halx99 commented Feb 3, 2024

@rh101 the bot try override luabindings: #1648

@rh101 rh101 deleted the extensions-structure branch February 3, 2024 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Axmol extension's includes conflict with project's includes
2 participants