C#: Upgrade the platform-specific TargetFramework version automatically

[shana]

Our minumum TargetFramework is net6.0, but for Android is net7.0, and for iOS is net8.0. By default, existing projects will be broken for these platforms, and the csproj files need to be manually changed by users in order to make things work.

When we generate a new csproj, we inject additional TargetFramework properties with a condition matching each platform, so we don't break our current requirements, and things still work for users trying these platforms out. We can do the same for existing projects, by injecting or updating these additional properties, if needed.

This PR finds all TargetFramework properties and, per platform, checks whether any of them is applicable to the platform. If no TargetFramework properties are usable, it adds a property with a platform check condition and the required version.

If the default TargetFramework is already set to a high enough value, no properties are injected.

If existing properties that match our platform condition (ones we've injected before) have an unusable version, they get updated.

Users can skip the automatic project update by setting a GodotSkipProjectUpdate property to true/1.

See https://github.com/godotengine/godot/pull/82762/files#r1345098244 and #82729 (comment) for more context

[mhilbrunner]

@raulsntos What is the status of this one? is this still wanted for 4.3/later/at all?

[raulsntos]

I think this is still wanted although I'm not sure it'll make it to 4.3. I may end up moving it to 4.4.

---

It doesn't seem to work in some cases. For example when the TFM is net7.0 and there's a conditional TFM for Android set to net6.0 it doesn't remove it, even though the minimum for Android should be net7.0.

[raulsntos]

I feel like if the logic for upgrading TFMs works really well I'd rather not have the GodotSkipProjectUpdate property. Also, with that name it feels like this should also affect the SDK upgrade.

So I'm thinking maybe we can remove this for now, and see if there's user demand later.

[shana]

I need this 😄 I've renamed it to something longer and more explicit of what it means.

[raulsntos]

I'm still a bit hesitant to adding this property. Can you describe your use case? I'm also not sure if it should be an MSBuild property or a ProjectSetting/EditorSetting, it feels weird for it to be an MSBuild property to be honest since it doesn't affect the build.

I was hoping we could merge this for 4.4.dev5, and this is the only blocker for me, so I was wondering if it'd be acceptable to you to extract this part to a separate PR so we can give it more time to discuss, without delaying the rest of the PR.

[shana]

I don't understand what the problem is of having a property that allows an advanced user to bypass the automatic rewriting of the csproj? I'm having to maintain custom .net SDKs for consoles, and the automatic rewriting is not great when I'm testing or when I'm needing to change the settings for specific use cases. Why would having this option be a blocker for you? It is not documented, very obvious about what it does, and it's only for advanced use cases. I would really like to not have to maintain custom patches to be able to bypass certain things, it causes a ton of conflicts every time we merge upstream godot into our forks.

[Delsin-Yu]

You certainly have your point; however, shouldn't the necessity for this feature be discussed and implemented in a separate PR? Even if this feature is not merged upstream, you can keep it in your local fork to enhance your merging experience.

Personally, I would cherry-pick the GodotSkipProjectUpdate part for my fork anyway, as I think it helps my use case.

[shana]

Keeping it in the fork is precisely the thing that causes no end of conflicts whenever we merge upstream to our fork, and precisely what I want to avoid. Taking away a user option and making users keep local patches in order to have control over things is contrary to a good user experience.

[raulsntos]

Every feature that gets added to the engine needs to be thoroughly discussed. And being undocumented won't prevent it from being used, which means users may start relying on it and complain later if we want to rename it, move it somewhere else, etc.

The rewriting of the csproj would happen anyway even with GodotSkipAutomaticTargetFrameworkUpdate because Godot will change the Sdk to Godot.NET.Sdk/{Version}. Does this not affect you?

Also, I don't understand why this would affect consoles if this PR was originally only supposed to update the TFM for Android and iOS platforms. And the TFM shouldn't change if it satisfies the minimum requirements anyway, are you saying consoles are using a .NET version older than the minimum requirement set by Godot?

[shana]

Given that you mentioned above that you would like to enforce the minimum required version for all platforms, then yes that affects all platforms, not just consoles. Rewriting the godot sdk net version doesn't affect consoles as much as this, and yes, consoles can follow a different version than the rest of the platforms. And given that the current target framework version is 6, not 8, and that this PR is on top of the current 4.3 head, not on top of your net8 update, yes, different platforms will have different requirements, the consoles will need to have updated requirements on these (a local patch I'm not keen on maintaining but can't go around that one), and having the ability to control whether these properties get automatically changed is important.

This is an integral part of this PR, it is not an optional thing to be discussed in a committee somewhere. If you prefer it documented, I will gladly document it. If you refuse to take it as part of this PR, I will withdraw this PR entirely, as well as the consent for it being used. I hope that clarifies my position on this.

[raulsntos]

I understand that console platforms may have different requirements. In 4.3, the minimum required TFM is net6.0 but Android requires net7.0 and iOS requires net8.0. To handle this we add additional TargetFramework properties with a condition to match the GodotTargetPlatform, I don't see why consoles can't do the same.

This PR would not touch those additional TargetFramework properties, it should only affect the ones that have no conditions or the ones that have a condition that matches the pattern $(GodotTargetPlatform)='{platform}' where {platform} is one of the hardcoded platforms (currently only android and ios).

[raulsntos]

@shana Would you be able to continue working on this? We want to merge #92131 which makes .NET 8.0 the new minimum required version, so it'd be nice to automatically fix user projects to target net8.0.

[shana]

@raulsntos I can take a look over the weekend.

[shana]

@raulsntos I've rebased and refactored and added comments on what it does, should do the trick now.

[raulsntos]

Thanks! I think it's looking great. Just a few comments.

[raulsntos]

Is there a way to add a minimum TargetFramework for all platforms? For example:

new PlatformFramework { Platform = "", TargetFramework = "net8.0" }

Hoping that we can reuse this to update all platforms to net8.0 when we move the GodotSharp packages to .NET 8.0.

[shana]

What's the current minimum target framework, is it 6 or 7?

[shana]

Adding a minimum non-platform-specific target framework requires more code and some refactoring of the logic, so that's going to need more testing. I've pushed a change to do that, I'll test today and see if it's working properly or not.

[Delsin-Yu]

What's the current minimum target framework, is it 6 or 7?

Today's minimum is 6, and #92131, which depends on this PR, will raise it to 8.

[shana]

Ok, I'm done with this. This is not the first or second or X time that I've had a PR nitpicked to death, where things that I consider important, and that I provide valid user cases for, are dismissed with "this isn't useful for me therefore it's useless", with no serious analysis or professional feedback.

Deciding on whether features are important or not requires actual arguments based on user cases, pros and cons of the worth of the features vs the cost of maintaining the feature, not personal opinions based on feels, personal anedoctes, individual needs. Designing good software and good experiences requires imagining how others might use it, what they might need, which may be, and often is, a completely different experience from the person designing it. It is a challenge that requires empathy, and respecting experiences and needs that may be foreign to the software designer.

What we have here in this PR, and almost every PR I put up that looks to improve the user experience, is not good feedback, it's not valid feedback - it's grammar nitpicking, it's feels projections, it's gatekeeping, and I am absolutely done with this. It brings me stress, and apparently I shoot myself in the foot by having to maintain local patches to make the features that I am contributing actually work for me and my users, because "this isn't useful for me, therefore it's useless" is the default response around here.