Make property_*_revert methods multilevel and expose them for scripting

[YuriSizov]

Closes #43078. These methods used to be hidden inside of the Inspector logic, though some classes implemented and exposed them to the user (for no particular purpose, since they were exposed just because they were registered with ClassDB and users couldn't do anything meaningful with them).

However, users could still use them, if they knew about them. This makes them officially a part of Object, exposed to API, and multilevel to boot, so that classes can extend each other instead of replacing each other.

You can use them like this now:

@tool
extends Node

@export var test_me : int = 0

func _property_can_revert(property : StringName) -> bool:
	if property == "test_me":
		return true
	return false

func _property_get_revert(property : StringName):
	if property == "test_me":
		return 3
	return null

---

I implemented the necessary core logic for GDScript and C#, but I would appreciate assistance with VisualScript, unless we no longer care about it due to recent changes.

Edit: I'm not sure if ML calls exist in VisualScript. I guess it's okay to leave it as is for now.

[YuriSizov]

Seems to fail on some godot-cpp stuff, I guess I have to update it for the CI to pass at some point. But this PR needs to be reviewed before that.

[neikeq]

Is there a reason for this to be implemented in ScriptInstance instead of just being a normal virtual call? If it's because of the special way GDScript does multilevel calls, then the CSharpInstance version should simply redirect to callp.

[YuriSizov]

It is because of the special multilevel call rules, yes. Seems to be the only way to do it in Object with all that boilerplate, and well, nothing in Object uses GDVIRTUAL for whatever reason. I can try hooking it up in CSharpInstance.

[YuriSizov]

Okay, I followed the CSharpInstance::callp and other ML methods implementations and figured how it's supposed to work for feature parity. Let me know if this is okay.

[neikeq]

What I meant is that the implementation should call callp directly, not that it should be implemented similarly. Multilevel calls in C# are done manually by the user, e.g.:

// Object

public virtual bool _PropertyCanRevert(StringName property)
{
    return false;
}

// A inherits Object

public override bool _PropertyCanRevert(StringName property)
{
    if (property == "foo")
        return true;
    return base._PropertyCanRevert(property);
}

// B inherits A

public override bool _PropertyCanRevert(StringName property)
{
    if (property == "test_me")
        return true;
    return base._PropertyCanRevert(property);
}

[YuriSizov]

What about the existing multilevel methods like get_property_list? I got what you meant, but it seemed wrong to make these methods an exception when their related method is implemented manually, and so is every other Object method that is similarly exposed (set, get, notification). That's what I implied with "parity".

[neikeq]

CSharpScript::get_property_list also leaves multilevel handling up to the user code. Notice the break; at the end:

godot/modules/mono/csharp_script.cpp

Lines 1820 to 1831 in 2e24b76

	if (method) {
	MonoObject *ret = method->invoke(mono_object);
	if (ret) {
	Array array = Array(GDMonoMarshal::mono_object_to_variant(ret));
	for (int i = 0, size = array.size(); i < size; i++) {
	props.push_back(PropertyInfo::from_dict(array.get(i)));
	}
	}
	break;
	}

It always stops after calling the first method it finds.

BTW, just calling callp will help to avoid conflicts with #64089 (regardless of which one is merged first), or at least make any conflicts trivial to solve.

[YuriSizov]

Ah, right, that's why there is a break. I can see how you plan to improve the code in the .Net 6 PR, it all makes sense now.

I think it should be correct now. Let me know if it can be improved.

Edit: Well, I guess it's not correct since it fails to compile. Can't call a non-const method from a const method. I'm open to suggestions from a more experienced C++ developer. Worst case, I can revert this change and add break; to mimic other ML calls implemented manually.

[neikeq]

I would try adding const to callp. It doesn't seem to be mutating anything, so that may compile.

[YuriSizov]

I would try adding const to callp. It doesn't seem to be mutating anything, so that may compile.

Wouldn't I need to change it through the inheritance chain in every base class it overrides too? It looks like it, and I don't think I can do that. I think being a generic call method, it cannot be expected to not mutate anything in every implementation.

Edit: The push below doesn't change anything meaningful, it will still fail to compile.

[neikeq]

Just tried adding const and it gave me an error, so it seems that's not possible. The other option is fine then:

Worst case, I can revert this change and add break; to mimic other ML calls implemented manually.

[YuriSizov]

Yeah, you basically get a method signature mismatch with the base method. Changes done, restored the methods to their previous state, but with a break if the (script) method is ever defined.

[reduz]

seems good to me

[YuriSizov]

Alright, I'll PR an update for godot-cpp and then we'll see if it builds.

[YuriSizov]

Lesgo!