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

Upgrade to Godot 4.4 #7

Closed
wants to merge 1 commit into from

Conversation

Joy-less
Copy link

@Joy-less Joy-less commented Mar 7, 2025

This will make sure all of the APIs are up-to-date (since Godot's API gets several breaking changes) and also removes the net6.0 and net7.0 targets which are no longer supported by Microsoft.

@Delsin-Yu
Copy link
Owner

This will make sure all of the APIs are up-to-date

This is not how NuGet works. It will select the best version based on what you already use. (instead of falling back to the minimum version).

Under GodotSharp use cases, the Godot.Net.Sdk already forces the GodotSharp package versions for you, so the GDTask will automatically use that newer version.

See this example: My project is using Godot 4.4, and I'm referencing the GDTask package. What I get is the GodotSharp version 4.4 instead of the 4.1 defined in the package here.

image

...and also removes the net6.0 and net7.0 targets which are no longer supported by Microsoft.

I wish to maintain the best compatibility for this package, so I do not intend to drop support for earlier DotNet and GodotSharp versions.

since Godot's API gets several breaking changes

This is incorrect; the Godot API, starting from 4.0, does not have breaking changes, all the API changes are additive not replacement, and even so, I'm sure GDTask is using none of the 'legacy APIs', as my in-house project, based on a customized version of Godot 4.4, with compat-api disabled, can also compile and work.

The behavior you are referring to is using the GDTask from a blank C# project, resulting in NuGet selecting the minimum version for you. However, the GodotSharp API won't work anyway under such circumstances.

@Delsin-Yu Delsin-Yu closed this Mar 7, 2025
@Delsin-Yu Delsin-Yu requested review from Delsin-Yu and removed request for Delsin-Yu March 7, 2025 23:29
@Joy-less
Copy link
Author

Joy-less commented Mar 7, 2025

This is incorrect; the Godot API, starting from 4.0, does not have breaking changes, all the API changes are additive not replacement

This is not correct. In fact one of my libraries (Godot Cookies) had to update to Godot 4.4 due to a breaking change with FileAccess.

This is not how NuGet works. It will select the best version based on what you already use. (instead of falling back to the minimum version).

You are correct that NuGet will attempt to use the latest version, but it will not recompile the library, meaning it could be missing out on overloads that bring better performance e.g. with Godot's new span overloads.

I wish to maintain the best compatibility for this package, so I do not intend to drop support for earlier DotNet and GodotSharp versions.

This really doesn't make sense. Both Microsoft and Godot Foundation have dropped support for earlier DotNet and GodotSharp versions. If someone wants to stick to Godot 4.1-4.3, then they could continue using an older version of the GDTask package.

@Delsin-Yu
Copy link
Owner

one of my libraries (Godot Cookies) had to update to Godot 4.4 due to a breaking change with FileAccess.

That sounds like a bug in the engine that needs to be addressed.

meaning it could be missing out on overloads that bring better performance e.g. with Godot's new span overloads.

I am the ROS PR author, and I don't recall the GDTask using any of them.

... and Godot Foundation have dropped support for earlier DotNet and GodotSharp versions

No, Godot has 4.1.4 and 4.2.2 as the maintenance releases for the previous versions, so the support for earlier versions has not dropped.

@Joy-less
Copy link
Author

Joy-less commented Mar 8, 2025

That sounds like a bug in the engine that needs to be addressed.

Here is the change that broke compatibility:
godotengine/godot#78289
Note that the breaking change was raised and discussed.

No, Godot has 4.1.4 and 4.2.2 as the maintenance releases for the previous versions, so the support for earlier versions has not dropped.

Okay, can we at least drop support for unsupported .NET versions? Godot 4.1 can be used with .NET 8.0 and .NET 9.0.

@Delsin-Yu
Copy link
Owner

Godot 4.1 can be used with .NET 8.0 and .NET 9.0.

But the minimum supported dotnet version for those is 6.0, We should be consistent with what's supported, also, there isn't any pain on supporting net6 and 7.

@Joy-less
Copy link
Author

Joy-less commented Mar 8, 2025

Godot 4.1 can be used with .NET 8.0 and .NET 9.0.

But the minimum supported dotnet version for those is 6.0, We should be consistent with what's supported, also, there isn't any pain on supporting net6 and 7.

I don't agree... .NET 6.0 and .NET 7.0 are massively lagging behind in APIs, plus they are no longer installed on many peoples' PCs.

@Delsin-Yu
Copy link
Owner

Delsin-Yu commented Mar 8, 2025

I don't see the connections here. My point is to stay consistent with Godot Supported, not about how many people are using them.

And a side note: we are already publishing the package with multi-targeting, which means the user always gets the most suitable dll compiled with corresponding versions of dotnet BCL. For example, a user who targets net9 will get the GDTask that's compiled targeting net9, etc.

What you want from me is to delete what we are supporting. This is not acceptable on my side.

Personally, I don't want to push people to the newer versions by manually dropping support for what could total be possible; I have products that target net6.0 for support on Windows7, so upgrade to newer dotnet version is no an option.

@Delsin-Yu
Copy link
Owner

plus they are no longer installed on many peoples' PCs.

Most, if not all, Godot .Net Games are delivered in self-contained mode, so whether the end user has dotnet runtime installed or not is irrelevant; I just want the GDTask to work with more of the targets possible, not all developers are willing to migrate just for a package.

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.

2 participants