Build configuration migration .net 4.7.1 & .net core 2.0

[Latency]

Take #2. Incorporated suggestions.

Had to create a new branch since my works in progress for my ResourceGenerator rewrite is on master.

[Zawodowiec1532]

Dude, it's too big for just single PR. Nobody will review it. Firstly I recommend you to:

1. Create new branch which is uptodate with Hearthstone-Deck-Tracker/master
2. Push there only ResourceGenerator changes
3. Create PR. :)

After you can create PR which only contain changes CRLF -> LF, etc.

[Latency]

Hello, I know it is big.. it is a MAJOR piece of work. The refactoring for the namespaces of many files and build configurations are required as to it being so drastic. Although, I did fix some of the NPE hotpoints as I went and code analyzed it. I already separated the ResourceGenerator into a different PR. In fact.. I am not finished with that .csproj yet. It was separated out a few days ago and I will be creating one for that in itself probably early next week. The other guys seem to understand the delima with this one. The master curator said he is busy for the next 2 weeks to get around to reviewing it. You are more than welcome to help out. I already reviewed it myself from the diffs on github 2x already. Everything looks correct. The dependency libraries were converted to support .NET Core 2.0 and .NET Framework 4.7.1 with strong name signed assemblies. I can only suggest that you download a copy of my forked version and test to make sure everything is working as I have done. There is nothing more I can do with this. It is what it is… and I was careful not to change any of the functionality along the way outside of using modern constructs for the new compiler version.

…

