Non-unique Unity-generated node-dss peer identifier on same machine

microsoft / MixedReality-WebRTC

MixedReality-WebRTC is a collection of components to help mixed reality app developers integrate audio and video real-time communication into their application and improve their collaborative experience

https://microsoft.github.io/MixedReality-WebRTC/

MIT License

909 stars 283 forks source link

Non-unique Unity-generated node-dss peer identifier on same machine #38

Closed djee-ms closed 4 years ago

djee-ms commented 5 years ago

When running an instance of the Unity editor and an instance of the PC build of the Unity sample on the same machine, the Unity-generated "unique" peer ID for node-dss is the same, which prevents connecting. The unique ID should be per instance, not per machine.

djee-ms commented 5 years ago

For reference:

The Unity sample uses SystemInfo.deviceUniqueIdentifier which is only unique per device, not per app instance. https://github.com/microsoft/MixedReality-WebRTC/blob/777396acc620d3cac7784882ca10e7d4e4cb6ac8/libs/Microsoft.MixedReality.WebRTC.Unity/Assets/Microsoft.MixedReality.WebRTC.Unity/Scripts/Signaling/NodeDssSignaler.cs#L105
TestAppUWP works around by detecting if a second instance of the app is already running: https://github.com/microsoft/MixedReality-WebRTC/blob/777396acc620d3cac7784882ca10e7d4e4cb6ac8/examples/TestAppUwp/MainPage.xaml.cs#L92-L95

djee-ms commented 4 years ago

Removed the generated unique identifier to let developers chose their own preferred approach. The only requirement for node-dss is the unity of the two peer IDs.

In TestAppUWP, the app now allows configuring the local peer ID, and takes by default the local machine name as local peer ID, and no default for the remote peer ID which needs to be filled. But fields are saved across app launches for convenience.
In Unity, the NodeDssSignaler component now allows configuring the local peer ID and serialize that field for convenience.

This should make debugging as easy and convenient as before, while being more explicit. Closing this issue which actually points at the ID being controlled by MixedReality-WebRTC, which is not the case.