search

AdamsLair / duality

a 2D Game Development Framework
https://adamslair.github.io/duality
MIT License
1.4k stars 291 forks source link

Moving package management to a csproj #786

Closed Barsonax closed 4 years ago

Barsonax commented 4 years ago

In order to move forward with duality we have to find a way to make dualities way of managing packages compatible with .netnstandard en .netcore.

As it already turned out the nuget upgrade is very painful, so much that I don't really see this upgrade happening anymore.

This issue is to research a completely different approach in which we actually remove the build in package manager from duality. Instead managing packages will be done like any other .NET project, from a csproj.

This has a number of advantages:

Some disadvantages:

Most optimal solution would be to stay as close to a default csproj as possible, which means I would lean more towards modifying duality to fit with the default instead of csproj wizardry.

Work has begun on a poc duality template https://github.com/Barsonax/DualityProjectTemplate

Barsonax commented 4 years ago

@ilexp

SirePi commented 4 years ago

Maybe something like this could be used? https://stackoverflow.com/questions/53474581/moving-dependencies-to-another-folder-during-release-build If it works, we could have the main folder with the executable only, and everything else (Duality + plugins) in an "engine" folder?

Barsonax commented 4 years ago

That could work but would require ppl to manually edit the csproj every time they change a dependency. Would prefer a solution that just works out of the box when using the standard tooling. From both user experience and a maintenance perspective this would be a good goal to keep in mind.

Which is why I was thinking of modifying duality so it will just work if all dlls are in the same folder.

However @ilexp probably put plugins in a plugin folder for a good reason so iam interested in what his vision is on this.

SirePi commented 4 years ago

Ah, I was more thinking of having an on-install script run when Duality is added to the project, but sure, better to wait word from the boss 😄

ilexp commented 4 years ago

However @ilexp probably put plugins in a plugin folder for a good reason so iam interested in what his vision is on this.

Mostly discoverability and general tidyness. The Plugins folder is watched for changes in the editor, and it's easy to iterate files in the folder to load them dynamically. However, since we don't have only plugin assemblies in there anymore and already do some filtering to only load .core.dll or .editor.dll assemblies, there is no harm done in putting them elsewhere. Bottom line is, it would be nice, but it's not a technical requirement.

But back to the start: Leveraging msbuild / VS package management instead of our own is a great idea, 👍 for that.

Having a custom package manager was nice at some point in the past, but by now it's a liability and a constant drain of resources due to maintenance cost. We should get rid of it as soon as possible without downgrading the user experience, especially on the first run. Duality has managed to be very "plug and play", and it should continue to be, both for experienced developers and beginners. If we figure out a way to achieve that, we're good to go.

Besides shedding some weight, I also think the "csproj based" way is a great opportunity to be one step closer to the developer who just quickly wants to set up a project. There could even be a VS project template that creates a full, up-to-date Duality setup in a project folder of choice. The editor and launcher would be nothing but local tooling that is installed into a user-created and controlled project for their convenience - not the host of everything they do.

Getting to the tech part. On the top of my head, here are a few points we need to figure out:

  1. How will the first install and start of Duality work?
    1. What will be in the download package?
    2. What will the user be required to do before they can launch the editor?
    3. Can we come up with some sort of executable that the user can launch to "unpack" everything? Essentially running msbuild / nuget on a bundled csproj I'd assume.
  2. How do we handle game content in packages?
    • Like shaders or materials bundled with a library that uses them, additional resources for editor support, etc.
    • Sample packages make heavy use of this.
    • They will need to end up in a Data subfolder on package install, and be removed on uninstall.
  3. How do we handle source code in packages?
    • It will need to end up next to the user's .csproj files and ideally be included in the solution.
    • Sample packages make heavy use of this.
  4. How will the directory structure look like in the first place?
    • Where do editor and launcher go, where will all the binaries be, where the game data, and the imported source data?
    • Given the fact that both tooling and game data should be persistent, they can't just be in the output directory of some project, which gets cleaned whenever VS does a rebuild, or the user clicks the wrong button. We will need a VS-independent folder structure somewhere.
    • How do we generate that, and how do we get msbuild to put our stuff there?

Overall, I think we can't get away without some msbuild programming here, maybe via .props and .targets files that are references in the "host" .csproj. However, if we do that, we might lose the use case where someone just installs Duality into their project, which would be a shame.

What about nuget install scripts? Do they still exist and are they supported long-term?

ilexp commented 4 years ago

Some quick research links on nuget install scripts:

Seems like they're no longer supported in all tooling. The concept of content files looks interesting though and could maybe (?) offer some help:

Barsonax commented 4 years ago

How will the first install and start of Duality work?

The download package will contain a solution with all the required dependencies in it. We could make a small bootstrap script to run nuget restore and build it. The solution + assets should be the source of truth here though and this also means you only have to check in the solution + assets when using source control. No binaries in source control is a nice improvement I think.

How do we handle game content in packages?

How do we handle source code in packages?

We could use the content files feature of nuget for this. One difference is that these files are immutable though. There doesn't seem to be another way to do this with packagereference.

How will the directory structure look like in the first place?

We could leverage a custom output path in the csproj. As packagereference should include all transitive dependencies you could build the the editor or launcher project and it should output everything you need to run it in the (custom) output path. VS does not clean this entire folder when rebuilding or cleaning, only the files that are part of the build. As long as these build artifacts are immutable after a build that should be fine.

