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

Godot.NET.Sdk/3.2.4 - Fix targeting .NETFramework with .NET 5 #44135

Merged
merged 4 commits into from
Dec 8, 2020

Conversation

neikeq
Copy link
Contributor

@neikeq neikeq commented Dec 6, 2020

Fixes #43717

Also allow game projects to use a Godot.NET.Sdk with a newer patch version, e.g.: a Godot 3.2.4 project can benefit from fixes in a hypothetical 3.2.5 version of Godot.NET.Sdk (newer patch version needs to be set manually though).

@neikeq neikeq added this to the 3.2 milestone Dec 6, 2020
@neikeq neikeq force-pushed the godot-net-sdk-fixes-n-impr branch from a39bba1 to b86491c Compare December 6, 2020 00:10
Allow game projects to use a Godot.NET.Sdk with a newer patch version.
The major and minor version are still required to be the same.

For example: Allow a Godot 3.2.4 C# project to use a hypothetical
3.2.5 version of Godot.NET.Sdk.
Our target was overriding the official one, while not doing its job
assuming it was already taken care of...
The property has no effect as the older VS project system doesn't
work with Sdk style projects.

The presence of the property was preventing Visual Studio for Mac
from opening the project if the Godot extension is not installed.
@neikeq neikeq force-pushed the godot-net-sdk-fixes-n-impr branch from b86491c to a07589e Compare December 6, 2020 00:12
@akien-mga akien-mga merged commit 0b5a9bf into godotengine:3.2 Dec 8, 2020
@akien-mga
Copy link
Member

Thanks!

@@ -2,8 +2,6 @@
<PropertyGroup>
<!-- Determines if we should import Microsoft.NET.Sdk, if it wasn't already imported. -->
<GodotSdkImportsMicrosoftNetSdk Condition=" '$(UsingMicrosoftNETSdk)' != 'true' ">true</GodotSdkImportsMicrosoftNetSdk>

<GodotProjectTypeGuid>{8F3E2DF0-C35C-4265-82FC-BEA011F4A7ED}</GodotProjectTypeGuid>
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey! Why have you removed GodotProjectTypeGuid?
Rider UnitTesting feature uses it for special casing Godot project.
https://github.com/JetBrains/godot-support/blob/f6d1349dfad81fc127a53e7c0671d95c6752770c/resharper/src/UnitTesting/GodotRunHostProvider.cs#L50

Copy link
Contributor

Choose a reason for hiding this comment

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

I was able to fix it by adding ProjectTypeGuids manually.
van800/godot-demo-projects@03594a8

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We used ProjectTypeGuids for the Visual Studio project system, but it no longer has any effect after we switched to Sdk style csproj. The property prevents Visual Studio for Mac from opening the project unless the Godot extension is installed, which I consider a problem, so it was removed. Could Rider do something different for special casing Godot projects? If not I'm willing to add it back (and maybe it works BuildingByReSharper condition?).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll see, what I can do, thank you for clarification.

@akien-mga akien-mga modified the milestones: 3.2, 3.3 Apr 20, 2021
@neikeq neikeq deleted the godot-net-sdk-fixes-n-impr branch May 13, 2021 23:14
@31
Copy link
Contributor

31 commented May 19, 2021

A handful of people in the Godot C# Discord channel have seen this PR's title (paraphrased in https://godotengine.org/article/godot-3-3-has-arrived) and see it as declaring that Godot now supports <TargetFramework>net5.0</TargetFramework>. My understanding is that this PR fixes using the .NET 5 SDK as the build tool, but Godot's target framework support is unchanged.

Can you confirm what this PR ended up doing?
Could you consider updating the blog post to make it clearer what was changed here?

The problem is that net5.0 kind of works, but suddenly you can hit an API that doesn't work, or a NuGet package that doesn't work, because everything is still running on the "old" Mono Framework in the end. This can be troublesome to diagnose, especially when we basically need to argue against a blog post.

(That said, making Godot work with the .NET 5 SDK without workarounds is great, and much appreciated! And I'm really looking forward to godotengine/godot-proposals#2333 giving us a path forward to nice .NET 6 support. 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.

4 participants