Feature - Platform System #416
Conversation
Added required classes, profiles, and inspectors
StephenHodgson
added
Enhancement
|
Looks like the Unit Tests didn't like this change. Gotta look into that later. |
|
Seems fair, but not clear on how a platform declares itself yet |
Getting there, ideally I think we want each upm package to register itself automagically when it's included/imported |
|
That I agree with, however we do need at least one example to compare, I'd prefer it to be Android based but Standalone should be sufficient to test |
updated the naming of the properties in platform configuration
XRTK-Core/Packages/com.xrtk.core/Interfaces/Platform/IMixedRealityPlatform.cs
Outdated
Show resolved
Hide resolved
# Conflicts: # Submodules/SDK
StephenHodgson
changed the title
# Conflicts: # Submodules/SDK
# Conflicts: # Submodules/Oculus # Submodules/SDK # Submodules/WindowsMixedReality # XRTK-Core/Packages/com.xrtk.core/Services/MixedRealityToolkit.cs
# Conflicts: # Submodules/SDK
StephenHodgson
added
Ready for review
XRTK-Core/Packages/com.xrtk.core/Definitions/MixedRealityServiceConfiguration.cs
Outdated
Show resolved
Hide resolved
XRTK-Core/Packages/com.xrtk.core/Services/MixedRealityToolkit.cs
Outdated
Show resolved
Hide resolved
| private SupportedPlatforms runtimePlatform; | ||
| private RuntimePlatformEntry platformEntries = new RuntimePlatformEntry(); | ||
|
|
||
| [NonSerialized] |
There was a problem hiding this comment.
Private fields are non serialized by default, aren't they?
There was a problem hiding this comment.
Correct, but the object value doesn't get cleared when the editor recompiles and leaves the last object value from the prev domain reload event. Adding the NonSerialized attribute here makes sure that value is cleared on each domain reload.
| /// <summary> | ||
| /// Used by the XRTK to signal that the feature is available on the Android platform. | ||
| /// </summary> | ||
| public class AndroidPlatform : BasePlatform |
There was a problem hiding this comment.
I see AndroidPlatform is defined in Core but LuminPlatform in the Lumin module. Wouldn't it make sense to define all the possible XRTK platforms in Core?
There was a problem hiding this comment.
That's actually the benefit of this change so we don't have to define everything in the core
| /// <summary> | ||
| /// Used by the XRTK to signal that the feature is available on the iOS platform. | ||
| /// </summary> | ||
| public class IOSPlatform : BasePlatform |
There was a problem hiding this comment.
See comment above. It's kind of random that most platforms are defined in Core and some not.
There was a problem hiding this comment.
We might move these out as we get platform specific sdks. I thought it was fine for now since we don't have an iOS submodule
* First pass at Platform System Added required classes, profiles, and inspectors * revert the active profile null check in the static accessors * Fixed unit tests * fixed inspector for platform system profile * Made the platform profile abstract and to be used as a base profile updated the naming of the properties in platform configuration * updated namespaces a bit * Added editor, osx, and windows standalone platform definitions * Removed the SupportedPlatforms enum from the platform configuration * Fixed compiler error * updated sdk checkout * updated sdk * some changes to auto registration * updated tests to support existing systems/services setup by the toolkit * Renamed IMixedRealityPlatform.IsActive to IMixedRealityPlatform.IsAvailable * Added the android platform data provider * Added IOS platform data provider * Added ability to get active platforms from platform system * Added a base platform class * better sorting of registered platform configurations * rolled back obsolete flag for now until I've figured out a better replacement path * Finalized platform feature * bumped version numbers * Apply suggestions from code review
Added required classes, profiles, and inspectors
XRTK - Mixed Reality Toolkit Change Request
Overview
The platform system aims to solve the problems we're having with multiple build targets on a single hardware platform.
Target of the change:
Is this enhancement for:
Changes