-
Notifications
You must be signed in to change notification settings - Fork 34
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 and macos tutorial #50
Conversation
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.
Thanks so much, this is an extremely valuable addition! 💪
It added several comments, mostly regarding formatting. Take your time when addressing them 🙂 the inline suggestions can be directly applied on the web interface.
The lint is currently also failing, you can click on the job to see details.
There are a few recurring issues:
- Tabs should be spaces
- Line length limit
- Code blocks should be surrounded by empty lines
- Code blocks must have a language,
```bash
or```text
depending on what it is
Responded to 1 comment and fixed all others.(and linting, 1 more issue related to the comment remaining) |
src/toolchain/export-mac-and-ios.md
Outdated
### APPLE_DEV_TEAM_ID - Apple Team ID | ||
|
||
|
||
Go to [developer.apple.com](https://developer.apple.com). Go to account. | ||
|
||
Go to membership details. Copy Team ID. | ||
|
||
```sh | ||
APPLE_DEV_TEAM_ID = 1ABCD23EFG | ||
``` | ||
|
||
|
||
### APPLE_DEV_PASSWORD - Apple App-Specific Password |
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.
Backticks for all variables, in both titles 😉
src/toolchain/export-mac-and-ios.md
Outdated
Copy the contents of the generated file, e.g.: | ||
APPLE_CERT_BASE64 = ...(A long text file) |
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.
Like I wrote before, 2nd line should be in code tags.
src/toolchain/export-mac-and-ios.md
Outdated
Afterwards you can use the following script | ||
for signing [ci-sign-macos.ps1](https://github.com/appsinacup/godot-rapier-physics/blob/main/scripts/ci-sign-macos.ps1). |
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.
Running random scripts from a third-party repository is both a bit insecure and brittle. It will break as soon as someone changes file name/path, and people can introduce code at any time.
The fact that the script has zero comments and is itself a fork of https://github.com/godot-jolt/godot-jolt/blob/master/scripts/ci_sign_macos.ps1 doesn't make it more trustworthy, either.
Is this the easiest/best way? Are there official resources that it can be complemented with?
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.
There is a tool, fastlane, which can be used. However I haven't used it, and for my use cases I needed a script as I wanted it automated.
I would propose adding it inline then, and we can give credits to the original script.
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.
What does the godot-rapier-physics version change compared to the godot-jolt one?
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 guess I was not owner of that repo, so thats why I didnt put that, as I tested this version of script and I would know what happens to it. I also put some of the logic in the godot-cpp-template repo and credited godot-jolt repo.
Could do same here, copy paste contents of script? It's basically some steps that are tedious to do by hand. Most people however if they do this, they would not do this step, if they build from godot(eg. godot automated this signing step). So this is only for people who export the lib as is, and those people might anyway use github actions for that. For that use case I could just give a reusable external action, eg. https://github.com/godotengine/godot-cpp-template/blob/main/.github/actions/sign/action.yml
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 could describe the steps individually and not use the script in this tutorial, and credit the creator somewhere.
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 think it's OK to link to an external script if it's somewhat reputed (like godot-jolt). We should just add a disclaimer that the user is responsible for the security and up-to-dateness.
As for describing the script's step, I'm not sure if that doesn't widen the scope of the tutorial too much. Given that the script is already quite long, explaining it would be its own huge page in the book, no?
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.
Ok, looking at the jolt one, looks it has less flag's on some commands, but probably works just as well. My only fear if linking to that and if it doesn't work, would be hard to change that.
That being said, updating with that script, if anything it's a good starting point for people willing do signing by hand (that step is optional if you build a game for eg, it's only needed in case you want to export the lib as is)
Updated again after feedback (the backticks and the link to godot-jol script and the disclaimer). Merged all commits in one for when it's merged. |
dc99113
to
8fa8fae
Compare
Update export-mac-and-ios.md Update export-mac-and-ios.md Update export-mac-and-ios.md update Update export-mac-and-ios.md Update export-mac-and-ios.md Update export-mac-and-ios.md fixes Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update src/toolchain/export-mac-and-ios.md Update export-mac-and-ios.md upd Update export-mac-and-ios.md lint Update export-mac-and-ios.md Co-Authored-By: Jan Haller <[email protected]>
8fa8fae
to
06e71de
Compare
Thank you so much! For the book repo, I can squash the commits on merge, so it's fine if you leave individual commits. On the main repo though, I need the PR authors to do it because of the merge queue 🙂 |
Ok, it's a big file, mostly based on my previous tutorial from here: https://github.com/godotengine/godot-cpp-template
Also, it references a script made by mihe from godot-jolt that is used to sign macOS libraries, to whom we need to give credit.
To Credit:
Also, we need a way to reference the script file, right now I am referencing it from a different repo, we need to put it in this repo maybe.