-Latency From: Paweł Gajdziński [mailto:notifications@github.com] Sent: Friday, January 12, 2018 1:51 AM To: HearthSim/Hearthstone-Deck-Tracker <Hearthstone-Deck-Tracker@noreply.github.com> Cc: Latency McLaughlin <latency@bio-hazard.us>; Author <author@noreply.github.com> Subject: Re: [HearthSim/Hearthstone-Deck-Tracker] Build configuration migration .net 4.7.1 & .net core 2.0 (#3535) Dude, it's too big for just single PR. Nobody will review it. Firstly I recommend you to: 1. Create new branch which is uptodate with Hearthstone-Deck-Tracker/master 2. Push there only ResourceGenerator changes 3. Create PR. :) After you can create PR which only contain changes CRLF -> LF, etc. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3535 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AAwHSKpUDg6kk1KLLhgmyUIuA3i5cMFkks5tJxzngaJpZM4Rb72E> .

[azeier]

I can only suggest that you download a copy of my forked version and test to make sure everything is working as I have done.

That's not a reasonable way to make sure this PR is fine. Me being busy for two weeks doesn't mean reviewing a +-3k diff isn't crazy. Not only that not checking what possibly non obvious implications all of the changes might have.

This should at least be split into PRs for:

• Cleanup
• .NET Core (see below - I'm not sure why this is necessary)
• .NET Framework 4.7 (see below)

This allows for much easier selective merging. It's okay for PRs to depends on one another.

There are also still what looks like to me line ending changes that result in major unnecessary diffs in a lot of the files. Also the CHANGELOG is user facing and should not contain technical changes.

Two major questions I have:

• What happens when HDT updates to the new version but the user only has .NET Framework 4.5 installed?
• What are the advantages of .NET Core besides cross platform (which are not relevant in a WPF project)?

[Latency]

Hello Alex, Thank you for getting back to me! Yes, I understand you are busy. I acknowledged that the last time I had heard from you. In regards to the PR, I have gone ahead and branched a copy yesterday and went over every diff within it 2x already over the past few weeks. It looks correct. The build configurations have completely changed. All of these changes are updates to the libraries and the outdated framework versioning. To build against assemblies not x-targeted for .NET Core or STD simply won’t work. I have repacked the dependency assemblies to make this happen and it is strong named signed. You can read more about the advantages and improvements here: https://blogs.msdn.microsoft.com/cesardelatorre/2016/06/27/net-core-1-0-net-framework-xamarin-the-whatand-when-to-use-it/#why-dotnet-core There still are a few reasons for strong naming an assembly, and if you need to, there are new options for reducing the pain, including (OSS signing and automatic binding redirects) In regards to updated to .NET Core.. means that this project will only ever be allowed to run on Windows. The GUI library support is not a feature set of CORE yet.. which is why Xamarin still exists. But it will be around the corner.. hence the reason why the msbuild configuration to CLP was not possible after spending efforts to try and get this to work. It will crap out on the Xaml stuff. Also, there are some problems with the AutoGenerateBindingRedirects. About 4 of the xaml files that specifically used the MahApps library I couldn’t get to work with the new build configuration. Hence, I have had to settle for one that is in-between what is currently being used today at the ones found in the dependency assemblies I have provided in my release. You should be able to see the differences in the layout and changes with the build-tools in the UI under the References section. I spent days on trying to make this happen.. and I spent at least 3 weeks in the changes relating to the PR. I am volunteering my efforts on this for the betterment of the project and the community at no cost. This should answer your last two questions. It doesn’t matter one way or another if the current project is in .NET 4.5. In fact, it requires that I install those RT’s just to get the project to work anyway. One reasons is because outdated versions of .NET Framework do not come out of the box installed in Windows 10. You can add these separate but it comes with v4.6-v4.6.2. The new version v1709 is said to run on 4.7. So, why is it that if I need to use a framework version that has about 10x less features and fixes to that of the latest one, that I wouldn’t just install the most recent version? Since I have to install 4.5 anyway, why not just install the latest one instead just to get the project to work? Many projects now have updated their libraries to target against framework versions greater than >= v4.5. This means that you will NOT be able to use them until you update the framework version of your project to match. Also, during my PR, I had went ahead and updated the NuGet package versions just because it is a 1-click easy to do thing and make sure everything is up-to-date. Whether it is required or not, is a matter of debate. Internal issues may be present within these 3rd party assemblies that you and I are obviously unaware of without inspection. This could be one of the reasons for why there is an update being provided to begin with. Without changing major build versions, semantics should be the same and if not, suddle ones at best. I have already made sure after doing this, that the project compiles so that validates any discrepancies to those concerns. If you have any additional questions, please don’t hesitate to ask. There is much to learn with this project and I have many questions I wish to know in other sections of the project as well.

[Latency]

It was recommended that I try to split this PR into 2-4 smaller ones.
I have since done this for :

1. ResourceGenerator
2. Build configuration migration
3. Hotpoint / NPE cleanup
4. Namespace refactoring to match the new build configurations are still left in this one here.

In fact, all 4x issues are bundled into this PR as final release on my fork in (master).

[antonfirsov]

@Latency

WORKING!!! TESTED!!!

This is not how OSS collaboration works. Project maintainers should be able to prove that it's also "WORKING!!!" for 100-s of other users using the project. The safest way do so is to break down your changes to smaller PR-s.

[azeier]

From what I could find out so far: It looks like Squirrel (our updater) is not able to upgrade .NET installations on update. Which means updating HDT to any newer .NET Framework version will not be possible. In addition to that, 15% of the HDT userbase are using the zipped version, which comes with it's own updater, that would also require a custom implementation.

There are possible workarounds like pushing an HDT update that installs the required .NET Framework version and then downloads another update only that version can download. That is super nasty though and really not something I want to do.

On top of all that: I have over the last two months been working on pulling basically all the non-UI code into a separate library and rewriting it (currently still a private project). Which means a lot of the changes here would have to be ported.

It doesn't seem like upgrading .NET Framework and using .NET Core (which I'm still not sure what the benefits of would be) are worth it at this point. If we had concrete plans to make HDT cross platform this might be a different story, but that is not currently on our roadmap.

[Latency]

The build configurations have been considerably consolidated and updated completely to support the new format. It is not dependant upon .NET Core at all. In fact, I do not include the 'FrameworkVersion' to include Core in the main ASM. Dependency assemblies support both and they compile just fine and were easy to port since they are APIs which do not necessarily include XAML page files and resources.

Note, these build configurations are easier to use / add plugins / resources / NuGet packages / etc. and have better handling for binding redirects.

Updating the framework allows the project to be able to use 3rd party plugins / libraries that are compiled with targeted framework versions greater than what the project currently supports. Since my system does not have .NET 4.5 installed nor does it even come with Widows 10, would require that I have to install the RTs and SDK anyway.

Also, there are benefits to upgrading, as newer features and fixes are within the framework itself. This can be illustrated as depicted in my fork; specifically, for the NuGet packages being used in the ResourceGenerator v2 upgrade required as example.

As for squirrel, and everything else in the project, it should work the same and backwards compatible. I removed a few references / usings in which it wasn't being used in the file as code cleanup / analyzers depicted. Yet, these appeared to be identified in somebody else's commit as noticeable inference anyway.
I do know that I am getting the dialog window to appear OnLoad, prompting to do a code update, but I hadn't initiated it to test as of yet.

This PR looks rather large. I am thinking it has mostly to do with refactoring the namespaces to be in alignment with the folder hierarchy matching default build rules within the new build configuration now.

See also:

    <!-- App.xaml --> 
    <ApplicationDefinition Include="App.xaml" SubType="Designer" Generator="MSBuild:XamlIntelliSenseFileGenerator" />
	
    <!-- XAML elements -->
    <Page Include="**\*.xaml" SubType="Designer" Generator="MSBuild:Compile" Exclude="App.xaml" />
    <Compile Update="**\*.xaml.cs" SubType="Designer" DependentUpon="%(Filename)" />
	
    <!-- Resources -->
    <Resource Include="Images\**\*.ico" />
    <Resource Include="Images\**\*.png" />
    <Resource Include="Resources\**\*.png" />
    <Resource Include="Resources\**\*.jpg" />
    <Resource Include="Resources\Chunkfive.otf" />

Globbing has been restored to nest the designer file under the page.
I am not totally sold on intellisense working in XAML with this.
I believe that the Generator should change from Generator="MSBuild:Compile" to Generator="MSBuild:XamlIntelliSenseFileGenerator"
If anybody wants to confirm this & resolve... that would be great!

If you go on Gitub/dotnet and read the experimental workarounds for support we are trying to add with the new build configurations and compiler, you can see it is rather challenging. XAML is not a feature of .NET Core yet and using new build configurations that potentially can target ALL frameworks has its qwirks.

First of all, default build rules are now internal. Targets for pre-build / post-build & its synopsis have changed and specific concerns relating to CLP as illustrated here.

Setting the XAML pages to be Included (instead of Updated) && enabled within the VS IDE, disable the file completely. This breaks the designer entirely and will not get the project to load properly as discovered. It took me several attempts and weeks to figure out how to get this to work properly and extensive research all over the internet as workaround.

In my previous PR, it addressed a compromise (medium) of this new release and the original one found within the project. But, I finally was able to get it working with the new build configuration this weekend.

Also, without having to use explicit rules to enable backwards compatibility support for old MSBuild rules and conventions requires that additional rules be added to handle the XAML files. By default, we can see rules added above to specifically override **\*.* - But this does not support globbing, so I have added a line to fix that.

To keep the build configuration small and tidy and the project namespaces consistent, refactoring should persist to preserve ordering and adhere to the new build rules.

Finally, the main UI assembly must be forced to compile as x86 to keep visibility to the dependency Process.Module assemblies that are required to load .. specifically "mono.dll" -
"Hearthmirror\ProcessView.cs" since "Hearthstone.exe" is targeting x86.

"You have to compile your program using the same architecture as the target-process. The target-process loaded modules that establish x86-support on x64-OS, hence you got to compile using x86-architecture in order to aquire x86-modules from the target-process." - Zat

Dependency assemblies can be targeted to AnyCPU, which will support both and SN'd now to add reference to the GAC or dependent assemblies that may require it in the future. Also, it is possible that migrations can be made to now 'pack' dependency project assemblies as NuGet packages and have bootstrap automation update the packages with NuGet package manager & restore. This will eliminate much of what the confusing batch files are doing for handling this today and make visibility & versioning easier to work with as shown by the elimination / modifications of particular batch files in my PRs.

[Latency]

See reference 3539