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#: Switch games to MSBuild Sdks and .NET Standard #40595

Merged
merged 1 commit into from
Jul 26, 2020

Conversation

neikeq
Copy link
Contributor

@neikeq neikeq commented Jul 22, 2020

Godot.NET.Sdk

Godot uses its own custom MSBuild Sdk for game projects. This new Sdk adds its own functionality on top of 'Microsoft.NET.Sdk'.

The new Sdk is resolved from the NuGet package.

All the default boilerplate was moved from game projects to the Sdk. The default csproj for game project can now be as simple as:

<Project Sdk="Godot.NET.Sdk/4.0.0-dev1">
  <PropertyGroup>
    <TargetFramework>netstandard2.1</TargetFramework>
  </PropertyGroup>
</Project>

Source files are included by automatically so Godot no longer needs to keep the csproj in sync when creating new source files.

Closes godotengine/godot-proposals#315

Define constants

Godot defines a list of constants for conditional compilation. When exporting games, this list also included engine 'features' and platform 'bits'.
There were a few problems with that:

  • The 'features' constants were only defined when exporting games. Not when building the game for running in the editor player.
  • If the project was built externally by an IDE, the constants wouldn't be defined at all.

The new Sdk assigns default values to these constants when not built from the Godot editor, i.e.: when built from an IDE or from the command line. The default define constants are determined from the system MSBuild is running on.

However, it's not possible for MSBuild to determine the set of supported engine features.
It's also not possible to determine if a project is being built to run on a 32-bit or 64-bit Godot executable.

As such the 'features' and 'bits' constants had to be removed.
The benefit of checking those at compile time was questionable, and they can still be checked at runtime.

The new list of define constants includes:

  • GODOT
  • GODOT_<PLATFORM>
    Defaults to the platform MSBuild is running on.
  • TOOLS
    When building with the 'Debug' configuration (editor and editor player).
  • GODOT_REAL_T_IS_DOUBLE
    Not defined by default unless $(GodotRealTIsDouble) is overriden to be 'true'.

.NET Standard

The target framework of game projects was changed
to 'netstandard2.1'.

Closes godotengine/godot-proposals#339

@neikeq neikeq added this to the 4.0 milestone Jul 22, 2020
@neikeq neikeq requested a review from aaronfranke as a code owner July 22, 2020 12:24
@aaronfranke
Copy link
Member

$ mono --version
Mono JIT compiler version 6.10.0.104 (tarball Fri Jun 26 19:38:44 UTC 2020)

On an empty project, with this PR and the above Mono on Ubuntu 20.04, I get this error when building:

Project "NeikeqTesting.sln" (Build target(s)):
	Message: Building solution configuration "Debug|Any CPU".
	Project "NeikeqTesting.csproj" (default targets):
		MakeDir: Creating directory ".mono/temp/bin/Debug/".
		MakeDir: Creating directory ".mono/temp/obj/Debug/".
		/usr/lib/mono/msbuild/Current/bin/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(234,5): error NETSDK1004: Assets file '/home/aaronfranke/NeikeqTesting/.mono/temp/obj/project.assets.json' not found. Run a NuGet package restore to generate this file. [/home/aaronfranke/NeikeqTesting/NeikeqTesting.csproj]
	Done building project "NeikeqTesting.csproj" -- FAILED.
Done building project "NeikeqTesting.sln" -- FAILED.

Running nuget restore as suggested fixes it, but Godot should do this automatically.

Also, when building after restoring, I got this message:

ERROR: Cannot find dotnet CLI executable. Fallback to MSBuild from Mono.
   at: godot_icall_GD_pusherror (modules/mono/glue/gd_glue.cpp:251)

With both Mono and .NET Core 3.1 (3.1.302) installed, I get the same error, with a different MSBuild:

Project "NeikeqTestingButWithDotnetInstalled.sln" (Build target(s)):
	Message: Building solution configuration "Debug|Any CPU".
	Project "NeikeqTestingButWithDotnetInstalled.csproj" (default targets):
		MakeDir: Creating directory ".mono/temp/bin/Debug/".
		MakeDir: Creating directory ".mono/temp/obj/Debug/".
		/usr/share/dotnet/sdk/3.1.302/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(234,5): error NETSDK1004: Assets file '/home/aaronfranke/NeikeqTestingButWithDotnetInstalled/.mono/temp/obj/project.assets.json' not found. Run a NuGet package restore to generate this file. [/home/aaronfranke/NeikeqTestingButWithDotnetInstalled/NeikeqTestingButWithDotnetInstalled.csproj]
	Done building project "NeikeqTestingButWithDotnetInstalled.csproj" -- FAILED.
Done building project "NeikeqTestingButWithDotnetInstalled.sln" -- FAILED.

As before, running nuget restore as suggested fixes it, but Godot should do this automatically.

@neikeq
Copy link
Contributor Author

neikeq commented Jul 22, 2020