Still thinking how we will bundle the assets though, I think we should put these in a separate folder?

ilexp commented 4 years ago

The download package will contain a solution with all the required dependencies in it. We could make a small bootstrap script to run nuget restore and build it. The solution + assets should be the source of truth here though and this also means you only have to check in the solution + assets when using source control. No binaries in source control is a nice improvement I think.

Sounds good. Maybe we can get down to just a .csproj, .sln and maybe an optional "extract + run" convenience script.

We could use the content files feature of nuget for this. One difference is that these files are immutable though. There doesn't seem to be another way to do this with packagereference.

I think immutable is generally okay for content and source that comes bundled with a package - it's not meant to be modified, and any modification to them in a project would be gone when updating the package. We should still check what exactly immutable means - does it flag the files as read-only? Are they only read-only in Visual Studio?

As long as the immutability is VS only, we might get around it still to allow users to experiment with samples as intended. Content files could be edited as usual in the editor, and bundled sample source files come with their own .csproj anyway, in which they would be editable again because from the bundled .csproj's perspective they're just files, not added via package. We should only make sure to not include the source files in the project that references the package to avoid having them around twice.

We could leverage a custom output path in the csproj. As packagereference should include all transitive dependencies you could build the the editor or launcher project and it should output everything you need to run it in the (custom) output path. VS does not clean this entire folder when rebuilding or cleaning, only the files that are part of the build. As long as these build artifacts are immutable after a build that should be fine.

Ah, so if VS doesn't touch stuff it didn't put there in the first place, that could actually be fine.

Still thinking how we will bundle the assets though, I think we should put these in a separate folder?

I'd recommend not changing Duality's general directory structure in the first step - it's an option, but as long as we can manage without, that would be the safer route.

So let's say we use a custom build output path that is the folder where the .csproj and .sln are itself, that would mean all the required binaries, launcher and editor would end up right there as well. The editor will create its own subfolders as needed when run, and all packages bundling content could target a Data\PackageXY subfolder. Bundled sample source code could target Source\Code\PackageXY, but that would only make sense if the users game plugins are there as well. So where is the user going to write their game code?

I think we could use an overview on the directory structure and files we're going for, so we have something concrete to talk about and iterate on. @Barsonax Can you create one based on prototype and latest thoughts in the thread?

Barsonax commented 4 years ago

Sounds good. Maybe we can get down to just a .csproj, .sln and maybe an optional "extract + run" convenience script.

I think so so this will make the initial package really small and we could probably turn it into a VS template or do something with dotnet new so you can start a new duality project purely from the command line. That's more stuff for the future though but a very nice to have.

I think immutable is generally okay for content and source that comes bundled with a package - it's not meant to be modified, and any modification to them in a project would be gone when updating the package. We should still check what exactly immutable means - does it flag the files as read-only? Are they only read-only in Visual Studio?

