-
Notifications
You must be signed in to change notification settings - Fork 19
Bump Subtester Editor version to 6000.2.2f1 #654
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
Conversation
Generated by 🚫 Danger |
- Update CircleCI config to use Unity 6000.2.2f1 Docker images - Add GameCI Unity orb for license activation - Update create-unity-package.sh script for Unity 6.2 compatibility - Add Unity 6.2 optimizations and robust download fallback
- Use game-ci/[email protected] (latest stable version) - Re-add temporary activation file workflow for Unity 6.2
- Remove orb version from editor_version parameter - Should be '6000.2.2f1' not '6000.2.2f1-3.1.0' - This fixes Docker image pull error
- Replace GameCI orb job (hardcoded to -base-1) with custom job - Use correct Docker image: ubuntu-6000.2.2f1-base-3 - Add proper Unity 6.2 environment variables - Include detailed logging and error handling
- Remove temporary activation workflow and job (no longer needed) - Add CircleCI context references to all Unity-related jobs - Configure proper license activation flow for CI/CD pipeline - Ready for Unity 6.2 production builds
- Remove custom activate-license command in favor of GameCI orb - Replace all activate-license calls with unity/prepare-env - Cleaner, more maintainable license handling using official orb - Automatic license activation with UNITY_ENCODED_LICENSE env var
- Add exit code checking for Unity commands - Fix PlayServicesResolver folder detection (was causing find errors) - Add debugging output for symlink, manifest, and folder operations - Verify package file actually exists before declaring success - Make folder detection robust to missing directories This should fix the 'store_artifacts' CI failure by properly detecting when Unity fails to create the package file.
- Show contents of source RevenueCat folder before symlink creation - Display full symlink contents including Scripts folder verification - Check for assembly definition files in the project - Add small delay for Unity to recognize symlinked assemblies - Remove script backup logic - keep all Subtester scripts intact This will help identify why RevenueCat assemblies aren't being found in CI
- Added logic to copy assembly definition files after symlink creation - Ensures RevenueCat assemblies are properly recognized by Unity in CI - Copies both .asmdef and .asmdef.meta files for Scripts and Editor folders - Includes verification step to confirm files are accessible - Fixes CS0246 'RevenueCat'/'Purchases' namespace not found errors
…sing 🔧 Major CI Fixes: 1. Linux Unity Symlink Issue: - Unity on Linux doesn't traverse symlink directories properly - Fixed by copying RevenueCat folder instead of symlinking in CI - Local development still uses symlinks for convenience - Eliminates 'Amount of processed bytes 0' error 2. Unity Command Line Parsing Bug: - Fixed missing filename for -logFile flag in CI - Was causing Unity to treat -disable-assembly-updater as log filename - Now uses -logFile /dev/stdout for proper CI logging These fixes should resolve the CS0246 'RevenueCat'/'Purchases' not found errors and ensure the -disable-assembly-updater flag is properly processed.
org.gradle.daemon=false | ||
org.gradle.workers.max=1 | ||
android.enableDexingArtifactTransform=false | ||
org.gradle.parallel=true |
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 was changed in #655 but I reverted them and tests still pass
@@ -96,23 +108,6 @@ commands: | |||
# rename all the .cs.break files in APITests to .cs | |||
for file in IntegrationTests/Assets/APITests/*.cs.break ; do mv "$file" "${file%.*}" ; done | |||
|
|||
activate-license: |
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 done using the orb now
|
||
- store_artifacts: | ||
path: Subtester/buildAndroid.apk | ||
path: Subtester/Builds/Android/Android.apk | ||
|
||
build-subtester-ios: |
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 works so I didn't change it to use the orb and it uses our build command. It's basically the same anyways
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.
Would it be worth to still use the orb for consistency? (Can be in a separate PR)
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.
Yeah, definitely worth it. I wanted to keep this PR to the minimum needed and will come in a separate PR
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.
Still need more review and to test it, but looking great so far!! Excited to get this 🙌 🙌
|
||
- store_artifacts: | ||
path: Subtester/buildAndroid.apk | ||
path: Subtester/Builds/Android/Android.apk | ||
|
||
build-subtester-ios: |
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.
Would it be worth to still use the orb for consistency? (Can be in a separate PR)
MultiplayerManager: | ||
m_ObjectHideFlags: 0 | ||
m_EnableMultiplayerRoles: 0 | ||
m_StrippingTypes: {} |
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.
Is this file needed?
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 was automatically created when migrating the project so I figured we would commit it. Maybe it can be ignored?
@@ -10,7 +10,7 @@ allprojects { | |||
// See which Gradle version is preinstalled with Unity here https://docs.unity3d.com/Manual/android-gradle-overview.html | |||
// See official Gradle and Android Gradle Plugin compatibility table here https://developer.android.com/studio/releases/gradle-plugin#updating-gradle | |||
// To specify a custom Gradle version in Unity, go do "Preferences > External Tools", uncheck "Gradle Installed with Unity (recommended)" and specify a path to a custom Gradle version | |||
classpath 'com.android.tools.build:gradle:4.0.1' | |||
classpath 'com.android.tools.build:gradle:8.7.2' |
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.
🎉
And just to check... would it be worth updating to the latest version? I think 8.12.2
at the moment?
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 can try. This was the recommended support version according to Unity docs. Gradle comes embedded in Unity so that's why they recommend a particular AGP version
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.
Alright, as I suspected, bumping this to 8.12.2 won't work because it's incompatible with the version of Gradle that Unity uses. I am checking 8.9.3 which looks like it's compatible with Gradle 8.11
A problem occurred evaluating project ':launcher'.
> Failed to apply plugin 'com.android.internal.version-check'.
> Minimum supported Gradle version is 8.13. Current version is 8.11.
Try updating the 'distributionUrl' property in /root/project/Subtester/Library/Bee/Android/Prj/Mono2x/Gradle/gradle/wrapper/gradle-wrapper.properties to 'gradle-8.13-bin.zip'.
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 ok to leave the current version if it works. Was just checking, but probably not worth spending too long there.
MultiplayerManager: | ||
m_ObjectHideFlags: 0 | ||
m_EnableMultiplayerRoles: 0 | ||
m_StrippingTypes: {} |
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.
Same here about whether this can be removed.
echo "" | ||
|
||
if [ ! -z "$CI" ]; then | ||
# In CI (Linux), Unity doesn't traverse symlinks properly, so copy the folder |
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.
Hmm that's weird... not sure why symlinks wouldn't work on Unix 🤔
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 have no idea to be honest but this was causing me trouble. It wouldn't work until I changed the approach to copy files instead of using symlinks
I think it's more of a problem with the new Unity editor in CI (Unix) not with Unix itself, because this was working properly before
|
||
rm $SYMBOLIC_LINK_PATH | ||
# Cleanup RevenueCat folder/symlink | ||
rm -rf "$PROJECT/Assets/RevenueCat" |
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.
Not for this PR!! But might be a good idea to move this script to fastlane or something that is easier to use and test... But as I said, this should be done separately.
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.
Please, it's horrible haha
@@ -13,6 +13,7 @@ | |||
} | |||
// Android Resolver Repos End | |||
apply plugin: 'com.android.library' | |||
apply from: '../shared/keepUnitySymbols.gradle' |
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.
Is this file generated on export? 🤔
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 generated when enabling the mainTemplate.gradle
setting in Unity settings. It was also updated when migrating to the new Editor
@@ -0,0 +1 @@ | |||
{} |
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.
Are these user settings needed? I wonder if we can ignore them
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 probably can, specially in the IntegrationTests
exit 1 | ||
fi | ||
else | ||
# Local development (macOS) - use symlink for convenience |
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.
Thinking out loud and probably Maybe out of scope, but we (should?) support any OS for local development, right?
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.
good point, this script should support windows as well. I am going to rewrite it in ruby or something easier to understand in the close future and will make sure that's supported
scripts/create-unity-package.sh
Outdated
echo "📄 Modified manifest.json:" | ||
cat $MANIFEST_JSON_PATH |
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 about using a --verbose
parameter to print in debug mode?
Otherwise it will be too verbose even when you don't care about the logs
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.
Sooo much better 🫶. Still needs an armv7 device unfortunately... But at least no more modifications 🙌
Updates the Unity Editor version we use in Subtester and IntegrationTests
I added an orb that is pretty much doing the same we were doing, but with some nice to have stuff, like caches. The
CIEditorScript
we currently used was actually borrowed from that orb already (original file), so that hasn't really changed.