-
Notifications
You must be signed in to change notification settings - Fork 12
[ID-3859] feat: Passport prefab #539
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
base: main
Are you sure you want to change the base?
Conversation
src/Packages/Passport/Runtime/Scripts/Private/Helpers/WindowsDeepLink.cs
Outdated
Show resolved
Hide resolved
src/Packages/Passport/Runtime/Scripts/Private/Helpers/WindowsDeepLink.cs
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.
Can we add this prefab to the sample app too? So customers can see a working example of how it's configured?
44577e2
to
5ff7369
Compare
5973acf
to
a57cd86
Compare
Generally we add what goes into the SDK into the sample app too as we often get customers to reference the sample app. I thought it would be a simple addition since it's a prefab. |
a57cd86
to
98a00b3
Compare
var cmdPath = GetGameExecutablePath(".cmd"); | ||
if (File.Exists(cmdPath)) |
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.
If this is removed, when will it be cleaned up?
"com.cysharp.unitask": "2.3.1", | ||
"com.unity.modules.unitywebrequest": "1.0.0", | ||
"com.unity.modules.jsonserialize": "1.0.0", | ||
"com.unity.modules.androidjni": "1.0.0" |
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.
Why do we need the android jni library?
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.
Unity added those when I started the sample app, so I guess it does need 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.
Do we need all 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.
I figure it's better to have more info than not enough...
- Add browser process isolation workaround for Unity Passport authentication - Configure protocol associations and browser permissions programmatically - Handle both full login flows and cached session scenarios - Fix MailSlurp API URL configuration for OTP retrieval - Implement controlled logout process to prevent app crashes - Increase AltDriver timeouts for CI stability
src/Packages/Passport/Runtime/Scripts/Public/PassportUIBuilder.cs
Outdated
Show resolved
Hide resolved
- Change concurrency group from per-platform to shared 'ui-tests-email-inbox' - Ensures macOS and Windows tests run sequentially to avoid MailSlurp race conditions - Prevents OTP emails being consumed by wrong test when tests run in parallel
d30a1e4
to
5e5b616
Compare
- Move PassportManager.cs and PassportUIBuilder.cs to Samples folder - Remove Unity.Modules.UI and Unity.TextMeshPro dependencies from core SDK - Create separate assembly definition for samples with UI dependencies - Core SDK is now UI-agnostic while preserving drag-and-drop prefab experience - Developers can use provided UI samples or build their own UI
- Add cancel-in-progress: false to ui-tests-email-inbox concurrency group - Allows queued tests to complete rather than being canceled by new runs - Improves CI resource utilization and debugging experience - Maintains existing protection against MailSlurp OTP race conditions
c38fb47
to
fe27997
Compare
Summary
ID-3859
Customer Impact
Added
Things worth calling out
Other things to consider: