Build and deploy window fixes #3965
Conversation
While looking into other build related issues (i.e. IL2CPP vs .NET) I noticed that there were some issues with the build window (with respect to SDK versioning and chosing the right SDK). There were a couple of issues: 1) There was a concept of a "min" SDK, but the code was actually doing an equality comparison to look for it. That is, if you had GREATER than the min SDK it would fall over. 2) It looks like even if you set a different target SDK than you current (i.e. if you had a higher target SDK than the current min one specified by the MRTK), it would lower your target. This does change things a bit in that this will now prefer the latest SDK (over always the min SDK). It's a marginal improvement over the old in that now future progress (where new SDK version drops occur) don't cause you to fall over because you didn't detect the "minimum" (because you actually have higher than the minimum).
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
| using UnityEngine; | ||
|
|
||
| namespace Microsoft.MixedReality.Toolkit.Build.Editor | ||
| { | ||
| public static class UwpBuildDeployPreferences | ||
| { | ||
| public const string MIN_SDK_VERSION = "10.0.17134.0"; | ||
| public static Version MIN_SDK_VERSION = new Version("10.0.17134.0"); |
There was a problem hiding this comment.
Shouldn't this be whatever version of the insider preview you guys are forcing people to use for mrtk?
There was a problem hiding this comment.
Yes, though this intentionally wasn't changed in this PR to keep the change scoped to changing the version-enumeration code.
|
I can't exactly remember why anymore, but during the whole transition from HTK to support the WMR headsets there was a good reason to enforce the minimum sdk target to a specific version. I'm trying to think if this requirement is really still justified/needed. There's a utility class that checks if a specific uwp api is availible and should be enough to handle that case (so long as people are actually checking before attempting to call them). I think this API checker expects the minimum sdk version to be the highest it can check against. Historical reference: #1289 |
The API version checker is a bit divorced from the SDK version - it's more designed to check to see if API support when running downlevel (i.e. running on a lower level version, or running on a platform doesn't support those particular APIs - i.e. running on a desktop PC without hand joint information vs running on the same version but with actual hand joint data) |
|
I know what it's supposed to do, I'm the original author lol. The point was that there used to be a particular reason to set the minimum sdk level in the build window to be the same as the sdk version checker, but I couldn't remember if it was still valid. Just a hold over from the crazy wmr days? |
While looking into other build related issues (i.e. IL2CPP vs .NET) I noticed that there were some issues with the build window (with respect to SDK versioning and chosing the right SDK).
There were a couple of issues:
This does change things a bit in that this will now prefer the latest SDK (over always the min SDK). It's a marginal improvement over the old in that now future progress (where new SDK version drops occur) don't cause you to fall over because you didn't detect the "minimum" (because you actually have higher than the minimum).
Also switches to using the Version type instead of doing string comparisons (as this allows us to properly sort and reason over versions instead of strings)