@aaronfranke There was a recent fix. Did you test with the commit 4a30289 included?

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

The new list of define constants includes:

Why not also include GODOT_PC, GODOT_MOBILE, and GODOT_WEB? These are essentially unions of operating systems, which we already define, so it should be easy to add these.

The architecture/bits and texture compression are of low importance, so it's fine to ditch them... and that covers everything!

modules/mono/csharp_script.cpp Outdated Show resolved Hide resolved
modules/mono/editor/GodotTools/GodotTools/BottomPanel.cs Outdated Show resolved Hide resolved
@aaronfranke
Copy link
Member

@neikeq If you rebased this PR, then that fix would be included :)

Godot.NET.Sdk
-------------

Godot uses its own custom MSBuild Sdk for game
projects. This new Sdk adds its own functionality
on top of 'Microsoft.NET.Sdk'.

The new Sdk is resolved from the NuGet package.

All the default boilerplate was moved from game
projects to the Sdk. The default csproj for
game project can now be as simple as:

```
<Project Sdk="Godot.NET.Sdk/4.0.0-dev2">
  <PropertyGroup>
    <TargetFramework>netstandard2.1</TargetFramework>
  </PropertyGroup>
</Project>
```

Source files are included by automatically so
Godot no longer needs to keep the csproj in sync
when creating new source files.

Define constants
----------------

Godot defines a list of constants for conditional
compilation. When exporting games, this list also
included engine 'features' and platform 'bits'.
There were a few problems with that:

- The 'features' constants were only defined when
  exporting games. Not when building the game for
  running in the editor player.
- If the project was built externally by an IDE,
  the constants wouldn't be defined at all.

The new Sdk assigns default values to these
constants when not built from the Godot editor,
i.e.: when built from an IDE or from the command
line. The default define constants are determined
from the system MSBuild is running on.

However, it's not possible for MSBuild to
determine the set of supported engine features.
It's also not possible to determine if a project
is being built to run on a 32-bit or 64-bit
Godot executable.

As such the 'features' and 'bits' constants had
to be removed.
The benefit of checking those at compile time
was questionable, and they can still be checked
at runtime.

The new list of define constants includes:

- GODOT
- GODOT_<PLATFORM>
  Defaults to the platform MSBuild is running on.
- GODOT_<PC/MOBILE/WEB>
- TOOLS
  When building with the 'Debug' configuration
  (editor and editor player).
- GODOT_REAL_T_IS_DOUBLE
  Not defined by default unless $(GodotRealTIsDouble)
  is overriden to be 'true'.

.NET Standard
-------------

The target framework of game projects was changed
to 'netstandard2.1'.
@neikeq neikeq force-pushed the godot-net-sdk-and-net-standard branch from 86a591e to ced77b1 Compare July 25, 2020 17:22
@neikeq
Copy link
Contributor Author

neikeq commented Jul 25, 2020

Re-based to latest master, added GODOT_PC/MOBILE/WEB define constants as suggested and made additional changes.

@aaronfranke
Copy link
Member

aaronfranke commented Jul 25, 2020

I still get this error message when building an empty project without .NET Core, though it still builds and runs.

modules/mono/glue/gd_glue.cpp:251 - Cannot find dotnet CLI executable. Fallback to MSBuild from Mono.

Does this mean that .NET Core is now required as a part of Godot moving to .NET Standard? So you need both Mono and .NET Core for Godot? If .NET Core is not required, then it should not throw an error. If it is required, that should be documented.

Also, should TOOLS be GODOT_TOOLS, or should it be left as TOOLS?

@neikeq
Copy link
Contributor Author

neikeq commented Jul 25, 2020

That's unrelated to this PR. That should probably be changed to a simple print rather than an error. It's printed when the selected build tool cannot be found (dotnet CLI is the default now) and Godot will try to fallback to a different one.

Also, should TOOLS be GODOT_TOOLS, or should it be left as TOOLS?

I decided to keep it TOOLS as it's commonly used constant when writing editor code. If anything, we may remove it later and move editor code into a separate csproj.

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

I don't understand every line of this but it works well and I have no further concerns.

@akien-mga akien-mga merged commit dc45605 into godotengine:master Jul 26, 2020
@akien-mga
Copy link
Member

Thanks!

@evelant
Copy link

evelant commented Jul 27, 2020

Is there a plan to backport this into the 3.x versions or would that be too difficult or not worth the effort? I've run into helpful nuget packages requiring netstandard2.1 which are unfortunately impossible to use with Godot 3.x.

@aaronfranke
Copy link
Member

aaronfranke commented Jul 27, 2020

@AndrewMorsillo Probably not. It's not really reasonable to expect major features like this to be backported to stable branches, since that's not the point of stable branches and major changes like this have a high chance of breaking things. If you want to use new features, you will simply have to upgrade to new versions when they come out.

EDIT: Also, this PR already has known compatibility breakages (though they are minor).

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.

NetStandard 2.1 support Switch to the new csproj system
4 participants