search

microsoft / MixedRealityToolkit-Unity

This repository is for the legacy Mixed Reality Toolkit (MRTK) v2. For the latest version of the MRTK please visit https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity
https://aka.ms/mrtkdocs
MIT License
6k stars 2.12k forks source link

Adding NETFX_CORE compiler flag #522

Closed DominicBoettger closed 6 years ago

DominicBoettger commented 7 years ago

Hello guys,

i have a app for multiple platforms which I also want to release for Hololens. To be able to share assets and prefabs I want to use the same project, but I get compiler errors as soon as the Toolkit is included because it's uwp code. I used the #if NETFX_CORE flag a few times and asked myself if you think it would make sense to add this to every c# script in the sources. What do you think?

Best regards Dominic

StephenHodgson commented 7 years ago

There's already a PR https://github.com/Microsoft/HoloToolkit-Unity/pull/443 and an issue https://github.com/Microsoft/HoloToolkit-Unity/issues/387 addressing some of these concerns (but instead it wraps everything in #if UNITY_WSA).

Here's a poll over on the MSFT Developer forums that asks the community what they think.

IMO I think wrapping 90% of the classes in this scripting define symbol is messy and doesn't address the problem very well. If you need the sharing services for the application to talk to each other, that should port over fine without any extra work.

Like you mentioned, the downside is that you'd need to import your shared assets into each project which can be time consuming, unless you're using asset packages and have good modules setup in git.

I think the ultimate goal is to make everything compile and play nicely across platforms at some point, but I think the HoloToolkit-Unity has a little ways to go before we can start thinking about optimizations like this. I think getting it to be cross platform should be a requirement before submitting it to the Unity Asset store.

DominicBoettger commented 7 years ago

Should we close that issue as it's duplicated? I understand the decision and of course it's not the best solution to use compiler flags, but maybe it's a workaround as long as the toolkit is not compatible....

StephenHodgson commented 7 years ago

I wouldn't say it's a duplicate, because of how the issue is worded and it's an alternate solution, but I think that's up to you on if it's closed. If you decide to close it, just be sure to comment your solution in the other issue, so others have a way to see it.

Yeah I think this is something we all need to discuss as a community and see if anyone has some clever ideas to fix this, without touching every class.

With that being said, I think it's super important for us to get cross compatibility at some point.

zajako commented 7 years ago

I'd rather messy code than code that prevents building the application on all non-Microsoft operating systems.

StephenHodgson commented 7 years ago

While I'm all for making the project cross platform, I think it's important to keep a few things in mind.

  1. Code of Conduct.
  2. Maintainability (Messy code could potentially create more work for others in the future).
  3. The main page clearly states that this project targets Windows Holographic.

    The HoloToolkit is a collection of scripts and components intended to accelerate development of holographic applications targeting Windows Holographic.

zajako commented 7 years ago

Just because it targets Windows Holographics doesn't mean it should prevent compiling for other platforms. For example, I have a scene for mobile apps and a scene for hololens. The hololens uses scripts and features from the mobile scene. Due to unity not allowing scripts or folders to be excluded from specific builds, it means we have to do this via pre-compiler tags in the scripts.

Another solution could be to have the entire set of c# scripts compiling into one External Plugin for unity. Which can be excluded from all other platforms.

As for pre-compiler tags being messy, I honestly think all pre-compiler tags look bad, however, they are a requirement for multi-platform and Unity.

Also pre-compiler tags are a fact of life for Unity development and this is HoloToolkit"-Unity"

StephenHodgson commented 7 years ago

Another solution could be to have the entire set of c# scripts compiling into one External Plugin for unity. Which can be excluded from all other platforms.

I really like this idea. In fact I've played around with the some builds like this and it also helps cut down compile time.

I think the project could be refactored a bit more to to compartmentalize the platform specific stuff a bit more is all. I think the general focus for MSFT has been Windows 10 Only, which is their OS so it makes sense. FYI, the sharing service is definitely portable. IMO it's really the only important part for cross platform support, where all the other features focus on Spatial Understanding and Windows Holographic Input support.

Also pre-compiler tags are a fact of life for Unity development

😆 Isn't that the truth!

StephenHodgson commented 6 years ago

@DominicBoettger is this still an open issue?

DominicBoettger commented 6 years ago

Hi @stephenhodgson,

I think this is solved.... Thanks!