Last time I checked you could actually modify them from visual studio. This seemed like a bug to me. Content files are shared (as in they don't actually reside in the solution directory) so I don't think you even want to modify them as that can cause some issues with other projects referencing those files. Even if you do as soon as you push it to a CI server your changes would be completely gone.

So let's say we use a custom build output path that is the folder where the .csproj and .sln are itself, that would mean all the required binaries, launcher and editor would end up right there as well. The editor will create its own subfolders as needed when run, and all packages bundling content could target a Data\PackageXY subfolder. Bundled sample source code could target Source\Code\PackageXY, but that would only make sense if the users game plugins are there as well. So where is the user going to write their game code?

Plugins should be a separate csproj but we can add it to the same solution file.

I'd recommend not changing Duality's general directory structure in the first step - it's an option, but as long as we can manage without, that would be the safer route.

Yup we can try to see how far we can get without changing the directory structure. I don't think we can get to 100% though but it will make the bigger picture more clear.

I think we could use an overview on the directory structure and files we're going for, so we have something concrete to talk about and iterate on. @Barsonax Can you create one based on prototype and latest thoughts in the thread?

Ye agreed I can setup a template, it will not work yet as it will require some changes in duality but it would make our idea's more concrete than just a discussion.

Barsonax commented 4 years ago

Worked a bit more on the template, its not in a working state but should give a clearer picture of the idea: https://github.com/Barsonax/DualityProjectTemplate

Issues/things to keep in mind:

@ilexp

Screenshot of the output folder: image

ilexp commented 4 years ago

Looks pretty good so far. We should keep track of all the small issues and todos though, so here's the old progress template that served me well in the past. I actually have that as a saved issue response and can only recommend it:

Progress

(pretty much all @Barsonax work)

ToDo

Barsonax commented 4 years ago

One thing Iam thinking of now. To properly test this I think we need to release a new nuget package but we don't want to break anything. I think we should make it possible to put a prerelease version (like -alpha suffix) on the nuget package. That way we can test it safely. Shouldn't be too hard I think.

ilexp commented 4 years ago

With the integrated package manager, I just put all the requried packages in a local folder on my machine and set that as a repository URL. I'm not sure nuget supports this as easily via VS, but you can add custom repository URLs. Maybe that's worth looking into?

If possible, I'd like to avoid releasing anything for testing. Another alternative would be to set up a custom package repository somewhere if it can't be done locally. Didn't GitHub add this as a feature for projects recently?

SirePi commented 4 years ago

It's definitely possible to have a local nuget repository, it's not that difficult even with command line. See https://docs.microsoft.com/en-us/nuget/hosting-packages/local-feeds

Barsonax commented 4 years ago

Ok Ill just test it that way then, thanks for the tip

Barsonax commented 4 years ago

So how do I build the duality packages? Getting this error when trying to run nightlybuild

C:\git\duality\Build\Documentation\Documentation.shfbproj(117,3): error MSB4019: The imported project "C:\SandcastleHelpFileBuilder.targets" was not found. Confirm that the expression in the Import declaration "\SandcastleHelpFileBuilder.targets" is correct, and that the file exists on disk.

EDIT: So had to disable the documentation build and after that download nuget.exe (because it couldn't find it?). Now I got the nuget packages so testing can start.

Barsonax commented 4 years ago

@ilexp how do you update the versions of the packages? You surely don't do this manually for every nuspec iam guessing? I saw there is a VersionUpdater project but it does not work because it depends on sourcetree which I don't have...

Could not find a part of the path 'C:\Users\Rick\AppData\Local\Atlassian\SourceTree\git_local\bin'.'

I think the build code can also use a bit of a cleanup to make this easier (it should handle all dependencies itself in my opinion). This is also why I prefer to use a tool like https://nuke.build/ because that will just work ootb as it does this all for you (the only thing you really need is .NET pretty much). Not something we should do now though but for the future.

After I fixed the path the version updater project doesn't seem to do anything and just exits.

ilexp commented 4 years ago

I can see you have already found some of the build tools, but for clarity's sake I'll back off a few steps and start at the beginning:

What you need for building packages and updating package version is the Build\Scripts folder, specifically Package Binary NoDocs.bat and Update Package Versions.bat.

Update Package Versions

Package Binary

Barsonax commented 4 years ago

Running into a weird bug: image So apparently it has a value but when I assign it to a variable it turns into a null: image

Assigning foo in the intermediate window does work and when you do this duality editor launches succesfully. Is this some native code trickery?

This bug happens when I move the dll's from the plugin folder to the root folder and then start duality. Paths are changed so it does find the dlls.

It all happens on the same main thread so it does not seem to be directly threading related.

There doesn't seem to be anything special about the ActiveInstance property either:

        public static GraphicsBackend activeInstance = null;
        public static GraphicsBackend ActiveInstance
        {
            get { return activeInstance; }
        }

The code that fills the value does run (and before the other code).

ilexp commented 4 years ago

Assigning foo in the intermediate window does work and when you do this duality editor launches succesfully. Is this some native code trickery?

None that I know of.

This bug happens when I move the dll's from the plugin folder to the root folder and then start duality. Paths are changed so it does find the dlls.

Now that you mention it, there may be one behavior that could cause issues when the plugins are in the main folder: The main folder is also where .NET natively searches for assemblies - so it might skip our custom AssemblyResolve for plugins, because the used AppDomain event is only invoked "when the resolution of an assembly fails." (see here).

That means, we would have different and potentially runtime-dependent behavior as soon as we start putting plugins in the main folder, with potentially nasty side effects - for example loading an assembly twice and now having two identically named types that are incompatible to each other, and to make things worse, it would be undefined which of the two is instantiated dynamically when deserializing stuff, or looking it up by name.

This definitely needs to be prevented. It might be that we actually need to put the plugins outside the main folder for that reason, simply because that way we can control how types are resolved. Even if it didn't break at runtime, it probably will in the editor when attempting to do a hot-reload of plugins on detected changes.

If we can't rule this out as a potential source for trouble, I'd say it's back to having a Plugins folder.

Barsonax commented 4 years ago

Yeah the dll's are getting loaded twice. Checked it with AppDomain.CurrentDomain.GetAssemblies(). I first expected it to prevent that from happening if its the same file but it apparently does not.

This caused the same static class to actually have 2 instances, even confusing the debugger to make it even nastier.

I think with some changes on the duality side this can be prevented though. We don't really need to load plugin dll's ourselves anymore if .NET already does that for us.

EDIT: this code seems to do the actually loading

        public Assembly LoadAssembly(string assemblyPath)
        {
            // Due to complex dependency resolve situations intertwined with our hot-reloadable
            // plugin system, we should manually resolve all dependencies. This is only possible
            // when obscuring where a certain Assembly has been loaded from. We need to load them
            // all as an anonymous data block to circumvent system dependency resolve.

            // Guess the path of the symbol file
            string pluginDebugInfoPath = Path.Combine(
                Path.GetDirectoryName(assemblyPath),
                Path.GetFileNameWithoutExtension(assemblyPath)) + ".pdb";
            if (!File.Exists(pluginDebugInfoPath))
                pluginDebugInfoPath = null;

            // Load the assembly - and its symbols, if provided
            if (pluginDebugInfoPath != null)
                return Assembly.Load(File.ReadAllBytes(assemblyPath), File.ReadAllBytes(pluginDebugInfoPath));
            else
                return Assembly.Load(File.ReadAllBytes(assemblyPath));
        }

Like the comment already explains it hides where a assembly is loaded from, thus .NET simply loads the assembly again as it thinks that dll is not loaded yet I think. Since everything is now in the same folder do we even need to do this?

ilexp commented 4 years ago

I think with some changes on the duality side this can be prevented though. We don't really need to load plugin dll's ourselves anymore if .NET already does that for us.

Loading plugins went through a few iterations and edge cases over the years, which I can't even summarize here for lack of complete memory, and we all know how critical it is for Duality to work. It's also been stable for quite a while - so to ensure stability, I don't think we should change behavior for this issue here to work unless there's no other way.

Another point would be that .NET locks the automatically loaded assemblies, so you can no longer do any hot-reloads in the editor, since the loaded plugins can no longer be overwritten. The only way around that is to do a custom load that doesn't lock them.

For now it seems having a Plugins folder is the easier way out. And it's kinda nice to have them grouped that way in the file system anyway.

Barsonax commented 4 years ago

For now it seems having a Plugins folder is the easier way out. And it's kinda nice to have them grouped that way in the file system anyway.

Meh that kinda breaks the UX as you now have to modify the csproj to exclude the duality dependencies when building the project...

Else you end up with all duality dlls in the plugins folder because of transitive dependencies.

ilexp commented 4 years ago

Could we change our approach to not use the binary output folder directly, but instead have it as an uninteresting temp folder somewhere, and then let msbuild copy the interesting parts to their appropriate destinations?

Barsonax commented 4 years ago

So loading the assemblies with Assembly.LoadFrom(assemblyPath); prevent .NET from loading duplicate dll's. Not sure if they are locked now though like you said.

EDIT: Yup it still locks them

Barsonax commented 4 years ago

Could we change our approach to not use the binary output folder directly, but instead have it as an uninteresting temp folder somewhere, and then let msbuild copy the interesting parts to their appropriate destinations?

Have to find a way so that ppl are not required to modify their packagereferences in their csproj for every dependency they add. So we could try to do this in a custom .targets file

ilexp commented 4 years ago

So loading the assemblies with Assembly.LoadFrom(assemblyPath); prevent .NET from loading duplicate dll's. Not sure if they are locked now though like you said.

EDIT: Yup it still locks them

Adding to that, I think the issue really isn't only the loading twice or locking thing - would have to trace back git history for all the details, but this comment in the source you quoted is probably there for a reason:

// Due to complex dependency resolve situations intertwined with our hot-reloadable
// plugin system, we should manually resolve all dependencies. [...]

I'd just like to heed my own warning from the past here. Reaching the same stability (and confidence of) just takes more energy than we should spare as a side quest of this issue.

Could we change our approach to not use the binary output folder directly, but instead have it as an uninteresting temp folder somewhere, and then let msbuild copy the interesting parts to their appropriate destinations?

Have to find a way so that ppl are not required to modify their packagereferences in their csproj for every dependency they add.

I mean, we would give up the ability to just add Duality package references to any new .csproj and end up with a working editor / launcher / game plugin tooling setup, but it reduces complexity for now - we can always improve things later on. Even the most basic implementation of this issue would still be a huge win.

So we could try to do this in a custom .targets file

Sounds good. We do have a naming convention to work with as a first step, which is probably easier to access via msbuild targets than the tags of the package where a file was delivered from - instead, we could just put everything that contains *.[core|editor].??? into the Plugins folder, and everything else in the main folder, keeping deep relative paths.

Barsonax commented 4 years ago

Adding to that, I think the issue really isn't only the loading twice or locking thing - would have to trace back git history for all the details, but this comment in the source you quoted is probably there for a reason:

Well atleast now I know there is a good reason to have a separate plugin folder.

As for the msbuild solution to his problem something like this is a nice start. Its not needed to do this for every plugin or package reference since this solves the problem in a generic way:

  <ItemGroup>
    <PluginFiles Include="$(OutputPath)*core.dll"/>
    <PluginFiles Include="$(OutputPath)*editor.dll"/>
    <LibFiles Include="$(OutputPath)*.dll" Exclude="@(PluginFiles)"/>
  </ItemGroup>

  <Target Name="Foo" AfterTargets="Build">
    <Message Text="Copying plugin files @(PluginFiles) to $(DeployFolder)/plugins" Importance="High" />  
    <Copy
    SourceFiles="@(PluginFiles)"
    DestinationFolder="$(DeployFolder)/plugins"
        />

    <Message Text="Copying lib files @(LibFiles) to $(DeployFolder)" Importance="High" />
    <Copy
    SourceFiles="@(LibFiles)"
    DestinationFolder="$(DeployFolder)"
        />
  </Target>

ToDo

Barsonax commented 4 years ago

So hit another blocker, packagereference does not support copying the documentation xml file... Same goes for the pdb file.

They are in the nuget folder though (C:\Users\Rick.nuget\packages\adamslair.duality.editor\4.0.0\lib\net45)

So we would need to parse the csproj to get the packages and versions then search for them..... Ugh.

ilexp commented 4 years ago

So hit another blocker, packagereference does not support copying the documentation xml file... Same goes for the pdb file.

Potentially related issue: https://github.com/NuGet/Home/issues/4837

There seem to be a few solution approaches in there - although it's still inconclusive, maybe one of them could make overcoming this a bit easier. One of the linked points was GeneratePathProperty, which doesn't seem to solve our issue, but it might help in locating the required paths. This also seems interesting, though we'd need to remodel it to work from the consuming project, not the published package.

On a different note, I think the xml and pdb are not supposed to be flagged as published binaries in NuGet anyway - if we'd flag them as content files (or something else) instead, would they be copied? Would there be any negative effects if they're no longer "binaries" in the nuspec?

Barsonax commented 4 years ago

There seem to be a few solution approaches in there - although it's still inconclusive, maybe one of them could make overcoming this a bit easier. One of the linked points was GeneratePathProperty, which doesn't seem to solve our issue, but it might help in locating the required paths. This also seems interesting, though we'd need to remodel it to work from the consuming project, not the published package.

Yeah but its not very nice if other developers have to start hacking in the csproj when just adding a package. Kinda defeats the point of having a nice UX with a package manager.

Iam fine with adding some complexity in the csproj if we can solve this in a generic way. I tried this but didnt get it to work as it only includes 1 file for some reason:

<PackageReferenceFiles Include="$(NugetPackageRoot)\%(PackageReference.FileName)\%(PackageReference.Version)\**\*.xml" />

On a different note, I think the xml and pdb are not supposed to be flagged as published binaries in NuGet anyway - if we'd flag them as content files (or something else) instead, would they be copied? Would there be any negative effects if they're no longer "binaries" in the nuspec?

Maybe I have to check this.

Barsonax commented 4 years ago

Seems there is a copytooutput on the file node in the nuspec https://docs.microsoft.com/en-us/nuget/reference/nuspec#example-contentfiles-section

EDIT: it seems it still does not copy to the output folder..

EDIT2: after some more fiddling with nuget I came up with this 

<?xml version="1.0"?>
<package xmlns="http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd">
  <metadata>
    <id>AdamsLair.Duality.Editor</id>
    <version>4.0.0</version>
    <title>Duality Editor</title>
    <authors>Fedja Adam</authors>
    <owners>Fedja Adam</owners>
    <licenseUrl>https://github.com/AdamsLair/duality/raw/release/LICENSE</licenseUrl>
    <projectUrl>https://github.com/AdamsLair/duality</projectUrl>
    <iconUrl>https://github.com/AdamsLair/duality/raw/release/Build/NuGetPackageSpecs/Icons/Duality.png</iconUrl>
    <requireLicenseAcceptance>false</requireLicenseAcceptance>
    <description>The Duality editor.</description>
    <summary>The Duality editor.</summary>
    <releaseNotes />
    <tags>Duality Editor</tags>
    <dependencies>
      <dependency id="AdamsLair.Duality" version="4.0.0" />
      <dependency id="AdamsLair.DockPanelSuite" version="2.8.2" />
      <dependency id="AdamsLair.TreeViewAdv" version="1.7.7" />
      <dependency id="AdamsLair.WinForms" version="1.1.17" />
      <dependency id="AdamsLair.WinForms.PopupControl" version="1.0.1" />
    </dependencies>
    <references>
      <reference file="DualityEditor.exe" />
    </references>
     <contentFiles>
      <files include="any/any/DualityEditor.xml" buildAction="None" copyToOutput="true"  />
    </contentFiles>
  </metadata>
</package>

The any/any/DualityEditor.xml should be in the contentFiles folder. If you do this then you will get this file in the output folder. Another thing I noticed that it now also appears in the project view which is not really desired: image

Barsonax commented 4 years ago

Managed to grab the documentation file (and any other files if desired) with some msbuild magic (note iam using Identity instead of Filename now):

<PackageReferenceFiles Include="$(NugetPackageRoot)\%(PackageReference.Identity)\%(PackageReference.Version)\lib\**\*.xml" />

which you can then copy to a desired folder:

<Copy SourceFiles="@(PackageReferenceFiles)" DestinationFolder="$(DeployFolder)/doc" />

https://docs.microsoft.com/en-us/visualstudio/msbuild/msbuild-well-known-item-metadata?view=vs-2019

Only thing I noticed it does not do this with transitive dependencies so gonna look into that.

EDIT this will grab all xml files in the same folder that the reference (dll file) is in. This also includes any transitive dependencies:

<PackageReferenceFiles Include="%(Reference.RelativeDir)*.xml" />

EDIT2: making it a bit more strict to just take the documentation file:

      <PackageReferenceFiles Include="%(Reference.RelativeDir)%(Reference.Filename).xml" />

The file might not always exist however but we can solve that edge case as well by adding a condition to the copy task:

    <Copy SourceFiles="@(PackageReferenceFiles)" 
          DestinationFolder="$(DeployFolder)/doc" 
          Condition="Exists('%(RootDir)%(Directory)%(Filename)%(Extension)')"/>

EDIT3: duality no longer crashes now, Still some erros in the log though but might be because I modified duality which I need to revert now.

SirePi commented 4 years ago

I have been thinking.. maybe we are going at this from the wrong direction. The only part where the hot reloading of libraries comes in play is in the editor, right? I have made a small POC program in .net core 3 with the capability to dynamically load and hot swap dlls.

The "plugin" file remains unlocked, can be renamed / deleted / updated, the program unloads the previous context, loads up the new one and there are no duplicated assemblies present.

So, why not try and go along the route of having a .net core editor (made with Avalonia maybe)? Of course this would also mean that until OpenTK doesn't come out with its .net standard compliant version, we can't proceed with a full upgrade, but that would be the case anyway, no?

If you'd like to try, pressing any key other than Escape will reload ClassLibrary1.dll and print the result of a ToString() method. The default dll prints "SIMPLESTRING" while the -dualityetc version will print a concatenation of an XmlDocument, a json parsed value (123) and a Duality Font resource. See that everytime the font changes, since a new instance is being used. (and the loaded dlls remains constant, without duplicates)

netcoreapp3.1.zip (requires .net core 3.1 as you can guess from the name 😄 )

Barsonax commented 4 years ago

So, why not try and go along the route of having a .net core editor (made with Avalonia maybe)? Of course this would also mean that until OpenTK doesn't come out with its .net standard compliant version, we can't proceed with a full upgrade, but that would be the case anyway, no?

This sounds like a total rewrite of the duality editor. A much bigger change than what we are trying to do now. Might be interesting for the future (duality 5.0.0? :P) though. We do want the duality editor to be crossplatform on netcore (or net 5 in the future) eventually.

Pretty much solved the path issues already, just running into some issues that is caused by duality nuget packages not following the (newer) standards.

SirePi commented 4 years ago

This sounds like a total rewrite of the duality editor. ... We do want the duality editor to be crossplatform on netcore (or net 5 in the future) eventually.

Yes, it would be 😂

Barsonax commented 4 years ago

Managed to get duality working now with the latest changes on https://github.com/Barsonax/DualityProjectTemplate

The only issue is that OpenAL ends up in the root folder instead of the plugin folder. You can copy them yourself for now.

If you want to test this yourself you need to build duality yourself using https://github.com/Barsonax/duality/tree/feature/csprojpackagemanagement, generate the nuget packages locally and add a custom nuget source before building the template as it uses a custom 4.0.0 version with the package manager and default project being removed as the most notable changes. I think I still need to cleanup this branch a bit though as I seem to have checked in a bit more than necessary (the build script kept complaining about not committed changes...), furthermore I the version metadata on the dlls itself is not yet updated.

TODO

@ilexp Does OpenAL really need to end up in the plugin folder? Its not exactly a plugin. Its a bit hard to figure out with msbuild where to put this dll since I have nothing in the name to work with. In https://github.com/AdamsLair/duality/blob/master/Source/Platform/DefaultOpenTK/Backend/Audio/AudioLibraryLoader.cs#L26 it explicitly looks for this dll in the plugin folder so a possible fix would be to look for it in the root folder (or make a new native folder?).

ilexp commented 4 years ago

OpenALSoft64.dll and OpenALSoft32.dll end up in the root but should end up in the plugin folder. Investigate why (probably because it does not have the editor or core suffix). Might have to change the nuget package for this.

That should be fine either way - since they're native binaries, they'll not be loaded as assemblies anyway. We'd just have to change this part of the OpenTK backend to allow them being placed in the root folder.

Edit: Ah, seems you were faster to edit than I was to answer.

Change the dll version metadata to be 4.0.0

The version updater should already take care of this. Saves a bit of a headache to get all versions and dependencies to align 🙂 I usually run it as the last step, after cleanup and everything else has been done.

ilexp commented 4 years ago

add a custom nuget source before building the template as it uses a custom 4.0.0 version with the package manager and default project being removed as the most notable changes

Is there a way to set the templates package references to "latest available"? This would be ideal for the download .zip package. As a neat side effect, that way we'd be able to test without the "4.0.0" versions (package management is inactive anyway, if there is no package config gile) set up locally for the time being.

Barsonax commented 4 years ago

That should be fine either way - since they're native binaries, they'll not be loaded as assemblies anyway. We'd just have to change this part of the OpenTK backend to allow them being placed in the root folder.

Ok then thats a easy fix.

The version updater should already take care of this. Saves a bit of a headache to get all versions and dependencies to align 🙂 I usually run it as the last step, after cleanup and everything else has been done.

The version updater only updated some of the packages so had to manually update the others. Seems it doesn't understand transitive dependencies completely? I ended up with like 4-5 different versions of duality being installed until I manually bumped all versions to 4.0.0.

Is there a way to set the templates package references to "latest available"? This would be ideal for the download .zip package. As a neat side effect, that way we'd be able to test without the "4.0.0" versions (package management is inactive anyway, if there is no package config gile) set up locally for the time being.

I guess this might be possible with floating versions? Not sure if this is a good idea though. Kinda breaks the whole reproducible build thing. Maybe when nuget lockfiles are more mature this would be a good idea.

ilexp commented 4 years ago

Is there a way to set the templates package references to "latest available"? This would be ideal for the download .zip package. As a neat side effect, that way we'd be able to test without the "4.0.0" versions (package management is inactive anyway, if there is no package config gile) set up locally for the time being.

I guess this might be possible with floating versions? Not sure if this is a good idea though. Kinda breaks the whole reproducible build thing. Maybe when nuget lockfiles are more mature this would be a good idea.

So I experimented a bit, and the closest I could get was a fixed major version number, with a floating minor and patch number, like this:

    <PackageReference Include="AdamsLair.Duality.Launcher" Version="3.*" />

The issue is that these floating versions don't disappear after the first install, so as you said they're not a good fit here - not sure about the nuget lock file thing either, probably better to not depend on that as well at this point.

However, this could mean that we'd need to do a new .zip / download release every few big updates or critical fix and leave all the small and patch updates for the users. We'd no longer have a "new install is latest" by default. Might be fine, but requires a slight shift in perspective and potentially adjusted workflow here and there. Something to keep in mind.

Edit: If we have an optional "install me" convenience script that people can just double click, we could sneak in a nuget update though, right? Hypothetically speaking, we'd still need to figure out if this is something we want.

The version updater only updated some of the packages so had to manually update the others. Seems it doesn't understand transitive dependencies completely? I ended up with like 4-5 different versions of duality being installed until I manually bumped all versions to 4.0.0.

It only updates packages for projects that have been modified, as well as packages that depend on them. If there there no change to a project or dependency, it gets left alone - probably why it didn't offer all of them in the version prompt. If you want a full global update, you can use the ApplyGlobalUpdate option via command line, use "Major", "Minor", or "Patch" as value. Forgot to mention that before, it's probably what you need for this particular update.

ilexp commented 4 years ago

Managed to get duality working now with the latest changes on https://github.com/Barsonax/DualityProjectTemplate

Just tested this (using floating 3.* package references so I don't have to do the local package repo part), looks good and works 👍

Only thing I had to fix manually was move "Nuget.Core.dll" to the main folder, because it ended up in Plugins, being a false-positive to our naming convention. Not critical for now, because it happens to be in a dependency we're getting rid of though. If a user dependency triggers the script that way and ends up in Plugins, it should be loaded just fine via custom assembly resolve. The reason it fails on NuGet is because this is needed before anything else is in place.

Barsonax commented 4 years ago

Only thing I had to fix manually was move "Nuget.Core.dll" to the main folder, because it ended up in Plugins, being a false-positive to our naming convention. Not critical for now, because it happens to be in a dependency we're getting rid of though. If a user dependency triggers the script that way and ends up in Plugins, it should be loaded just fine via custom assembly resolve. The reason it fails on NuGet is because this is needed before anything else is in place.

Nuget.Core.dll shouldn't even be there I think you are still using the old duality code? I remove the package manager and its dependencies in my branch.

However, this could mean that we'd need to do a new .zip / download release every few big updates or critical fix and leave all the small and patch updates for the users. We'd no longer have a "new install is latest" by default. Might be fine, but requires a slight shift in perspective and potentially adjusted workflow here and there. Something to keep in mind.

Hmm but currently with the zip install you have to update that zip right?

We could always just run nuget update or make a script for that. Easy enough now we are using the default tooling :).

EDIT: ah you already mentioned this dll is removed :P

Barsonax commented 4 years ago

@ilexp I noticed there still some logic in duality related to creating a new project. I don't think we need this anymore since this task will be taken over by the template and tools like dotnet new.

Aside from that duality seems to be working now with the openal path fixed. As a bonus we no longer get a (ignored) exception anymore since we were trying to load a native assembly.

TODO

EDIT: yup all content packages are currently broken. This has to do with the nuget package having the wrong format though.

EDIT2: modified the sample packages so that they now use the contenfiles folder. Tested the physics sample and it works. Removed the source files from the package as this does not work with contenfiles. Might be better to put in a link to github for ppl that want to see the source code?

EDIT3: installing Singularity now works as well. The plugin ends up in plugins and the core library ends up in the root folder.

ilexp commented 4 years ago

Sounds great, seems there's quite a bit of progress 🙂

Nuget.Core.dll shouldn't even be there I think you are still using the old duality code? I remove the package manager and its dependencies in my branch.

Yeah, I used the 3.x versions from the online repo since I didn't have time for the local package setup and I just wanted to quickly test how the template is behaving.

Hmm but currently with the zip install you have to update that zip right?

We could always just run nuget update or make a script for that. Easy enough now we are using the default tooling :).

Yep, that's what I meant by the "install me" convenience script. We will need an updated .zip at least once, and I'd probably update it every major version step in case a user ignores the script, so they aren't too far off, but that could be a good solution.

@ilexp I noticed there still some logic in duality related to creating a new project. I don't think we need this anymore since this task will be taken over by the template and tools like dotnet new.

Yeah, take it out as well. That part is from before we even had the builtin package management. It's super legacy. When you do, make sure to check if that makes some editor utility code paths unused, which could be removed as well.

EDIT2: modified the sample packages so that they now use the contenfiles folder. Tested the physics sample and it works. Removed the source files from the package as this does not work with contenfiles. Might be better to put in a link to github for ppl that want to see the source code?

That kind of takes away a big part of why you'd have a sample package in the first place - to show users how stuff can be done, right next to their own code, and even allow them to play around with it. Only distributing binaries with a GitHub link is way less usable.

I think we should at least investigate if there is some way to keep this. Otherwise, we should check how to mitigate the drawbacks.

Do you see any way how source code could be distributed this way? What exactly do you mean by "does not work with contentfiles"?

After the template is working investigate how to replace tokens such as namespace and distribute the template (vsix, dotnet new etc?).

Let's do this as a step two after we've released this the old way / via downloadable .zip. I'm all for it, but I think we should get the basics out there ASAP, and any additional distribution mechanism just inflates the scope right now.

Barsonax commented 4 years ago

Yep, that's what I meant by the "install me" convenience script. We will need an updated .zip at least once, and I'd probably update it every major version step in case a user ignores the script, so they aren't too far off, but that could be a good solution.

There doesn't seem to be a update all command in the dotnet cli (https://github.com/NuGet/Home/issues/4103). However we can probably achieve a similar result with a script using dotnet list package and dotnet add package. The package manager in visual studio can update all packages (it probably uses a similar approach under the hood).

Yeah, take it out as well. That part is from before we even had the builtin package management. It's super legacy. When you do, make sure to check if that makes some editor utility code paths unused, which could be removed as well.

Ok gonna remove that code then. DONE

That kind of takes away a big part of why you'd have a sample package in the first place - to show users how stuff can be done, right next to their own code, and even allow them to play around with it. Only distributing binaries with a GitHub link is way less usable.

You could include the (immutable) source files and not the dll so that when you build the gamelauncher/gameeditor the code is still there. Playing around with it is simply not possible anymore as nuget does not support this, there is no way to modify the source files.

That also changes the assembly qualified name though so that probably does not even work :(. Including them both is quite confusing and in that case might be better to put a link somewhere to the source code.

Another option is to include the cs files but set the build action to none and copytooutput to false so that it only appears in the project view and not in the build output. Thats kinda faking it as the included source code is not actually used..

So basically I do not see a way to do this properly with the newer nuget. Might as well be explicit about it then and direct users to a github link. This is also why I opted to just include the binary.

Let's do this as a step two after we've released this the old way / via downloadable .zip. I'm all for it, but I think we should get the basics out there ASAP, and any additional distribution mechanism just inflates the scope right now.

Ok I will keep it super simple then. Made a new issue #788 to start discussing possible solutions to this.

TODO

ilexp commented 4 years ago

That also changes the assembly qualified name though so that probably does not even work :(. Including them both is quite confusing and in that case might be better to put a link somewhere to the source code.

Another option is to include the cs files but set the build action to none and copytooutput to false so that it only appears in the project view and not in the build output. Thats kinda faking it as the included source code is not actually used..

Yeah, I agree - none of this sounds good. However:

You could include the (immutable) source files and not the dll so that when you build the gamelauncher/gameeditor the code is still there. Playing around with it is simply not possible anymore as nuget does not support this, there is no way to modify the source files.

Don't we copy around stuff from a temp folder to the actual folder now anyway? I thought the immutable part is only valid when it's a file that is included via the builtin automatism?

As long as we aggregate in temp folder A and copy to actual output folder B, we should be in control. For the sample projects, we also don't really need to do anything with the source files - they don't need to show up anywhere or be included in any project, since the samples bring along their own project that compiles to their own plugin. Only thing we'd still need to do is set up the sample project in a way that it will work out of the box without processing the csproj.

(Automatically adding any copied .csproj to the solution file somehow would be an added bonus on top, though I'm not yet sure how that would reasonably work.)

Ok I will keep it super simple then. Made a new issue #788 to start discussing possible solutions to this.

👍

I think we should also update the projects that target pcl to netstandard 2.0 before releasing 4.0.0. @ilexp, would be done in this PR though #741

Yeah, we should do that before the release, but maybe not a dependent part of this issue? I'd suggest the following:

Since we'd have master in a major release cycle lock as soon as anything related is merged, we should ensure there are no critical bugs to fix for any 3.x packages before we start, but as far as I can see that's already the case.

(Thinking about that, might be worth considering to "official" git flow at some point in the future, since it avoids that kind of release cycle lock - but that's a topic for another day)

Barsonax commented 4 years ago

Don't we copy around stuff from a temp folder to the actual folder now anyway? I thought the immutable part is only valid when it's a file that is included via the builtin automatism?

Yes we copy the build output to a temp folder. Its considered immutable because it gets overwritten on every build. The source of truth resides in the (immutable) nuget package.

As long as we aggregate in temp folder A and copy to actual output folder B, we should be in control. For the sample projects, we also don't really need to do anything with the source files - they don't need to show up anywhere or be included in any project, since the samples bring along their own project that compiles to their own plugin. Only thing we'd still need to do is set up the sample project in a way that it will work out of the box without processing the csproj.

I don't know how we could do this. If you want endusers to be able to modify the code you have to do a rebuild of the csproj at some point . Also since its in the build output it will be overwritten by the next build. Better to just make it easy for users to view the source code in github in my opinion. Maybe nuget will support this scenario again in the future.

Yeah, we should do that before the release, but maybe not a dependent part of this issue? I'd suggest the following:

Ye it should be done in separate issues/PR's, already did most of the work for alot of these issues just have to update the branches and fix the merge conflicts once this issue is done.

ilexp commented 4 years ago

Its considered immutable because it gets overwritten on every build.

Ah, got it - overlooked that part 🙂 Thanks!

Better to just make it easy for users to view the source code in github in my opinion.

I agree, that's probably for the best given the options we have. We should add (or just refer to) the link in the package summary, so users can't miss it at least.

On the plus side, the immutability issue was always there and it's reasonable design that the sample packages kind of broke - at least now we align with that. We'll find a proper way to improve the affected aspects in time. Moving on!

Ye it should be done in separate issues/PR's, already did most of the work for alot of these issues just have to update the branches and fix the merge conflicts once this issue is done.

👍

Barsonax commented 4 years ago

Solved in #789