Revert to .NET Framework

[Metadorius]

Description

.NET 7 provides considerably worse user experience for average desktop user over .NET Framework and brings many issues for distribution .NET Framework 4.8 is still going to be supported for a good while and supports most modern tech from C# 11 because of .NET Standard 2.0 support and many backports, and because the changes are not tremendous to go back to .NET Framework it was decided to do so.

The proposed way

Currently I suggest to take the current develop and with power of polyfills, backports and git reverts where there's runtime/API incompatibility (like default interface implementations) make the current version work on .NET Framework 4.8. The reason is there is a lot of changes since the revert and reapplying them one by one would be, in my opinion, a great headache with conflicts and such, and current code is pretty stable from our limited testing with @Starkku in Project Phantom closed beta.

TODO

• Decide if we keep UniversalGL/.NET 7 target for native Linux compatibility or ditch it completely in favor of simplicity
• Polyfill where needed (or revert if not possible) the .NET 4.8 incompatible code from client
  • Same thing in updater, see Revert to .NET Framework cncnet-client-updater#17
  • Same thing in launcher, see Revert to .NET Framework dta-mg-client-launcher#13
• Sort out the polyfills/backports to use (some overview), I initially wanted to use Polyfill but PolySharp seems to work better? Currently some projects use one or another
• Revert to the old built client file structure (exes in resources, updater name) where needed/applicable to lessen the burden of upgrading for game/mod maintainers

[SadPencil]

Polyfill where needed (or revert if not possible) the .NET 4.8 incompatible code from client: Same thing in launcher, see [link pending]

The launcher has not introduced any .NET 4.8 incompatible codes for now. Is introducing Polyfill necessary? See: CnCNet/dta-mg-client-launcher#13

[SadPencil]

Update progress. Now, the client can be successfully built in .NET 4.8.

Several commitments of mine require additional check:

• d7fd5dc introduces a temp version of CnCNet.ClientUpdater of commit 2e53286. After the updater be released, remember to override CnCNet.ClientUpdater.1.0.16.(s)nupkg files. Also, delete C:\Users\<username>\.nuget\packages\cncnet.clientupdater\1.0.16 folder for those having worked on this branch.
• 9a1ab74 applies a workaround for Polyfill. Further investigation should be proceed with. See this comment for more details. --- Polyfill's author seems not interested in this issue, while it is more likely an MSBuild-related issue, therefore making a workaround 9a1ab74 for it is acceptable.
• 3d9033d stops copying Newtonsoft.Json.Bson.dll. It seems this file is unused. Keeping this line causes a file-not-found error triggered by AfterPublish.targets. This file is introduced in b221a3a without comments, while in the previous version b0304ea this file does not exist. --- Now (6cb7e75), the build script will only process this file if it exists, so this line can be considered obsolete.

[SadPencil]

Decide if we keep UniversalGL/.NET 7 target for native Linux compatibility or ditch it completely in favor of simplicity

In my opinion, .NET 7 target can be preserved, as #if !NETFRAMEWORK is only used for forth time.

[SadPencil]

Now the client runs successfully

[SadPencil]

Sort out the polyfills/backports to use (some overview), I initially wanted to use Polyfill but PolySharp seems to work better? Currently some projects use one or another

Definitely Polyfill. Switching to PolySharp brings additional compile errors, while Polyfill works. Although Polyfill has missing Index/Range issue but it should be an MSBuild-related issue. Therefore, making a workaround 9a1ab74 for it is acceptable.

[SadPencil]

1b987c4 Changing assembly loading priority is needed so that people could just override the binaries folder without deleting it first.

Explain in example: ClientCore.dll used to be a common dll, but not anymore at the time when the client supported both .NET 6 and .NET 4.8. If someone upgraded his client without deleting "Binaries\ClientCore.dll" first, the client fails to launch.

This commit changes the priority so even if "Binaries\ClientCore.dll" is not deleted, the client should be able to launch as it finds the correct "Binaries\Windows\ClientCore.dll".

Possible con: If a dll was a specific one and become a common one now, user must delete the old dll file.

[Belonit]

All assemblies except UniversalGL work fine at a quick glance. Below are logs of an attempt to launch UniversalGL on Windows and Linux
ClientCrashLog_linux.txt
ClientCrashLog_win.txt

[SadPencil]

All assemblies except UniversalGL work fine at a quick glance. Below are logs of an attempt to launch UniversalGL on Windows and Linux ClientCrashLog_linux.txt ClientCrashLog_win.txt

Should be fixed in f6acb03

[SadPencil]

• update CalculateHashes()

[SadPencil]

Thanks for the help. However, the latest commit a2bbf5a brings some changes which, in my opinion, requires further discussions:

• This commit removes GitVersion and on the other hand, relies on GitHub CI for the versioning. This brings an issue that apart from the CI, if modders compile the client via Build-All.ps1 by their hands, the version would be 0.0.0 as indicated by this file. I think GitHub CI is a helper rather than a necessary component. It would be harder to distribute the client in a testing version as the CI will not cover this.
• This commit replaces the NuGet package Polyfill with Meziantou.Polyfill without descriptions. Maybe we should revert this?
• This commit reverts 063c319, which brings out a problem that the dependency DLL files of .net 4.8 and .net 8 are different, despite that they have common file names. In my opinion it is better to keep the Binaries folder with .net 4.8 client files and move .net 8 files to another folder, otherwise we might meet a DLL hell in the future. Example crash: Revert to .NET Framework #494 (comment), which was fixed in f6acb03 and furtherly improved in 063c319
• This commit includes several #if NETFRAMEWORK which seems to be less necessary. In these cases, using the old .net framework codes should be just fine.
• This commit completely overrides 3db1443. new Startup().Execute() is not in a try-catch statement again. Whether this commit is correct or not requires further tests. Hint: make an incorrect INItializableWindows INI file to trigger an exception.
• In this commit, CnCNet.ClientUpdater is made a NuGet package. Further discussion are needed.
• This commit brings an issue in FileHashCalculator. Note that every player should get the same hash value if their client build and modding assets are the same. #if !NETFRAMEWORK in FileHashCalculator.cs might breaks the rule and remains further discussion.
• In PreStartup.cs file, ApplicationConfiguration.Initialize(); is called when #if NETFRAMEWORK does not hold. This is not accurate since ApplicationConfiguration was brought in .NET 6. #if NET6_0_OR_GREATER is overridden by this commit.
• using System.Linq brings a lot of handy extension methods. However, in Program.cs file, this line is moved into an #if NETFRAMEWORK section.
• This commit removes the HighDPI related things in manifest file. Although the app.config file specifies these settings, I need to stress that a great number of modders might think .config files are useless and therefore delete them! (Think about those mods that requires .NET 3.5 just because modders have deleted the .exe.config file alongside with the launcher executable, for example, Mental Omega) This is why we should preserving both the so-called "legacy" way and the new method.
• This commit reverts 6cb7e75, where the latter commit makes Newtonsoft.Json.Bson.dll file optional, because in .NET framework build this file does not exist at all. Further investigation are required. Also this is exactly the reason why I am strongly opposing mixing DLLs in .NET 4 and .NET 8.
• GetOperatingSystemVersion() is re-implemented. Someone needs to check whether it is correct.
• This commit changes the CI yaml files so that versions of some dependencies are marked as latest instead of their current latest version name. This is a dangerous behavior as it will prevent historical commits to be built in the future -- who would know what "latest" version would be in the past? Such version information should not be replaced with "latest".

[SadPencil]

TODO:

• figure out which dlls should be shared; double check dll dependencies
• WindowsGL build breaks on my machine. figure out why
• make PRs to target rampa's two libraries for .NET 8
• sometimes the build script fails because of "Rampastring.XNAUI..Debug" package is missing. However, all parameters have been explicitly given. This issue only shows up randomly, and will disappear if the script is run for a second time. figure out why --- update: it is probably due to a bad compilation error or something. Compiling WindowsXNA build has a change to trigger it

[SadPencil]

WindowsGL builds (both for .NET 4.8 and .NET 8) fails because of this commit: Rampastring/Rampastring.XNAUI@f32bcda

Fix: Rampastring/Rampastring.XNAUI#29

[SadPencil]

Upstream dependencies prerequisites:

• Target net 8 and netstandard Rampastring/Rampastring.Tools#11
• gameForm could be null in WindowsGL build Rampastring/Rampastring.XNAUI#29
• Target net8 with some cleanups Rampastring/Rampastring.XNAUI#30
• And also Revert to .NET Framework cncnet-client-updater#17, of course

After these pull requests are merged and new packages are released, those "*.(s)nupkg" files acting as temporary files can be removed from this branch

[SadPencil]

All works are done. We could either wait for upstream dependencies updates or just merge this PR before the dependencies updates (temporarily preserving those NuGet packages files in References directory like we did in the past).

Ready for review.

[alexlambson]

I don't see anything I had an issue with. May be worth waiting for approval from someone with stronger C# experience if you want style feedback though.

[Metadorius]

Outstanding work, although as the OG author of the PR I can't approve that. I have no complaints apart from the one I left.

[Metadorius]

I think should add the explanation for this and a cref to the original method to a docstring so that if someone updates one of the methods the second would need to be updated too, and a comment in the second place backreferencing that one.