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

[iOS] Fix building as static library or xcframework, add iOS config and xcframework build script to the test project. #1302

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Nov 5, 2023

  • Adds dummy argument to new/delete operations to avoid conflicts with the equivalent engine symbols when static linking (on iOS both engine and extension can be static library).
  • Adds example.gdextension config and build script for iOS .xcframework (device + simulator) build.
  • Also fixes macOS framework names, and adds missing arch for Linux/Windows to the example.gdextension.

Bugsquad edit: Depends on godotengine/godot#84493

@bruvzg bruvzg added bug This has been identified as a bug platform:ios labels Nov 5, 2023
@bruvzg bruvzg force-pushed the ios_static branch 2 times, most recently from c5d29d3 to 30704d8 Compare November 7, 2023 05:54
@bruvzg bruvzg marked this pull request as ready for review November 7, 2023 07:18
@bruvzg bruvzg requested a review from a team as a code owner November 7, 2023 07:18
@dsnopek
Copy link
Collaborator

dsnopek commented Nov 8, 2023

Thanks!

Having to add the dummy arguments to new and delete isn't ideal, but I don't know that there's any better way to do it, and since we only ever use macros to run them, this should be fine.

However, it looks like this needs to be rebased - there's a conflict

@akien-mga akien-mga added this to the 4.2 milestone Nov 9, 2023
@akien-mga
Copy link
Member

akien-mga commented Nov 9, 2023

Might be good to add a comment to explain what these dummy arguments are for.

@dsnopek This shouldn't be cherry-picked for 4.1 unless godotengine/godot#84493 also is, but for now it's unclear whether it will be merged in time even for 4.2.

…nd xcframework build script to the test project.
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@akien-mga
Copy link
Member

CI is failing on the Linux (GCC) build, for some reason it doesn't want to pick up a runner and seems to be stuck on running the unit tests. Two attempts failed the same way.

This doesn't seem due to this PR specifically, it was likely just opened at a time where GHA was misbehaving and now it's stuck in a bad state. Other tests passed so this should be mergeable.

@akien-mga akien-mga merged commit adb0cfc into godotengine:master Nov 10, 2023
11 of 12 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug platform:ios
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants