XRTK / com.xrtk.core

The Official Mixed Reality Framework for Unity
https://xrtk.io
MIT License
309 stars 34 forks source link

Improve Platform support configuration to handle multiple variants correctly #378

Closed SimonDarksideJ closed 4 years ago

SimonDarksideJ commented 5 years ago

XRTK - Mixed Reality Toolkit Feature Request

Is your feature request related to a problem? Please describe

Current Platform evaluations for services do not cover the full extent of the potential requirements, given that services require the ability to operate in the following conditions:

  1. Editor Only - Service that only runs in the editor (mock) - any platform
  2. Editor AND Platform - Service that only runs in the editor (mock) - specific platform
  3. Platform(s) Only - Service that only runs on the platform (actual)
  4. Everything - Service that is designed to work both in the editor and on any platform

Currently, only the first three modes are supported based on #374

As we expand the list of potential platforms (#377) these capabilities need to be reviewed

How would you classify your suggestion

Describe the solution you'd like

(Leading from the example given in #374)

Figure Editor Runtime Description
1 X Only Editor
2 X X Both Editor and Runtime
3 X Only Runtime
4 X (or) X Everything
  1. This should run ONLY in the editor regardless of the target build platform but never the build:image

  2. This should run ONLY in the editor if the build target IS Selected:image

  3. This should never run in the editor and only runs on the build:image

  4. This can be run in the Editor and any build target (Image tbc)

Describe alternatives you've considered

No alternatives currently exist

Additional context

Related to #377

StephenHodgson commented 5 years ago

We already do this sans case 4.

The 4th case could easily be handled in the service implementation.

SimonDarksideJ commented 5 years ago

Agreed, this is simply setting the requirement for the entire ask (so nothing gets missed in testing)

WIth 4 however, given the platform Utility is currently dictating when a service can be run, you couldn't do that in the service implementation :D But it's something to review. Maybe by having a service define it's "supportability"

StephenHodgson commented 5 years ago

I am open to doing something similar to #377 though

StephenHodgson commented 5 years ago

With 4 however, given the platform Utility is currently dictating when a service can be run, you couldn't do that in the service implementation

Yeah that's exactly what I was saying 😅

You'd essentially enable the EVERYTHING flag and then in your implementation #if/#else out the implementation.

StephenHodgson commented 5 years ago

But I'm also having a hart time trying to find a use case for 4.

Could you describe it to me?

StephenHodgson commented 5 years ago
  1. This can be run in the Editor when the build target is selected and can ALSO run in the Build

This just sounds like case 2.

I think it should be updated to read:

  1. This runs in the editor ALL the time, and only on the selected platform build
SimonDarksideJ commented 5 years ago

Simply put, it meets these two conditions

In option 2, it would only ever run in the editor for that platform and not the player. (you have a mock service for a platform)

StephenHodgson commented 5 years ago

Simply put, it meets these two conditions

  • it will run in the editor if the selected build target is the same as the editor target
  • it will also run in the selected target player.

This is word for word case 2:

This should run ONLY in the editor if the build target IS Selected:

StephenHodgson commented 5 years ago

it would only ever run in the editor for that platform and not the player

Is this more of what you want case 4 to be?

If we're doing a mock service for this use case why can't it be platform agnostic if it's just a mock service and use case 1?

StephenHodgson commented 4 years ago
Figure Editor Runtime Description
1 X Only Editor
2 X X Both Editor and Runtime
3 X Only Runtime
4 X (or) X Editor OR Runtime
StephenHodgson commented 4 years ago

Can I get confirmation that use case 4 is effectively Everything in the current dropdown?

SimonDarksideJ commented 4 years ago

Updated requirements

StephenHodgson commented 4 years ago

Can we agree that the current implementation already satisfies this issue?

SimonDarksideJ commented 4 years ago

The implementation discussed last night, where a service can run all the time did satisfy the requirement.