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

Add iOS build support #743

Merged
merged 36 commits into from
Jan 9, 2025
Merged

Add iOS build support #743

merged 36 commits into from
Jan 9, 2025

Conversation

axmo
Copy link
Contributor

@axmo axmo commented Dec 27, 2024

This adds support for running on (and cross-compiling to) iOS devices, as described in #643 (comment)

See ios/README.md for build instructions.

Copy link
Collaborator

@crudelios crudelios left a comment

Choose a reason for hiding this comment

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

Thanks for this!

Here are my small suggestions.

ios/README.md Outdated Show resolved Hide resolved
src/platform/ios/ios.m Outdated Show resolved Hide resolved
axmo and others added 2 commits December 27, 2024 20:16
Co-authored-by: José Cadete <[email protected]>
use snprintf instead of strncpy for safety

Co-authored-by: José Cadete <[email protected]>
@crudelios
Copy link
Collaborator

I'm also thinking: instead of adding the sources to ios/SDL2, can't we use the existing ext/SDL2?

That way the directory convention would be consistent in all builds.

@axmo
Copy link
Contributor Author

axmo commented Dec 28, 2024

I'm also thinking: instead of adding the sources to ios/SDL2, can't we use the existing ext/SDL2?

That way the directory convention would be consistent in all builds.

We could! For iOS I'm building SDL and mixer from source, and I wasn't sure if the other platforms using ext/SDL2 were expecting the source or binary releases there, which is why I put it under ios/SDL2. (Where are other platforms finding SDL2_mixer?)

@crudelios
Copy link
Collaborator

What we usually do is use ext/SDL2/SDL2-2.X.X and ext/SDL2/SDL2_mixer-2.X.X.

Things get a bit messy if you're trying to configure for different platforms from the same base directory, but with some effort it works and, more importantly, it works just fine for github actions.

Speaking of which, the test build is failing. I think it's an include in julius.c which should be inside some#ifdefs but isn't. Can you check please?

Also: do you think you can add a github actions workflow to automate ios builds? If not, can you at least provide command line instructions here for building the app so we later create the workflow ourselves?

Thanks!

@axmo
Copy link
Contributor Author

axmo commented Dec 29, 2024

Ok. for iOS I'm adding the SDL2 and SDL2_mixer CMake projects with add_subdirectory, as suggested in this libSDL forum post, so I'll use ext/SDL2/SDL2/... and ext/SDL2/SDL2_mixer/...

Automated iOS builds for distribution would require putting a signing certificate and provisioning profile on GitHub, which I'm not prepared to do myself, but if you just want to check that it builds correctly, or run tests in a simulator I guess that's doable. A warning: GitHub actions running on macOS cost 10x the price of GitHub actions on Linux, and in my experience builds on GitHub take ages. (that was a few years ago, maybe it has gotten better?)

@crudelios
Copy link
Collaborator

Yeah for now we just want to check that it builds.

The macOS action is free, our mac build is done using it. I think we use a self signing certificate.

@axmo
Copy link
Contributor Author

axmo commented Dec 29, 2024

Ignore previous stuff about the ext/SDL/... location, I've taken a look at /.ci_scripts/ and am adding an install_sdl_ios, which will follow the same directory structure as the others.

axmo added 5 commits December 29, 2024 21:31
- use cmake built-in ios toolchain
- disable MOD support in sdl mixer to avoid missing library error
- add asset catalog for app icon and launch screen
- use iOS-specific Info.plist
- replace hardcoded 'julius' where SHORT_NAME references would be better.
@axmo
Copy link
Contributor Author

axmo commented Dec 31, 2024

Hm. Debug builds for simulator and for device work, but creating a release archive (used for App Store or Ad-Hoc distribution) causes an error:

Wrong SDL_config.h, check your include path?, which seems to be because USING_GENERATED_CONFIG_H is set, but I'm not sure why it would only be set for release builds.

I thought I had everything working, but apparently I was using old build artifacts. Clean builds currently fail when building SDL2. Error: Wrong SDL_config.h, check your include path?, which seems to be because USING_GENERATED_CONFIG_H is set, but I'm not sure WHY it is set.

@crudelios
Copy link
Collaborator

Did you try deleting the SDL2 folder (and adding it brand new) and all intermediate build caches on your PC?

Android studio also has a lot of problems rebuilding, and deleting everything to start fully clean usually fixes them.

@axmo
Copy link
Contributor Author

axmo commented Jan 2, 2025

Aha. looks like I had multiple copies of SDL2 and SDL2_mixer in ext/SDL2 after running install_dependencies, which were all being copied to build/ext and then due to header search path precedence, it was loading the wrong SDL_config.h as a result.

All seems to be working now, and I've added the necessary run_cmake and run_build scripts

@axmo
Copy link
Contributor Author

axmo commented Jan 4, 2025

Please let me know if anything else is needed for this to be considered ready to merge.

src/platform/julius.c Outdated Show resolved Hide resolved
@crudelios crudelios requested a review from bvschaik January 4, 2025 20:42
Copy link
Owner

@bvschaik bvschaik left a comment

Choose a reason for hiding this comment

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

Looking good in general 👍 I mainly have some small questions due to not being familiar with the inner workings of iOS.

.ci_scripts/run_cmake.sh Outdated Show resolved Hide resolved
res/ios/Assets.xcassets/AppIcon.appiconset/julius_1024.png Outdated Show resolved Hide resolved
src/platform/ios/ios.h Outdated Show resolved Hide resolved
src/platform/ios/ios.m Outdated Show resolved Hide resolved
src/platform/julius.c Outdated Show resolved Hide resolved
src/platform/julius.c Outdated Show resolved Hide resolved
src/platform/julius.c Show resolved Hide resolved
src/platform/ios/GameDataPickerController.h Outdated Show resolved Hide resolved
src/platform/ios/GameDataPickerController.m Outdated Show resolved Hide resolved
@crudelios
Copy link
Collaborator

The Visual Studio build is failing with this error:

D:\a\julius\julius\src\platform\ios\ios.h(4,1): error C2773: #import and #using available only in C++ compiler [D:\a\julius\julius\build\julius.vcxproj]
  (compiling source file '../src/platform/julius.c')

Maybe only include ios.h from julius.c if the target build is iOS? It's not ideal, but if that's the only way to prevent the issue, then so be it.

Or, instead of #import, is it possible to use #include?

axmo added 3 commits January 7, 2025 20:42
- show alert message when there's an error importing files
- show alert message if user cancels the game-data document picker
- show the document picker again if game data import was unsuccessful
iOS 14 is required for the document picker methods used.
@axmo
Copy link
Contributor Author

axmo commented Jan 8, 2025

Or, instead of #import, is it possible to use #include?

Ah, yes, it should indeed be #include (and now it is)

@axmo
Copy link
Contributor Author

axmo commented Jan 8, 2025

Once again ready for consideration.

@crudelios
Copy link
Collaborator

Ah nice, the builds are working on all platforms now.

Thinking a bit ahead, you're still planning to add support for augustus, right?

In that case, is it okay if, when this is merged to Julius, I then merge the latest Julius changes to augustus and you work from there?

Copy link
Owner

@bvschaik bvschaik 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, let's merge 👍

@bvschaik bvschaik merged commit 33c7770 into bvschaik:master Jan 9, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants