fix(godot): embed ios frameworks on export#148
Conversation
Register GodotIap iOS frameworks during export and keep the post-export fixer available in release artifacts. Update the fixer to restore framework Info.plist files, normalize framework bundle references, and avoid duplicate Xcode framework links. Document the automatic export path and fallback workflow for Godot iOS setup.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds iOS framework embedding support to the GodotIap plugin by enhancing the export plugin to detect iOS exports and register framework embedding, providing a comprehensive post-export Xcode fixup script, packaging the script in releases, and documenting the complete setup workflow. ChangesiOS Framework Embedding Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
hyochan
added
godot-iap
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request automates iOS framework embedding for the Godot IAP plugin by updating the export plugin and significantly improving the post-export fix script. The documentation has also been updated to reflect these changes. Key feedback includes correcting a typo in the Godot 4.3/4.4 API method name, tightening regex patterns in the fix script to avoid over-matching sub-files, and addressing fragility in the project file formatting logic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/docs/src/pages/docs/setup/godot.tsx (1)
209-226:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReading flow: "Then confirm the runpath" follows collapsed
<details>content.After the rewrite, the "Manual Xcode fallback" steps live inside a collapsible
<details>, but the "The frameworks are located at" code block and the "Then confirm the runpath" instructions remain outside. With the disclosure collapsed (default), readers using the automatic export path will see "Then confirm the runpath" without any antecedent — and runpath setup is something Godot's export already wires up, so it is really part of the manual fallback flow.Consider moving the framework paths and runpath steps into the
<details>block, or rewording so they don't appear conditional on the collapsed steps.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/docs/src/pages/docs/setup/godot.tsx` around lines 209 - 226, The "The frameworks are located at:" CodeBlock and the "Then confirm the runpath" ol are currently outside the collapsible Manual Xcode fallback <details>, causing them to appear when the details are collapsed and misleading readers; either move the CodeBlock and the entire runpath instructions (the <ol> starting with "Go to the Build Settings tab") into the same <details> block that contains the "Manual Xcode fallback" content, or reword the heading/text to make it explicit these steps apply only to the manual fallback flow (reference the existing CodeBlock, the <ol> with "Runpath Search Paths" and the Manual Xcode fallback <details> disclosure).
🧹 Nitpick comments (2)
libraries/godot-iap/addons/godot-iap/godot_iap_plugin.gd (1)
32-35: 💤 Low valueAdd a typed array annotation for
IOS_FRAMEWORKS.Minor type-safety nit:
Array[String]makes the contract explicit and lets the parser/editor catch accidental non-string entries.♻️ Proposed change
- const IOS_FRAMEWORKS = [ + const IOS_FRAMEWORKS: Array[String] = [ "res://addons/godot-iap/bin/ios/GodotIap.framework", "res://addons/godot-iap/bin/ios/SwiftGodotRuntime.framework", ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libraries/godot-iap/addons/godot-iap/godot_iap_plugin.gd` around lines 32 - 35, The constant IOS_FRAMEWORKS should be declared with an explicit typed array to ensure all entries are strings; update the declaration of IOS_FRAMEWORKS to include the GDScript type annotation Array[String] (e.g. const IOS_FRAMEWORKS: Array[String] = [...]) so the parser/editor enforces string-only entries for the array.libraries/godot-iap/scripts/fix_ios_embed.sh (1)
24-32: ⚡ Quick winSilently picks first
.xcodeprojwhen multiple exist.
find ... | sort | head -n 1returns a single project without telling the user there were multiple. If a Godot iOS export ever places more than one.xcodeprojat the top level (or a user runs this against the wrong directory), the script will quietly fix the wrong one. A small guard makes the failure mode explicit while keeping the existing single-project happy path.♻️ Proposed change
if [ -z "$XCODEPROJ" ]; then - XCODEPROJ="$(find "$IOS_EXPORT_DIR" -maxdepth 1 -name "*.xcodeproj" -type d | sort | head -n 1)" + mapfile -t _xcodeprojs < <(find "$IOS_EXPORT_DIR" -maxdepth 1 -name "*.xcodeproj" -type d | sort) + if [ "${`#_xcodeprojs`[@]}" -gt 1 ]; then + echo "Error: multiple .xcodeproj found in $IOS_EXPORT_DIR:" >&2 + printf ' %s\n' "${_xcodeprojs[@]}" >&2 + echo "Set XCODEPROJ=/path/to/App.xcodeproj to disambiguate." >&2 + exit 1 + fi + XCODEPROJ="${_xcodeprojs[0]:-}" fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libraries/godot-iap/scripts/fix_ios_embed.sh` around lines 24 - 32, The script silently chooses the first .xcodeproj when multiple exist by assigning XCODEPROJ="$(find "$IOS_EXPORT_DIR" -maxdepth 1 -name "*.xcodeproj" -type d | sort | head -n 1)"; change this logic to detect if more than one .xcodeproj is present and fail fast: use the same find pipeline but capture all matches, count them, and if count > 1 print an error listing the found projects and instruct the user to set XCODEPROJ or correct IOS_EXPORT_DIR, then exit 1; if count == 1 continue and assign XCODEPROJ to that single path so the existing single-project path remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/docs/src/pages/docs/setup/godot.tsx`:
- Around line 209-226: The "The frameworks are located at:" CodeBlock and the
"Then confirm the runpath" ol are currently outside the collapsible Manual Xcode
fallback <details>, causing them to appear when the details are collapsed and
misleading readers; either move the CodeBlock and the entire runpath
instructions (the <ol> starting with "Go to the Build Settings tab") into the
same <details> block that contains the "Manual Xcode fallback" content, or
reword the heading/text to make it explicit these steps apply only to the manual
fallback flow (reference the existing CodeBlock, the <ol> with "Runpath Search
Paths" and the Manual Xcode fallback <details> disclosure).
---
Nitpick comments:
In `@libraries/godot-iap/addons/godot-iap/godot_iap_plugin.gd`:
- Around line 32-35: The constant IOS_FRAMEWORKS should be declared with an
explicit typed array to ensure all entries are strings; update the declaration
of IOS_FRAMEWORKS to include the GDScript type annotation Array[String] (e.g.
const IOS_FRAMEWORKS: Array[String] = [...]) so the parser/editor enforces
string-only entries for the array.
In `@libraries/godot-iap/scripts/fix_ios_embed.sh`:
- Around line 24-32: The script silently chooses the first .xcodeproj when
multiple exist by assigning XCODEPROJ="$(find "$IOS_EXPORT_DIR" -maxdepth 1
-name "*.xcodeproj" -type d | sort | head -n 1)"; change this logic to detect if
more than one .xcodeproj is present and fail fast: use the same find pipeline
but capture all matches, count them, and if count > 1 print an error listing the
found projects and instruct the user to set XCODEPROJ or correct IOS_EXPORT_DIR,
then exit 1; if count == 1 continue and assign XCODEPROJ to that single path so
the existing single-project path remains unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e166ce0-69e7-4d3a-8d9d-50acce231242
📒 Files selected for processing (5)
.github/workflows/release-godot.ymllibraries/godot-iap/addons/godot-iap/godot_iap.gdlibraries/godot-iap/addons/godot-iap/godot_iap_plugin.gdlibraries/godot-iap/scripts/fix_ios_embed.shpackages/docs/src/pages/docs/setup/godot.tsx
Tighten the iOS export fallback script, add an extra Apple framework API fallback, and move manual runpath guidance into the manual docs section.
|
@coderabbitai review |
|
/gemini review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Code Review
This pull request automates iOS framework embedding in Godot exports and enhances the robustness of the fix_ios_embed.sh script. Key changes include updating the Godot export plugin to register frameworks automatically, rewriting the post-export script to handle complex Xcode project structures and duplicate links, and updating documentation to reflect the improved workflow. Review feedback identifies an incorrect method name in the export plugin and suggests a more robust regex for parsing Xcode project files to accommodate varying indentation.
| if has_method("add_apple_embedded_platform_embedded_framework"): | ||
| call("add_apple_embedded_platform_embedded_framework", path) | ||
| return | ||
| if has_method("add_apple_embedded_framework"): | ||
| call("add_apple_embedded_framework", path) | ||
| return |
There was a problem hiding this comment.
The method name add_apple_embedded_platform_embedded_framework appears to be incorrect and does not exist in the Godot Engine source. The standard modern method for Godot 4.2+ is add_apple_embedded_framework. I recommend removing the redundant and incorrect check to simplify the logic while maintaining compatibility with older versions via the existing fallback.
if has_method("add_apple_embedded_framework"):
call("add_apple_embedded_framework", path)
return
There was a problem hiding this comment.
No code change for this one. I checked the local Godot 4.6.2 binary and it exposes add_apple_embedded_platform_embedded_framework, add_apple_embedded_platform_framework, add_ios_embedded_framework, and add_ios_framework, but not add_apple_embedded_framework. Keeping add_apple_embedded_platform_embedded_framework preserves the modern Godot 4.6 path; add_apple_embedded_framework remains as an extra compatibility fallback before add_ios_embedded_framework.
1 participant
Summary
Related: #146
Changes
Godot iOS export
GodotIap.frameworkplusSwiftGodotRuntime.frameworkas embedded frameworks.Post-export fallback
scripts/fix_ios_embed.shin the Godot release zip..xcodeproj, copy missing frameworkInfo.plistfiles, normalize framework bundle references, and avoid duplicate framework link entries.Docs
Test plan
bash -n libraries/godot-iap/scripts/fix_ios_embed.shgit diff --checkmake testinlibraries/godot-iap(92 passed)bun run typecheckinpackages/docsbun run lintinpackages/docs/tmp/openiap-godot-ios-exportsucceeded and produced archive/IPAfix_ios_embed.shapplied to exported Xcode projectxcodebuild -project /tmp/openiap-godot-ios-export/Martie.xcodeproj -scheme Martie -sdk iphoneos -configuration Debug -destination 'generic/platform=iOS' buildSummary by CodeRabbit
New Features
Documentation