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

.NET 6 C# - ScriptPropertiesGenerator fail with overridden initialized property on master #71102

Open
AlmightyLaxz opened this issue Jan 9, 2023 · 2 comments · May be fixed by #98396
Open

Comments

@AlmightyLaxz
Copy link
Contributor

AlmightyLaxz commented Jan 9, 2023

Godot version

v4.0.beta.mono.custom_build [56522f7] (with double)

System information

Arch Linux, Vulkan

Issue description

C# projects with a parent class including a virtual property that is then overridden & initialized in a child class appear to cause ScriptPropertiesGenerator & ScriptSerializationGenerator to fail to generate source.

The problem appears to be caused by #70483, the Source Generators run fine if built immediately before this PR was committed.
I'm not sure how to debug the Source Generators for a more useful error/stack trace than below, sorry.

Parent Class

using Godot;
public partial class TestParentClass : RigidBody3D
{
    public virtual int NodeType { get; set; } = 1;
}

Child Class

If the initialized property override below is commented then the Source Generators work fine.

public partial class TestChildClass : TestParentClass
{
    public override int NodeType => 10;
}

Warnings/Errors produced

Warnings produced when a child class overrides the initialized property from its parent:

0>CSC: Warning CS8785 : Generator 'ScriptPropertiesGenerator' failed to generate source. It will not contribute to the output and compilation errors may occur as a result. Exception was of type 'NullReferenceException' with message 'Object reference not set to an instance of an object.'
0>CSC: Warning CS8785 : Generator 'ScriptSerializationGenerator' failed to generate source. It will not contribute to the output and compilation errors may occur as a result. Exception was of type 'NullReferenceException' with message 'Object reference not set to an instance of an object.'

Any [Export] properties then throw build errors as ScriptPropertiesGenerator failed:

0>csharpbug_03c26d6/Godot.SourceGenerators/Godot.SourceGenerators.ScriptPropertyDefValGenerator/Nothing_ScriptPropertyDefVal.generated.cs(9,33): Error CS0117 : 'Node.PropertyName' does not contain a definition for 'TestInt'
0>csharpbug_03c26d6/Godot.SourceGenerators/Godot.SourceGenerators.ScriptPropertyDefValGenerator/Nothing_ScriptPropertyDefVal.generated.cs(11,33): Error CS0117 : 'Node.PropertyName' does not contain a definition for 'TestFloat'

Steps to reproduce

Build the solution in the minimal reproduction project.

Minimal reproduction project

csharpbug_03c26d6.zip

@raulsntos
Copy link
Member

I didn't consider that there may be non-readonly properties with a null SetMethod. #71128 should fix it.

@Framebuffers
Copy link

[Context: This bug is so elusive and hard to catch, that I cannot reproduce it reliably enough to isolate a proper issue. Here's a way it happened to me if it helps anyone. Using Godot 4.3-stable and a self-compiled build of 4.3-stable with PR #79731 baked in. Both of them worked the same. This makes it C#/Mono exclusive.]

The one thing that triggered this bug is this asset from the Asset Library. It's not the asset itself that causes this. This bug happened unexpectedly (while on a game jam), even if under the same conditions it worked before.

The conditions were:

  • Implementing from a class below any level of abstraction that required overriding an auto-set virtual property. (same as shown above)
  • Have, or not, any kind of [Export] attribute attached to the derived class. Could derive from any Godot object, or anything below done by the programmer. (same circumstances as described on the issue, but on a larger scale)
  • The [Export] property were both on auto-set property and fields. This one happened on the asset above.
  • Once you got an error on one, it propagated to any previously-functioning class with similar conditions. Probably because CSC gave up, like the error shown on the issue.
  • Not even a dotnet clean could fix anything.

The only solution was:

  • Removing every virtual property and exported field from all files on the game.
  • Manually editing the project.godot file and the .tscn files to remove some broken script references.
  • Converted my virtual properties to abstract.
  • Changed to .NET 8.0 as a target.
  • Delete the whole .godot folder.
  • Regenerate the mesh import database.
  • dotnet clean from the terminal.
  • dotnet build from the terminal.
  • Build _from within Godot.
  • Play.

It might've been a game-breaker for many. Some steps might look unnecessary but they were in this particular case. The game was so tainted by several days of having this time bomb lurking that I did lose some code, and thankfully I did have some working backups to fall back into.

I did find a comment on the ScriptPropertyDefValGenerator.cs file that kind of makes me think of something.

I am not a professional C# programmer, nor expert in the field, but I did find a pattern:

  • Godot did let me, at some point, export a field/property that was read-only, had a private setter, or was, as described here, virtual and overridden later on. Not a single previous warning in MSBuild, the engine or previous warning.
  • Godot did not know it happened, or that it was bad, until something triggered the error above and then it cascades down below to parts I did not were wrong.
  • The code generator got stuck in such a way that anything, even things completely unrelated, became erratic. Exported fields/properties became so unpredictable that builds sometimes failed, sometimes passed, sometimes changed slightly. It's all because CSC failed at some point. My game at some point booted with the camera completely inverted for one person. On several it caused the audio to revert to -0dB (in my case, something that was almost tinnitus-inducing). Compiles became completely non-deterministic. I can blame that to my own code, though... but it does seem weird the random way it manifests.
  • Of course, the stack trace in Godot .NET is so sparse (to put it very mildly) that it took me around 6 hours to figure this out. The issue in godot-propsals/issues/6811 was in full swing. The only way I could notice it was where it was failing: that asset. And it was working until suddenly it didn't.

I write this after quite a while of figuring out the details of this bug... but I cannot reproduce them 100% reliably, mainly because the aforementioned evasive nature of it. But my gut feeling is that it's not resolving this specific combination of abstraction correctly, and that, worst case, we should get a warning this pattern is not recommended and can cause later harm.

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