Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C#: Allow use of .NET 7 #71825

Merged
merged 1 commit into from Jan 26, 2023
Merged

Conversation

RedworkDE
Copy link
Member

@RedworkDE RedworkDE commented Jan 21, 2023

Unlike #71241 this does not change the target framework the godot assemblies, instead it just allows the godot editor to run using the latest .NET version installed on the machine (including .NET 7).

The change in InteropStructs.cs only exists to work around dotnet/runtime#80624. It does not actually change anything, but otherwise if the latest installed .NET version is 7.0.0 or 7.0.2 godot would crash 1.

Behavior with different .net version installed:

action none .net6 .net7 .net6 and .net7
running the editor on master crash uses .net6 crash uses .net6
running the editor on PR crash uses .net6 uses .net7 uses .net7
exporting project targeting .net6 on master n/a uses .net6 n/a uses .net6
exporting project targeting .net6 on PR n/a uses .net6 uses .net6 2 uses .net6
exporting project targeting .net7 on master n/a build fails n/a build fails
exporting project targeting .net7 on PR n/a build fails uses .net7 uses .net7

Running the exported project does not depend on the installed .net versions and always runs on the target version specified in the the csproj.

I think this is generally the behavior on would expect, except that .net6 projects may run under .net7 inside the editor, but that should generally not change anything and if it does cause issues for a project can be worked around by uninstalling .net7.

Test projects that shows a message box with the current running .net version: Net7.zip

Probably makes #68652 and maybe #70129 go away, although there the underlying issue seems to be a .NET 6 installation that got corrupted in the process of installing .NET 7 and not actually something that can really be fixed.


For users this PR means that they can use the godot editor (mono version) without crashing at startup if only .NET 7 is installed on their machine, and if they have .NET 7 installed they can change the TargetFramework in their .csproj to net7.0 and start using .NET 7 and C# 11 features in their code.

Footnotes

  1. 7.0.1 does not exist and 7.0.4, the version that likely contains the fix is not released yet.

  2. I was concerned about this case, but somehow the exported projects correctly runs using .net6 even tho that version is not installed anywhere

The editor will use .NET 7 if it is installed and fall back to .NET 6 otherwise.
Exported projects will use .NET 7 or .NET 6 depending on the value of TargetFramework in the csproj.
@RedworkDE RedworkDE requested a review from a team as a code owner January 21, 2023 22:30
@raulsntos raulsntos added this to the 4.0 milestone Jan 22, 2023
@raulsntos
Copy link
Member

About exporting projects targeting .NET 7 with only .NET 6 installed, I'd say that's expected to fail.

@@ -8,6 +8,7 @@

<!-- To generate the .runtimeconfig.json file-->
<EnableDynamicLoading>true</EnableDynamicLoading>
<RollForward>LatestMajor</RollForward>
Copy link
Member

@raulsntos raulsntos Jan 22, 2023

Choose a reason for hiding this comment

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

Have you tried using Major instead? AFAIK that should prefer the .NET 6 SDK when both are installed and may be safer than using a newer major version that may include breaking changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but that means you can't use projects targeting .net7 in the editor if both .net6 and .net7 are installed on a machine, but after uninstalling .net6 it will then work, which is IMO really unintuitive.

Copy link
Member

@raulsntos raulsntos Jan 22, 2023

Choose a reason for hiding this comment

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

🤔 Oh yeah you are right, I agree it's unintuitive.

Copy link
Member

Choose a reason for hiding this comment

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

We talked about this the other day. The problem with Major is that (assuming both net6 and net7 are installed) if GodotPlugins targets net6, then that's what the Godot editor will load. If that happens, there will be problems if a game project targets net7, because only one runtime can be loaded.
As such, we decided LatestMajor was the way to go. The only drawback here is the risk of a future .NET version breaking backwards compatibility. I think this is a risk we will have to live with, but it would be great if we can have a workaround for users in case that ever happens. My idea was to add a project/editor setting to use Major if desired.

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 question really is how long term .NET 6 support is supposed to be. If the plan is to fully switch to .NET 7 as soon as all the kinks are worked out, I would just not bother with being able to use 6 over 7, when both are installed. If on the other hand .NET 6 is going to remain supported for the full 4.0 release, I would load the framework based on the runtime config of the user project and only fall back to the runtime config of GodotPlugins, if the project was not built yet / the desired target framework is not installed.

@RedworkDE
Copy link
Member Author

About exporting projects targeting .NET 7 with only .NET 6 installed, I'd say that's expected to fail.

Yeah, that is also what the table is supposed to say about this case? The header row is the .net versions installed on the machine for that column and the first row is the action to take.

The weird case with the footnote is exporting a project with target framework .net6 on a machine with only .net7 installed, which I would have expected to either fail (ok) or export using .net7 (possibly problematic) but the reality is that it works and the exported project correctly uses .net6 without that version being installed anywhere on the machine (and I did the .net7 only tests first on the VM, so .net6 had never even been installed in the past)

@neikeq
Copy link
Member

neikeq commented Jan 22, 2023

The weird case with the footnote is exporting a project with target framework .net6 on a machine with only .net7 installed, which I would have expected to either fail (ok) or export using .net7 (possibly problematic) but the reality is that it works and the exported project correctly uses .net6 without that version being installed anywhere on the machine (and I did the .net7 only tests first on the VM, so .net6 had never even been installed in the past)

Hmm, perhaps there's some sort of backward compatibility and it downloads the dependencies from NuGet?

@RedworkDE
Copy link
Member Author

Hmm, perhaps there's some sort of backward compatibility and it downloads the dependencies from NuGet?

Can confirm that this is indeed where the runtime comes from; same thing also happens for cross platform builds.

@akien-mga akien-mga merged commit 2edef2a into godotengine:master Jan 26, 2023
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants