godot

godot copied to clipboard

Add "this." to prevent errors caused by duplicate variable names. "ScriptPropertiesGenerator" uses the "using" of user script to prevent the error of missing "using".

加上"this.",防止和生成的变量名重名导致的错误. "ScriptPropertiesGenerator"使用用户脚本的"using",防止缺少"using"的错误.

Bugsquad edit:

• Fixes https://github.com/godotengine/godot/issues/65938.

I will need some example script code to understand what this fixes.

using Godot;
using Godot.Collections;
using System;

public partial class new_script : Godot.RefCounted
{
	string name = "test";

 	[Export]
	public Array<string> Skills { get; set; } = new Array<string>();

}

These codes generate these errors

test4bug.zip

I still think it would be preferable to always use fully qualified names (and not omitting global::) in generated code instead of adding usings. To clarify what this PR intends to fix is the generator for GetGodotPropertyDefaultValues which, currently, for an exported property like this:

[Export]
public Array<Node> MyNodes { get; set; } = new Array<Node>();

Generates this code:

internal new static System.Collections.Generic.Dictionary<StringName, object> GetGodotPropertyDefaultValues()
{
    var values = new System.Collections.Generic.Dictionary<StringName, object>(1);
    Godot.Collections.Array<Godot.Node> __MyNodes_default_value = new Array<Node>();
    values.Add(PropertyName.MyNodes, __MyNodes_default_value);
    return values;
}

The issue is the default value expression isn't fully qualified since it's taken as is from the source.

This PR intends to fix this by including the same usings in the generated file as the ones declared in the source, this could still conflict and I'd prefer if we didn't omit global either.

This code would still fail even with this PR:

// using Godot; // <-- No using Godot namespace

public partial class Sprite2D : Godot.Resource {} // <- class with same name as one of the classes in Godot namespace, can be defined in a different file

public partial class MyNode : Godot.Node
{
    [Godot.Export]
    public Sprite2D Sprite { get; set; } = new Sprite2D();
}

---

This PR also fixes conflicts when the exported members share the same name with a local variable defined in the generated code by adding this., I have no problem with this part of the PR.

// using Godot; // <-- No using Godot namespace

public partial class Sprite2D : Godot.Resource {} // <- class with same name as one of the classes in Godot namespace, can be defined in a different file

public partial class MyNode : Godot.Node
{
    [Godot.Export]
    public Sprite2D Sprite { get; set; } = new Sprite2D();
}

In the latest PR, this paragraph is valid.

At the beginning, I really wanted to use the fully qualified name, but after a long time of searching the roslyn document, I couldn't find a good way, so I had to do it first.

To cope with various methods, the current solution also has its own advantages

A _x = new A();
A _x = A.C();
A _x = A.C(new B());
A _x = new A(new B());
//Other more and more complex
//Find and compare "fully qualified names" for each one

在最新的PR中,这段是有效的. 刚开始的时候,我确实想用完全限定名,但是翻了很久的roslyn文档,也没找到好的办法,就只能先这样了. 要应对各种方法的情况下,现在的解决方案也有它自己的优势.

A new modification has been added. Try to find the default value of property getters. In the following cases, the default values can now be displayed normally.

    string _aaa = "test";

    [Export]
    string AAA
    {
        get
        {
            return _aaa;
        }
        set
        {
            _aaa = value;
        }
    }

    private string _newAnimation = "xxxxx";

    [Godot.Export(Godot.PropertyHint.Enum, "xxxx,xx,xx")]
    public string NewAnimation
    {
        get => _newAnimation;
        set
        {
            _newAnimation = value;
        }
    }

Adjusted according to #68580

Generally speaking, the default value of the attribute should not change. Users should be advised to avoid using dynamic default values. MyExpensiveMethodWithSideEffects(), perhaps this approach could be adopted.

[Export]
public int MyProperty { get; set; } = MyExpensiveMethodWithSideEffects();

//generated
    private int __MyProperty = MyExpensiveMethodWithSideEffects();

    internal new static System.Collections.Generic.Dictionary<StringName, object> GetGodotPropertyDefaultValues()
    {
        ...
        string __MyPropert_default_value = __MyProperty;
        ...
    }

通常来说,属性默认值应该是不太会变动的. 应该建议用户,避免使用动态的默认值. MyExpensiveMethodWithSideEffects()这种,或许可以采用这样的办法.

Yes, I'll review this tonight. Property default values are delicate so I want to make sure we're not doing anything undesirable.

Thanks!

Looks good to merge.

I think the code to get the initializer and it's value is duplicated in 3 places now, so it's a good candidate to extract to a method. But it's not necessary in order to merge.

For me, naming the method is a very distressing thing ha. There are 4 places where this paragraph is used, and the next time you refactor the file you can consider extracting it into the method.

var sm = context.Compilation.GetSemanticModel(initializer.SyntaxTree);
value = initializer.Value.FullQualifiedSyntax(sm);