CSharpScript should not own method infos of the base class

[paulloz]

• Supersedes CSharpScript should not own method infos of the base class #77273

This is simply a rebased and squashed version of the original PR. We took way too long to review this, and the original author has not been answering the last ping. But this is still a needed, and appreciated, fix.

[raulsntos]

Thanks. I think it makes sense for CSharpScript to only contain the members declared in the C# type that it represents, then the inherited members can be retrieved by iterating the hierarchy chain.

The same change should likely be made to the other members (properties and signals), and we must make sure the behavior matches GDScript (i.e. if GDScript::get_method_list iterates the hierarchy chain and returns inherited methods, then CSharpScript::get_method_list should return inherited methods too).

For example, we still have a loop in RPC functions that retrieve the RPC attribute for methods from base types, and in Event signals that retrieve the signals from base types.

The method generated method GetGodotPropertyList would similarly have to be updated to only consider the properties in the current type (and not the base types), this would also fix #75271.

I know this sounds like a lot, but I'd rather make all these changes in the same PR to avoid ending up in an partial change where some members are retrieved only for the current type, and others also retrieve from the base types.

[paulloz]

I know this sounds like a lot, but I'd rather make all these changes in the same PR to avoid ending up in an partial change where some members are retrieved only for the current type, and others also retrieve from the base types.

While I share the sentiment in principle, this PR is someone else's work that I salvaged because we were saying it should and could still be merged in our internal discussions. I'm not sure that I'm comfortable changing the scope of the PR, and putting their name on code they did not initially intend.

[paulloz]

Currently, the way we connect signals relies on the fact scripts signal lists include all the hierarchy¹. I'm not entirely sure about RPC functions, because I'm not familiar with that part of the codebase at all.

Footnotes

1. https://github.com/godotengine/godot/blob/c4279fe3e0b27d0f40857c00eece7324a967285f/modules/mono/csharp_script.cpp#L1774 ↩

[raulsntos]

Currently, the way we connect signals relies on the fact scripts signal lists include all the hierarchy.

We should change that to iterate the hierarchy, I see that you've already done this in 1d70855.

I'm not entirely sure about RPC functions, because I'm not familiar with that part of the codebase at all.

It looks like the dictionary returned by Script::get_rpc_config must contain the RPC config inherited from base types, so I guess we can keep the current implementation.

For the reference, GDScript initializes this dictionary by duplicating the dictionary from the base script:

godot/modules/gdscript/gdscript_compiler.cpp

Lines 2721 to 2725 in 1d70855

	// Duplicate RPC information from base GDScript
	// Base script isn't valid because it should not have been compiled yet, but the reference contains relevant info.
	if (base_type.kind == GDScriptDataType::GDSCRIPT && p_script->base.is_valid()) {
	p_script->rpc_config = p_script->base->rpc_config.duplicate();
	}

[raulsntos]

Looks good and works as expected, thanks!

[akien-mga]

Thanks!