-
Notifications
You must be signed in to change notification settings - Fork 61
feat: [SDK-4774] support Unity no-location builds #877
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
Changes from all commits
fb619b7
4291ea8
6e9c68f
cd3a148
f93d84f
ccf13de
688d184
a515431
b522eae
54775fc
bee7273
01aea3f
06b4c06
79d56f0
6423e81
c56f7f3
f46893a
93006c2
479be50
ae5bd1f
a1ba64a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| /* | ||
| * Modified MIT License | ||
| * | ||
| * Copyright 2023 OneSignal | ||
| * | ||
| * Permission is hereby granted, free of charge, to any person obtaining a copy | ||
| * of this software and associated documentation files (the "Software"), to deal | ||
| * in the Software without restriction, including without limitation the rights | ||
| * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
| * copies of the Software, and to permit persons to whom the Software is | ||
| * furnished to do so, subject to the following conditions: | ||
| * | ||
| * 1. The above copyright notice and this permission notice shall be included in | ||
| * all copies or substantial portions of the Software. | ||
| * | ||
| * 2. All copies of substantial portions of the Software may only be used in connection | ||
| * with services provided by OneSignal. | ||
| * | ||
| * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
| * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
| * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
| * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
| * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
| * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
| * THE SOFTWARE. | ||
| */ | ||
|
|
||
| using System; | ||
| using System.IO; | ||
| using UnityEditor; | ||
| using UnityDebug = UnityEngine.Debug; | ||
|
|
||
| namespace OneSignalSDK | ||
| { | ||
| [InitializeOnLoad] | ||
| internal static class OneSignalAndroidDependencies | ||
| { | ||
| static OneSignalAndroidDependencies() | ||
| { | ||
| OneSignalSDKSettings.Changed += WriteDependencies; | ||
| WriteDependencies(); | ||
| } | ||
|
|
||
| internal static void WriteDependencies() | ||
| { | ||
| var contents = OneSignalSDKSettings.EffectiveDisableLocation | ||
| ? DisabledLocationDependencies | ||
| : DefaultDependencies; | ||
|
|
||
| try | ||
| { | ||
| if ( | ||
| File.Exists(_dependenciesPath) | ||
| && File.ReadAllText(_dependenciesPath) == contents | ||
| ) | ||
| return; | ||
|
|
||
| Directory.CreateDirectory(Path.GetDirectoryName(_dependenciesPath)); | ||
| File.WriteAllText(_dependenciesPath, contents); | ||
| AssetDatabase.ImportAsset(_dependenciesPath); | ||
| } | ||
| catch (Exception exception) | ||
| { | ||
| UnityDebug.LogWarning( | ||
| $"Could not update OneSignal Android dependencies at {_dependenciesPath}: {exception.Message}" | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| internal const string Version = "5.9.5"; | ||
|
|
||
| private static readonly string _dependenciesPath = Path.Combine( | ||
| "Assets", | ||
| "OneSignal", | ||
| "Editor", | ||
| "OneSignalAndroidDependencies.xml" | ||
| ); | ||
|
|
||
| private static string DefaultDependencies => | ||
| $@"<dependencies> | ||
| <androidPackages> | ||
| <repositories> | ||
| <repository>https://repo.maven.apache.org/maven2</repository> | ||
| </repositories> | ||
| <androidPackage spec=""com.onesignal:OneSignal:{Version}"" /> | ||
| </androidPackages> | ||
| </dependencies> | ||
| "; | ||
|
|
||
| private static string DisabledLocationDependencies => | ||
| $@"<dependencies> | ||
| <androidPackages> | ||
| <repositories> | ||
| <repository>https://repo.maven.apache.org/maven2</repository> | ||
| </repositories> | ||
| <androidPackage spec=""com.onesignal:core:{Version}"" /> | ||
| <androidPackage spec=""com.onesignal:notifications:{Version}"" /> | ||
| <androidPackage spec=""com.onesignal:in-app-messages:{Version}"" /> | ||
| </androidPackages> | ||
| </dependencies> | ||
| "; | ||
| } | ||
| } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,32 +25,72 @@ | |
| * THE SOFTWARE. | ||
| */ | ||
|
|
||
| using System.Threading.Tasks; | ||
| using System; | ||
| using OneSignalSDK.Android.Utilities; | ||
| using OneSignalSDK.Debug.Utilities; | ||
| using OneSignalSDK.Location; | ||
| using UnityEngine; | ||
|
|
||
| namespace OneSignalSDK.Android.Location | ||
| { | ||
| internal sealed class AndroidLocationManager : ILocationManager | ||
| { | ||
| private const string LocationModuleNotAvailable = | ||
| "OneSignal location module is not available. Add the location dependency to use OneSignal.Location."; | ||
|
|
||
| private readonly AndroidJavaObject _location; | ||
|
|
||
| public AndroidLocationManager(AndroidJavaClass sdkClass) | ||
| { | ||
| _location = sdkClass.CallStatic<AndroidJavaObject>("getLocation"); | ||
| try | ||
| { | ||
| _location = sdkClass.CallStatic<AndroidJavaObject>("getLocation"); | ||
| } | ||
| catch (Exception) | ||
| { | ||
| SDKDebug.Error(LocationModuleNotAvailable); | ||
| } | ||
| } | ||
|
|
||
| public bool IsShared | ||
| { | ||
| get => _location.Call<bool>("isShared"); | ||
| set => _location.Call("setShared", value); | ||
| get | ||
| { | ||
| try | ||
| { | ||
| return _location != null && _location.Call<bool>("isShared"); | ||
| } | ||
| catch (Exception) | ||
| { | ||
| SDKDebug.Error(LocationModuleNotAvailable); | ||
| return false; | ||
| } | ||
| } | ||
| set | ||
| { | ||
| try | ||
| { | ||
| if (_location != null) | ||
| _location.Call("setShared", value); | ||
| } | ||
| catch (Exception) | ||
| { | ||
| SDKDebug.Error(LocationModuleNotAvailable); | ||
| } | ||
| } | ||
| } | ||
|
Check warning on line 81 in com.onesignal.unity.android/Runtime/AndroidLocationManager.cs
|
||
|
Comment on lines
55
to
81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 On Android, when the native location module is missing, Extended reasoning...Bug: In
Why existing code doesn't prevent it: the try/catch around each accessor only catches the JNI exception that would come from a real Impact: the PR description says it 'logs SDK-level errors when location APIs are called without the native location module', and the iOS bridge ( Step-by-step proof: consider the no-location demo (which this PR adds) with Addressing the refutation: the refuter is right that (a) the constructor already logs once, (b) the README documents the no-op contract, and (c) logging every frame can be argued as spam. Those are fair points — which is why this is filed as a nit, not normal severity. But the PR's own description and the iOS implementation set the precedent that each API call without the module produces a log, and Fix: remove the null short-circuit so the NRE path catches and logs (matching get {
if (_location == null) {
SDKDebug.Error(LocationModuleNotAvailable);
return false;
}
try { return _location.Call<bool>("isShared"); }
catch (Exception) { SDKDebug.Error(LocationModuleNotAvailable); return false; }
} |
||
|
|
||
| public void RequestPermission() | ||
| { | ||
| var continuation = new BoolContinuation(); | ||
| _location.Call<AndroidJavaObject>("requestPermission", continuation.Proxy); | ||
| try | ||
| { | ||
| var continuation = new BoolContinuation(); | ||
| _location.Call<AndroidJavaObject>("requestPermission", continuation.Proxy); | ||
| } | ||
| catch (Exception) | ||
| { | ||
| SDKDebug.Error(LocationModuleNotAvailable); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| /* | ||
| * Modified MIT License | ||
| * | ||
| * Copyright 2023 OneSignal | ||
| * | ||
| * Permission is hereby granted, free of charge, to any person obtaining a copy | ||
| * of this software and associated documentation files (the "Software"), to deal | ||
| * in the Software without restriction, including without limitation the rights | ||
| * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
| * copies of the Software, and to permit persons to whom the Software is | ||
| * furnished to do so, subject to the following conditions: | ||
| * | ||
| * 1. The above copyright notice and this permission notice shall be included in | ||
| * all copies or substantial portions of the Software. | ||
| * | ||
| * 2. All copies of substantial portions of the Software may only be used in connection | ||
| * with services provided by OneSignal. | ||
| * | ||
| * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
| * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
| * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
| * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
| * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
| * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
| * THE SOFTWARE. | ||
| */ | ||
|
|
||
| using System; | ||
| using System.IO; | ||
| using UnityEditor; | ||
| using UnityEngine; | ||
|
|
||
| namespace OneSignalSDK | ||
| { | ||
| public static class OneSignalSDKSettings | ||
| { | ||
| public const string DisableLocationEnvVar = "ONESIGNAL_DISABLE_LOCATION"; | ||
|
|
||
| public static event Action Changed; | ||
|
|
||
| public static bool DisableLocation | ||
| { | ||
| get => _settings.disableLocation; | ||
| set | ||
| { | ||
| if (_settings.disableLocation == value) | ||
| return; | ||
|
|
||
| _settings.disableLocation = value; | ||
| Save(); | ||
| Changed?.Invoke(); | ||
| AssetDatabase.Refresh(); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Resolved value used by the build pipeline. The <see cref="DisableLocationEnvVar"/> | ||
| /// environment variable, when present, overrides the persisted Editor setting so CLI | ||
| /// and CI builds can opt out without mutating project settings. | ||
| /// </summary> | ||
| public static bool EffectiveDisableLocation | ||
| { | ||
| get | ||
| { | ||
| var environmentValue = Environment.GetEnvironmentVariable(DisableLocationEnvVar); | ||
| if (!string.IsNullOrEmpty(environmentValue)) | ||
| { | ||
| var normalized = environmentValue.Trim(); | ||
| return normalized.Equals("true", StringComparison.OrdinalIgnoreCase) | ||
| || normalized == "1"; | ||
| } | ||
|
|
||
| return _settings.disableLocation; | ||
| } | ||
| } | ||
|
|
||
| public static void Save() | ||
| { | ||
| Directory.CreateDirectory(Path.GetDirectoryName(_settingsPath)); | ||
| File.WriteAllText(_settingsPath, JsonUtility.ToJson(_settings, true)); | ||
| } | ||
|
|
||
| [MenuItem("OneSignal/Disable Location Module")] | ||
| private static void ToggleDisableLocation() | ||
| { | ||
| DisableLocation = !DisableLocation; | ||
| } | ||
|
|
||
| [MenuItem("OneSignal/Disable Location Module", true)] | ||
| private static bool ToggleDisableLocationValidate() | ||
| { | ||
| Menu.SetChecked("OneSignal/Disable Location Module", DisableLocation); | ||
| return true; | ||
| } | ||
|
|
||
| private static readonly string _settingsPath = Path.Combine( | ||
| "ProjectSettings", | ||
| "OneSignalSettings.json" | ||
| ); | ||
|
|
||
| private static Settings _settings = Load(); | ||
|
|
||
| private static Settings Load() | ||
| { | ||
| if (!File.Exists(_settingsPath)) | ||
| return new Settings(); | ||
|
|
||
| try | ||
| { | ||
| return JsonUtility.FromJson<Settings>(File.ReadAllText(_settingsPath)) | ||
| ?? new Settings(); | ||
| } | ||
| catch | ||
| { | ||
| return new Settings(); | ||
| } | ||
| } | ||
|
|
||
| [Serializable] | ||
| private sealed class Settings | ||
| { | ||
| public bool disableLocation; | ||
| } | ||
| } | ||
| } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
🔴 The release workflow in
.github/workflows/create-release-pr.ymlonly updates files underexamples/demo/and never touches the newexamples/demo-no-location/project, so the four committed dependency files there (Assets/OneSignal/Editor/OneSignal{Android,iOS}Dependencies.xml,Assets/Plugins/Android/mainTemplate.gradle,ProjectSettings/AndroidResolverDependencies.xml) will retain stale versions after each release. Additionally, the existing sed patterns only match the umbrellacom.onesignal:OneSignal:VERSIONartifact and would not match this demo's granular artifacts (core/notifications/in-app-messages,OneSignalXCFramework/OneSignal\[InAppMessages\]) even if the paths were added. Fix needs both path additions and new sed patterns that target the granular coordinate forms.Extended reasoning...
What the bug is. This PR adds a new demo project at
examples/demo-no-location/and checks in four files that hardcode the OneSignal native SDK versions (currently 5.9.5 for Android, 5.5.3 for iOS). The release workflow at.github/workflows/create-release-pr.ymlis responsible for bumping these hardcoded versions in the umbrellaexamples/demo/project on every release, but its sed steps only enumerateexamples/demo/paths —examples/demo-no-location/is never touched.The specific code path. Lines 113-115 (Android) update
examples/demo/Assets/OneSignal/Editor/OneSignalAndroidDependencies.xml,examples/demo/Assets/Plugins/Android/mainTemplate.gradle, andexamples/demo/ProjectSettings/AndroidResolverDependencies.xml. Line 137 (iOS) updatesexamples/demo/Assets/OneSignal/Editor/OneSignaliOSDependencies.xml. None of these touchexamples/demo-no-location/. Beyond the missing paths, the regexes themselves are umbrella-artifact-only:com\.onesignal:OneSignal:[0-9][0-9.]*matchescom.onesignal:OneSignal:5.9.5but does not match the granular forms used by the no-location demo (com.onesignal:core:5.9.5,com.onesignal:notifications:5.9.5,com.onesignal:in-app-messages:5.9.5).Why existing code doesn't prevent it. The new
OneSignalAndroidDependencies.cs/OneSignaliOSDependencies.csfiles do regenerate the XML manifests via[InitializeOnLoad]when Unity opens the project, so the two XMLs underAssets/OneSignal/Editor/will eventually self-heal — but only once a developer opensdemo-no-locationin the Unity Editor. The release workflow runs in CI without Unity, so the committed release state still ships stale XML. More importantly,Assets/Plugins/Android/mainTemplate.gradleandProjectSettings/AndroidResolverDependencies.xmlare EDM4U snapshots that only refresh on an explicit Force Resolve, which is not triggered by anything in the release pipeline.Impact. A release bump produces a commit where
examples/demo/is at the new version andexamples/demo-no-location/is still at the previous version, across both Android (gradle + resolver xml) and iOS (pod manifest). Anyone checking out the release tag sees inconsistent demo state. SDK consumers installing via UPM are unaffected — this is a release-engineering / demo-hygiene correctness issue, not a runtime bug.Step-by-step proof. Suppose the next release is invoked with
android_version=5.10.0,ios_version=5.6.0:com.onesignal.unity.android/Editor/OneSignalAndroidDependencies.csVersion const →5.10.0. ✓sed -i -E "s/<androidPackage spec=\"com\.onesignal:OneSignal:[0-9][0-9.]*\"/.../" examples/demo/Assets/OneSignal/Editor/OneSignalAndroidDependencies.xml→ matches and rewrites. ✓examples/demo-no-location/Assets/OneSignal/Editor/OneSignalAndroidDependencies.xml. The file still readscom.onesignal:core:5.9.5,com.onesignal:notifications:5.9.5,com.onesignal:in-app-messages:5.9.5. ✗com\.onesignal:OneSignal:[0-9][0-9.]*does not matchcom.onesignal:core:5.9.5(artifact idcore, notOneSignal). Still stale. ✗examples/demo-no-location/Assets/Plugins/Android/mainTemplate.gradle(implementation 'com.onesignal:core:5.9.5'etc.) andexamples/demo-no-location/ProjectSettings/AndroidResolverDependencies.xml(<package>com.onesignal:core:5.9.5</package>etc.). ✗examples/demo/updated to 5.10.0/5.6.0 andexamples/demo-no-location/stuck at 5.9.5/5.5.3.How to fix. Extend the Android step to also run granular-artifact substitutions across the demo-no-location files, e.g.:
And the iOS step needs to update
examples/demo-no-location/Assets/OneSignal/Editor/OneSignaliOSDependencies.xml— the existingversion="[0-9][0-9.]*"regex would actually match the granularOneSignalXCFramework/OneSignalandOneSignalXCFramework/OneSignalInAppMessagesiosPod entries, so just adding the path to line 137 is sufficient on the iOS side.