-
Notifications
You must be signed in to change notification settings - Fork 286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch From PCLs to .NET Standard #573
Comments
Its good to switch to standard since PCL's are deprecated now and in the future that could mean it will be harder and harder to use other libraries. However there are quite some things that have to be researched/changed beforehand as you already pointed out. |
Related note on Twitter about .NET Standard libraries has a nice and short way to put it:
This reflects the fact that .NET Standard is, unlike PCL, not a "build once, run anywhere" deal, but a "build once, then use it for building for any platform" deal. Unfortunately, this clashes with the current portability model of Duality, which is "build once, run anywhere [just switch launcher and backend]". We can deal with this for all the official packages, but there will be cases where people publish Duality packages that depend on .NET Standard libraries with different dependency situations across platforms, which will be lost when used in Duality because the editor is a .NET Framework runtime. For the most part, this won't really matter - we'll still get Mono compatibility and a bit more on top of that, and we haven't really had a lot of cross-platform experience beyond that so far anyway. It may turn out that, for true, robust platform support, we will need a proper build pipeline and different sets of dependencies built into the editor anyway, and if anything .NET Standard might help with that by unifying this requirement across all platforms and pushing the issue. |
Side note about NuGet packing support for NetStandard packages: http://blog.tdwright.co.uk/2017/11/20/the-perils-of-publishing-a-net-standard-library-to-nuget/ |
Hey @ilexp a few days ago I converted Duality to .Net Standard. I have not put up a pull request yet because I had run into some problems when trying to use .Net Standard 1.1. Because of those problems, I ended up using .Net Standard 2.0 instead. However, when using 2.0 almost no code changes were needed, and I have been able to run Duality with no problems so far. In fact only two code changes were needed for Duality and all of it's plugins (or at least the plugins in this repo) combined! And the changes were just removing code that had become obsolete because of .Net Standard. Needless to say I don't think using 2.0 instead of 1.1 will cause compatibility problems. But never the less, I wanted to make sure you are okay with using 2.0 instead of 1.1, also I wanted to know which branch I should put a pull request on (if any). On a side note, I have only been using Duality for a few weeks now, but I really like it, and hope to be able to help make it even better. |
Hey @Caylis1397, and welcome! Glad you're up for a PR on this 🙂 The base branch for working on this should be I think that we should try to target .Net Standard 1.1 first though, since the API surface we require should be as small as possible with multi platform support in mind. Can you be more specific which problems you encountered by targeting 1.1? I'll be off for a week, but I'll get back to you after that. |
Well as far as 1.1 goes, there is a lot of missing api. For instance, here are some of the calls to missing api in Duality.Backend.DefaultPluginLoader: All of those are not available in .Net Standard 1.1. Some of those have workarounds, but I can't figure out how to get past most of the missing api. In NVorbis.ParameterChangeEventArgs, the serializable attribute is also missing. So if 1.1 is a hard requirement, then this will take a lot of work and planing to figure out. I do have a lot of free time, but I also have other projects, and some of them are semi time sensitive. So I can definitely give it a shot, it just might take a while. The real concern I have though, is that I might not have the knowledge / skill to find away around all these missing api calls. If 1.1 is not a hard requirement, and you don't want to use 2.0. Then I am sure there is a workable middle ground. I would still like to help out on this. So whatever way you think we should go about this, I will definitely try to help in any way I can. |
I don't think we want to have to implement all those :P Is there a version before 2.0 and after 1.1 that does cover most of the used API? Also Is .NET core 2.0 really that much harder to use cross platform? Wasnt that the whole point of core? |
Do you really care about Windows 8.1 (Store apps), Windows Phone 8.1, and Windows Phone Silverlight 8.0? There's no launcher for these anyway... I say just make the jump to .NET Standard 2.0 now. All modern .NET supports it:
See this table for more details. |
But if you do care... .NET Standard 1.3 adds most of the filesystem and crypto APIs. The assembly load stuff can be handled by cross-targeting (e.g. use the current code on .NET Framework/Mono and use |
Looking at that table I say we go for 2.0. Windows phone is completely dead (I can know it I used to own one) and windows 8.1 is close to being dead. Not worth it imho. |
I do agree that .Net Standard 2.0 is the way to go. After all, when converting Duality to 2.0 the only problems I found were caused by Visual Studio's new project format. And I still have not run into any bugs when using Duality running on 2.0. Or at least none that were caused switching to 2.0. But, 2.0 does have one down / up side; the number of available APIs in 2.0 is 32000 compared to 1.6's available APIs of 13000. The upside to this is that there are a lot more available tools to use, but the downside is that having so much functionality also means having less portability. I don't think this will be a problem, but it should be considered when making a decision. |
I would consider the following: if we go with .Net Standard 1.1, would we end up switching to 2.0 in the future anyways? If so, it seems we might as well 'bite the bullet' and go with 2.0 now. |
It seems like the discussion is currently leaning towards adopting .NET Standard 2.0, but I do think that there is value in restraint when it comes to API usage. The less we use, the easier platform adoption is - and this includes potential future platforms as well, which may work with a reduced API subset. My point is not that we should bend over backwards to get compatibility with 1.1, but that I think we're so close to 1.1 that it makes sense to try to stick to it, rather than abandoning it on the first sight of trouble. If we can't make it work, 2.0 makes sense, but I think we should put a bit more energy into trying 1.1 first.
That's an option too, though I don't think we need them for the most part. Let's investigate where the problem originates from and see if we can fix remaining compiler errors on a case-by-case basis. For example, all file system access has been removed from the core lib in v2.0, and now lives in the system backend plugin, which is unaffected by the PCL-to-.NET Standard switch.
With .NET Standard 1.1, is the new If that's the case, can you fix this by moving the file elsewhere and adjusting the links from all other projects? How does the .NET Standard 1.1 compile hold up error-wise after the fix? |
Hey @ilexp, Thanks for pointing out that the Default Plugin Loader was not part of Duality's main core, I had missed that completely. And when I moved it to somewhere else, Visual Studio stopped complaining about most of the errors, even ones that were unrelated. All that was left were 4 errors in Duality.Drawing.PixelData do to Parallel.ForEach not being part of .Net Standard 1.1 by default. However, that was easily fixed by adding a direct reference to System.Threading.Tasks.Parallel in Duality's project. It now compiles and runs without an errors! TL;DR: @ilexp all that needs to be done now is to change to a newer version of NuGet, correct? I have never published anything on NuGet so I don't know what changes will be required, but your the one who publishes Duality on NuGet anyways right? If so, then I'm sure you can do that part. |
That's great news 🙂
I'm no longer sure if this is really a hard requirement (discussion in #574), and also not yet sure if this is all that needs to be done. Either way, I'd say we should proceed with this step by step and see how that works out. To make sure we'll have a safe way back, I have created a new |
Nice job on the progress @ilexp @Caylis1397. Looking forward to switching to core.
As far as I know all core projects use the new .csproj format. It includes *.cs files by default (which for the most part is good since it dramatically reduces the size of the csproj file). No need to move the file though as you still can exclude them. |
Moving this to the Future milestone, as it increasingly seems like this is going to be a bigger feature to complete than anticipated. As I'm currently moving toward slowly wrapping up v3.0, I think this will have to remain in a branch for now, to be considered for binary release at some later point. This will also give us more time to gather experience with this in practice. tl;dr: Let's carry on pursuing this, but don't expect it in v3.0. |
Progress
ToDo:
|
When #710 is mergedif then duality should be able to install multi framework packages that also target netstandard. No need to update nuget to v4.x it seems saving us a ton of work. If we also update |
VS2019 no longer supports PCL's out of the box: It is possible however to install a targeting pack for it. |
I created a new branch here which is based upon the netstandard branch we had. Some changes:
Tests are green and iam able to start duality and do some simple stuff. Haven't gone much farther than running a scene with a rigidbody that is falling. EDIT: some more changes:
Also noticed that starting the game from visual studio breaks debugging. Attaching after the fact works fine for some reason. Its not loading anything according to visual studio: |
So let's pick this up again 🙂
Looking at the branch comparison, it seems like something's wrong there. All those "merge master into release" commits shouldn't be part of the branch. Can you create a new branch off the current master and cherry-pick the commits we need?
Nice work 👍
This could be a critical issue, so we should investigate a fix or workaround for this first to get a clear picture of our options in proceeding. Progress
ToDo:
Did I miss any ToDos? |
Ugh it doesnt happen often but I made a error with git :(. Now to fix it... EDIT: Fixed it but I had to do a force push so if you have the branch locally you have to discard your local one.
For this I found a solution since the buildoutput is no longer copied to the build output folder with package reference:
Yes for the projects that stay on the netframework we need to target 4.7.2. I already did this in my netstandard branch but idk if that needs to be done elsewhere as well. Also the project template for the plugin project that gets generated for the enduser needs to be updated to netstandard2.0 instead of 1.1. |
Extended the status message / ToDo list: Progress
ToDo:
Not sure whether it really makes sense to update all those dependencies. No harm done to move to the same level as the Duality editor and Desktop backends, but is there any gain from it doing it in a bulk edit? Sounds to me like we might as well leave them be and update when they require any newer framework on their own. They can be used just fine from 4.7.2, whichever framework they're targeting themselves, correct? Let me know if I'm missing something here. Affected projects would be:
|
This was broken while attempting to migrate to the new csproj format to clean up the nuget packages and make that more consistent. I decided to do it since all the plugin projects are already migrated now. Its not strictly neccessary. Since it turned out to be more complex than anticipated it might be better to do this later (and create a separate issue about it).
Yes, you will get warnings if you try to reference netstandard 2.0 though but it should work. |
I reverted the csproj format change, tests are working again. EDIT: Instead of adopting the new clean csproj format for the tests I only migrated to packagereference. This achieves the goal of not mixing the old and new nuget restore behavior without breaking any tests. All projects are now using packagereference. |
I have been thinking about this, why not simply set the output path like so: https://github.com/Barsonax/Singularity/blob/master/src/Singularity/Singularity.csproj#L24 For the netstandard projects you also need to disable appending the targetframework like so:
That should make sure everything ends up in Build\Output or whatever output folder we require.
Currently making a build of singularity that only targets netstandard (I only had the netframework for compatibility with duality anyway) so we should be able to test it end to end soon :). EDIT: alternatively you could specify the output folder when calling msbuild and keep the csproj file clean? |
Update on the current progressProgress
ToDo:
BugsTest what happens when confronting the Duality package manager with a package that targets only .NET Standard, and nothing else. If that doesn't work, fix it.
Might be related to the fact that with packagereference only root dependencies are installed? |
Not sure how to fix the bug I just found. Seems to be a bug in nuget but since its 2.14 I doubt it will ever get fixed. |
Didn't find time yet for a full response, but regarding the singularity NuGet thing, the runtime exception does look related to the package installation problem, could be missing or misplaced dependencies / libs. Did you try what happens when you manually package it with an explicit |
The package is correct its nuget that is not correct here. If you change the packagemanager target to netstandard it does include singularity (and alot of other dlls). |
Assumed as much, but wondered about potential workarounds. That said, if you're using the new default way to create a package for .NET Standard, and that fails, we're in trouble regardless. While it would be possible to require Duality packages to be compatible with the old NuGet, that would cut us off from the bigger part of the global package repository at some point as potential dependencies. Not great at all. So, we might actually be on the safer side if we backlogged this one again and first tackled #574 in order to proceed. But since we're softblocked by #668 right now anyway, might as well do that for a proper solution. |
Made a issue for this: #737 |
Summary
Microsoft has officially deprecated Portable Class Libraries, which is what Duality is based on with regard to portability. Projects still compile just fine and will for the foreseeable future, but it will get progressively harder and less intuitive to keep things working and expanding on it. Consider switching all PCL projects to .NET Standard for v3.0.
Analysis
csproj
format. Don't use the.json
format, seems pretty much deprecated by now.csproj
PackageReference
item is not supported for all project types yet, see here. Might be better to stick topackages.config
for now.csproj
package reference format requires at least NuGet 4.0.The text was updated successfully, but these errors were encountered: