Skip to content

Revert to .NET Framework#13

Merged
SadPencil merged 6 commits intoCnCNet:masterfrom
SadPencil:revert-net7
Feb 25, 2024
Merged

Revert to .NET Framework#13
SadPencil merged 6 commits intoCnCNet:masterfrom
SadPencil:revert-net7

Conversation

@SadPencil
Copy link
Member

@SadPencil SadPencil commented Oct 19, 2023

This PR migrates the launcher to support .NET 4.8 + .NET 8 client.
The minimum .NET Framework requirement for the launcher is downgraded to 4.0.

@SadPencil SadPencil marked this pull request as draft February 6, 2024 09:03
@SadPencil
Copy link
Member Author

SadPencil commented Feb 6, 2024

TODO:

  • make the launcher be able to run .NET 4.8 clients as well as .NET 8 clients

@SadPencil SadPencil changed the title Revert .NET 7 migration Revert to .NET Framework Feb 20, 2024
@SadPencil SadPencil marked this pull request as ready for review February 20, 2024 14:16
<PropertyGroup>
<TargetFramework>net471</TargetFramework>
<LangVersion>11.0</LangVersion>
<TargetFrameworks>net40; net45; net471</TargetFrameworks>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm is there any reason for all those old frameworks and not just net48?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if it relies on 4.0, is there a need to compile anything else?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the launcher prompts users about missing frameworks. Win7 comes with net35 (due to the need of accessing registry in 32bit view, the launcher cannot be downgraded to that old), while Windows 10 only comes with net46.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any benefit in compiling multiple versions then? I think we could settle on one version that fits the most and leave it at that.

Copy link
Member Author

@SadPencil SadPencil Feb 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.net 4.0 does have limitations, for example the launcher can not differ amd64 from arm64. This is brought by net45. So that's why I reserved other two frameworks versions in case we need to upgrade the framework for some reasons, for example if Qualcomm’s processors would being popular in laptops someday

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't care about win8 users. Just wondering if there are some win7 users that only have .net 4.0 installed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know only that net40 came as an update to Win7. Also Win8 has net45 bundled.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way, I am fine with either.

Copy link
Member Author

@SadPencil SadPencil Feb 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remembered wrong. arm64 detection was supported on net471. So we should leave net40 as the default which is already done in GitHub CI

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am in favor of <TargetFrameworks>net40; net471</TargetFrameworks> over <TargetFramework>net40</TargetFramework> in case someone needs it for ARM64. The GitHub CI has been configured to compile net40 only, providing binary for downloads in Release

@SadPencil SadPencil merged commit b9db743 into CnCNet:master Feb 25, 2024
@SadPencil SadPencil deleted the revert-net7 branch March 13, 2024 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants