CnCNet / xna-cncnet-client

XNA / MonoGame based client for playing classic Command & Conquer games both online and offline with a CnCNet game spawner.
Other
223 stars 86 forks source link

Revert to .NET Framework #494

Closed Metadorius closed 4 months ago

Metadorius commented 8 months ago

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

SadPencil commented 8 months ago

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: https://github.com/CnCNet/dta-mg-client-launcher/pull/13

SadPencil commented 8 months ago

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

Several commitments of mine require additional check:

SadPencil commented 8 months ago

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 commented 7 months ago

Now the client runs successfully

SadPencil commented 7 months ago

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 commented 7 months ago

1b987c48b522bf3e98209d7c0e7b32502b79b2e5 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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago
SadPencil commented 6 months ago

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

SadPencil commented 5 months ago

TODO:

SadPencil commented 5 months ago

WindowsGL builds (both for .NET 4.8 and .NET 8) fails because of this commit: https://github.com/Rampastring/Rampastring.XNAUI/commit/f32bcda575ed45aefcfabc19f426e5bc7e76e707

Fix: https://github.com/Rampastring/Rampastring.XNAUI/pull/29

SadPencil commented 5 months ago

Upstream dependencies prerequisites:

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 commented 4 months ago

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.