-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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 C# iOS support #82729
Add C# iOS support #82729
Conversation
if (err != OK) { | ||
return nullptr; | ||
} | ||
Error err = OS::get_singleton()->open_dynamic_library(native_aot_so_path, r_aot_dll_handle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is because open_dynamic_library
already does path finding logic to figure out where the file actually is, depending on the platform it's running in. There's no point in doing the check here, as we don't actualliy know what the path actually is at this point.
// Check that the .NET assemblies directory exists before trying to use it. | ||
if (!DirAccess::exists(GodotSharpDirs::get_api_assemblies_dir())) { | ||
OS::get_singleton()->alert(vformat(RTR("Unable to find the .NET assemblies directory.\nMake sure the '%s' directory exists and contains the .NET assemblies."), GodotSharpDirs::get_api_assemblies_dir()), RTR(".NET assemblies not found")); | ||
ERR_FAIL_MSG(".NET: Assemblies not found"); | ||
} | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For NativeAOT, this check isn't right, as we're not actually going to be loading managed assemblies from this particular location. It's not really making anything safer, other parts of the code do path finding to try and find the correct locations for libraries. We don't know whether we're doing nativeaot yet at this point, so we can't even optionally do this only for non-nativeaot. It's going to make it harder to optionally enable nativeaot later for platforms that support both native and managed libraries, so we might want to revisit this check in the future.
Just FYI the CI doesn't go past static checks because there appears to be a typo 🙃
|
gah, and I ran it three times to make sure! jeez 🙈 |
1a3ecd4
to
2c28442
Compare
</PropertyGroup> | ||
|
||
<ItemGroup Condition="$(RuntimeIdentifier.Contains('iossimulator'))"> | ||
<LinkerArg Include="-isysroot /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally the path to xcode would come from xcode-select
, instead of being hardcoded here. However, we can't easily run xcode-select
before this item group is evaluated (we would have to do it as a target before ilccompile and parse the result, etc), AND ideally the dotnet 8 release won't need these anymore, so this is a temporary workaround. If dotnet 8 release still needs this, then we should revisit this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ivanpovazan do you know if this is tracked/known somewhere on the dotnet/runtime repo side?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are planning to introduce such changes officially with the support for library mode when targeting iOS with NativeAOT: dotnet/runtime#88737
Although, that is currently marked with a .NET9 milestone. @MichalStrehovsky should we try to get it in sooner?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's discussed in dotnet/runtime#91997
There's actually a better workaround that doesn't involve setting these directly, so I'll fix this up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although, that is currently marked with a .NET9 milestone. @MichalStrehovsky should we try to get it in sooner?
I'll leave the prioritization on you, depending work/risk/other priorities/etc. There's demand for it from here and from the original issue filer and it might meet the .NET 8 bar ("Blocking key partner scenarios or blocking adoption"). But if there's good workarounds, that could mean .NET 9 is fine. I don't know how much of a problem this is as I don't do mac stuff.
(This discussion can move to dotnet/runtime#88737, just wanted to ask if we have an issue and we do).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI (and I'll add that comment too upstream, but just so it's here as well), it seems like the workaround is only for the -lc++
linker argument, and not for the -isysroot
arg, so it would be super great ❤️ ❤️ if (at least) the sysroot path inference bit could be fixed for .NET 8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've worked around this issue for now by discovering the Xcode path via xcrun xcode-select -p
.
I've made a custom build of 4.2-dev6 (57a6813) + this PR cherrypicked (2c28442989a646acc26c241c8544180e17876429) to ease testing. It includes the macOS editor, and the iOS export templates with C# support enabled. https://downloads.tuxfamily.org/godotengine/testing/4.2-dev6+pr82729/mono/ Please test and let us know how it works on your C# projects!
Edit 2: The builds have been updated (same link, same names), now it should work™. Edit 3: More recent build below: #82729 (comment) |
modules/mono/editor/GodotTools/GodotTools.ProjectEditor/default.csproj.template
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools.ProjectEditor/default.csproj.template
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools.ProjectEditor/default.csproj.template
Outdated
Show resolved
Hide resolved
</PropertyGroup> | ||
|
||
<ItemGroup Condition="$(RuntimeIdentifier.Contains('iossimulator'))"> | ||
<LinkerArg Include="-isysroot /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ivanpovazan do you know if this is tracked/known somewhere on the dotnet/runtime repo side?
modules/mono/editor/GodotTools/GodotTools.ProjectEditor/default.csproj.template
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on iPad, M1 Mac and M1 iPhone 15 Simulator, seems to be working fine.
A few issues:
- Seems like export with C# is always looking for the export template in the default location (and won't run even when custom template is set). It might be a generic iOS export or C# export issue and probably unrelated to the PR.
- I'm always getting the following error in the export log (it's not happening with non-C# iOS export), but exported project is fully functional:
editor/export/editor_export_platform.h:182 - Xcode Build: Xcode project build failed, see editor log for details.
error: Unexpected duplicate tasks
note: Command: SignatureCollection ~/Library/Developer/Xcode/DerivedData/ios_mono-bszfhymtdguyrpfslpyhpguifwus/Build/Intermediates.noindex/ArchiveIntermediates/ios_mono/BuildProductsPath/Debug-iphoneos/ios_mono.xcframework-ios.signature
note: Command: SignatureCollection ~/Library/Developer/Xcode/DerivedData/ios_mono-bszfhymtdguyrpfslpyhpguifwus/Build/Intermediates.noindex/ArchiveIntermediates/ios_mono/BuildProductsPath/Debug-iphoneos/ios_mono.xcframework-ios.signature
** ARCHIVE FAILED **
Note for testing: It is possible to run in simulator in the Compatibly/OpenGL mode. Also, with the Apple Silicon powered Mac, you can run the device version by selecting |
Right, I should specify that it won't work for the ios simulator in arm64 mode specifically - besides the vulkan issues, we don't currently ship the ios simulator arm64 template binaries right now because of some issues with the cross-compiler. I haven't tested on my intel mac, so maybe it will work on the simulator there 🤞 |
ARM64 simulator works fine, if you build the simulator export template library on macOS (official builds do not have it, but it's OSXCross limitation). |
I've been testing with custom templates set in the export options and it's been working fine (that's how I iterate locally, I build the templates and package them in a zip file with the rest of the expected contents that the zip file is suppoed to have). For reference, the custom template fields in the export options expect a zip file, and they can be set to the
Does this happen when the "Only export the project" checkbox is NOT set (so it's trying to generate the ipa)? |
Good to know! I couldn't get to work on my M1, but then again I think I was testing the simulator with Xcode 13 and not Xcode 14 at the time, so maybe it's Xcode 13's fault. |
Only happens if
It's using a custom template if it's set, but is still checking if the default one is present and showing a warning message in the export dialog. It seems to be an issue with |
I did the same locally, and I didn't get that issue, so it might be something unrelated. I'm on an M1 (Monterey) with Xcode 14.2 and 14.3. It could be an Xcode 15 issue maybe? |
Ok, I pushed some updates to this:
For those testing this, you can still use the export templates that @akien-mga linked above (nothing has changed on those), but you'll need a new build of the editor to get the sdk and build tooling changes. Remember to wipe your nuget package cache ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking really good. I only have a few concerns about how these iOS-specific changes are intertwined with the rest of the export process.
modules/mono/editor/GodotTools/GodotTools.ProjectEditor/ProjectGenerator.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Internals/GodotSharpDirs.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Build/BuildManager.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Build/BuildManager.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
I made another test build from the last commit of this PR (bfed26e), rebased on top of c2b9167: https://downloads.tuxfamily.org/godotengine/testing/4.2-dev-pr82729/mono/ It's a full build for all platforms, including non-"mono" builds, as I used this as a pretext to test an update to our build containers. For what's relevant in this PR, this means macOS and iOS builds made with Xcode 15 via latest osxcross/cctools-port. |
7873b61
to
ac9d351
Compare
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
e682aad
to
c56f6c4
Compare
There was a problem hiding this 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, thanks for the amazing work!
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Build/BuildManager.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
c56f6c4
to
816efa2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style looks good!
This support is experimental and requires .NET 8 Known issues: - Requires macOS due to use of lipo and xcodebuild - arm64 simulator templates are not currently included in the official packaging
816efa2
to
ee9a735
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me! Amazing work.
I haven't tested building the very latest iteration but it built fine a couple of days ago, I'm confident it's still the case. Let's get it tested widely in 4.2 beta 1 :)
Thanks so much! |
Thank you so much for this awesome work! I'm not super technical, so pardon me if I'm asking something really silly. Would this in-theory also work with Hotfixing bugs on a Live iOS game? e.g. I re-compiled all the scripts into a GameLogic.dll and do Assembly.LoadFile("GameLogic.dll") |
This PR adds C# support for iOS targets, using the official .NET Core 8 NativeAOT support. This requires (and has been tested) .NET Core 8 RC 1.
To support iOS+NativeAOT, the csproj gets a lot more complicated than just the two entries it had before, so the generated content is moved to a template, embedded as a resource in the GodotTools.ProjectEditor assembly. It's possible that the release version of dotnet core 8 (around November) will make the csproj template simpler, there's some linker workarounds that are currently required that will likely be fixed by then, so this will need to be revisited when dotnet 8 gets released.
Godot still has a bunch of reflection code which breaks trimming, so the csproj generates an rd.xml whitelisting the godot and project assemblies, so things work out of the box and don't crash at runtime. Once the reflection code gets cleaned up, we can revisit this.
Update/some more notes
The target framework for the user project is set to 8.0 only when the RuntimeIdentifier contains "ios", so in all other cases it should still work fine as is with .net 6 - if I managed to hit all the right buttons, this shouldn't affect any other platforms.
Godot doesn't run on iOS Simulator right now because we're using Vulkan features that it doesn't support, but this PR includes all the needed bits for the simulator (so when #74227 eventually gets fixed, this should Just Work(tm))
Contributed by W4Games