-
Notifications
You must be signed in to change notification settings - Fork 18
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
FFI v0.12.2 & RPC #62
Conversation
@@ -122,13 +126,13 @@ private static void InitializeSdk() | |||
const bool captureLogs = false; | |||
#endif | |||
|
|||
NativeMethods.LiveKitInitialize(FFICallback, captureLogs); | |||
NativeMethods.LiveKitInitialize(FFICallback, captureLogs, "unity", ""); // TODO: Get SDK 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.
I verified this works for now but I'd love to get SDK version in. it's just not clear where to get it from at runtime? Might need to inject it into a script like we do for the node SDK. any ideas @cloudwebrtc @theomonnom ?
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 found this but don't know if it works as expected
https://discussions.unity.com/t/how-get-packageinfo-by-name-and-version/762057/8
using UnityEngine;
using System.Linq;
using UnityEditor.PackageManager;
// ... within some class ...
private PackageInfo GetPackageInfo(string packageName)
{
return UnityEditor.AssetDatabase.FindAssets("package")
.Select(UnityEditor.AssetDatabase.GUIDToAssetPath)
.Where(x => UnityEditor.AssetDatabase.LoadAssetAtPath<TextAsset>(x) != null)
.Select(PackageInfo.FindForAssetPath)
.Where(x => x != null)
.First(x => x.name == packageName);
}
string packageName = "com.unity.cinemachine";
Debug.Log(GetPackageInfo(packageName).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.
From what I saw online, UnityEditor
isn't available at runtime on-device, only when running from the Unity Editor? Do you think that's accurate?
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.
gonna just punt on 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.
Yeah, you are right, maybe we can add a version.cs, but we must check that the version number in version.cs is consistent with package.json before releasing a new 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.
in the node sdk we dynamically generate a version.js file as part of the build process, but i'm not sure there's a clean way to pull that off here.
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 the Unity build process can generate version.cs, I think it is a good idea.
@@ -122,13 +126,13 @@ private static void InitializeSdk() | |||
const bool captureLogs = false; | |||
#endif | |||
|
|||
NativeMethods.LiveKitInitialize(FFICallback, captureLogs); | |||
NativeMethods.LiveKitInitialize(FFICallback, captureLogs, "unity", ""); // TODO: Get SDK 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.
From what I saw online, UnityEditor
isn't available at runtime on-device, only when running from the Unity Editor? Do you think that's accurate?
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.
lgtm!
This PR upgrades the FFI version to the latest and includes the RPC feature.
I did not adopt the majority of FFI changes but my limited testing did not suggest they are breaking changes so I believe this is safe but @theomonnom and @cloudwebrtc would know for sure.
The only other blocking issue is that the latest FFI does not build for all runtimes, which will prevent us from releasing a new version of this SDK.