EditorProperty* are bound but are not recognized within Editor #24042

xDGameStudios · 2018-11-28T18:24:27Z

Godot version:
v3.1.alpha.calinou.a8a92ec

OS/device including version:
OSX Mojave

Issue description:
Posted a while back referring to EditorProperty, being broke #24032 but it seems that it's not the only one. This time the problem seems to be that this classes are not begin recognized inside the editor.
Even though EditorPropertyResource, EditorPropertyInteger, EditorPropertyColor.... they have bound methods and events and everything else but are not recognized within editor and cannot be instantiated.

Steps to reproduce:
try to create any of these:
EditorPropertyResource.new();
EditorPropertyInteger.new();
EditorPropertyColor.new();

doesn't compile!!

akien-mga · 2018-11-28T18:28:07Z

No need to open two issues. I'll close the former.

xDGameStudios · 2018-11-28T18:38:00Z

No need to open two issues. I'll close the former.

Sorry about that, thought it could be considered a different problem all together!

nicktgn · 2019-03-06T08:31:53Z

This is still the issue in Godot 3.1 beta 11 e930fb9
Is anyone working on this? Are there plans to fix this in 3.1?
Writing custom EditorPropertys seems to bring so much more flexibility to editor plugins in Godot. But without concrete classes like EditorPropertyInteger and others, the whole feature seems fairly useless, as you're basically back to creating your own fully custom property controls for each type anyway.

xDGameStudios · 2019-03-06T18:35:10Z

This is still the issue in Godot 3.1 beta 11 e930fb9
Is anyone working on this? Are there plans to fix this in 3.1?
Writing custom EditorPropertys seems to bring so much more flexibility to editor plugins in Godot. But without concrete classes like EditorPropertyInteger and others, the whole feature seems fairly useless, as you're basically back to creating your own fully custom property controls for each type anyway.

Well now that I look at this issue again I don't think exposing all the EditorProperty* classes to be the best approach because then you wouldn't have much flexibility as they have their GUI already implemented.. but we could be sure be given more access to the EditorProperty base class... for example right now we have a get_edited_object() but there is no way to set it... so what is the purpose? or even get_edited_property() as it is right now I don't even understand the purpose of the class all together.

@akien-mga btw don't know if you think it is better to close this thread and make a new one, that is more based on the EditorProperty usage or let it stay like it is!

nicktgn · 2019-03-07T04:39:47Z

Lets consider a trivial use case:
Say we have a custom CharaterStats class derived from Resource with one property, name it Speed, dependent on the other property, say Agility. In the inspector for this class we want our Speed be displayed, but be read-only and not be editable, and its value in the inspector to be updated whenever value of the Agility changes.

# CharacterStats.gd
extends Resource
class_name CharacterStats

export (int, 1, 100) var agility = 1
export (int) var speed setget set_speed, get_speed

func get_speed():
     return agility * 2

func set_speed():
     pass    # make it read-only in code (but this is not properly reflected in the inspector)

Lets say we implement a quick editor plugin to add this "custom" inspector. We can currently do this by extending EditorInspectorPlugin and that would ideally look something like this.

tool
extends EditorPlugin

const CharacterStatsInspectorPlugin = preload("res://addons/custom_editor/CharacterStatsInspectorPlugin.gd")

var plugin

func _enter_tree():
    plugin = CharacterStatsInspectorPlugin.new()
    add_inspector_plugin(plugin)

func _exit_tree():
    remove_inspector_plugin(plugin)
    plugin.free()

# CharacterStatsInspectorPlugin.gd
tool
extends EditorInspectorPlugin 

var speedControl
var agilityControl

func can_handle (object):
    if object is CharacterStats:
        return true
    return false

func parse_property (object, type, path, hint, hint_text, usage):
	match path:
		'speed':
                        speedControl = EditorPropertyInteger.new()
                        control.read_only = true
                        add_property_editor("speed", speedControl)
			return true
		'agility':
			agilityControl = EditorPropertyInteger.new()
			control.connect("property_changed", self, "_on_agility_changed")
			add_property_editor("agility", agilityControl)
			return true
	return false

func _on_agility_changed(property, value, field, changing):
      speedControl.update_property()

But this currently doesn't work.
You can already add EditorProperty using add_property_editor() (or add_custom_control() if you make a fully custom Control) and editor neatly substitutes default property control with the one you declared in code. But beyond that nothing else related to EditorProperty seem to work.

So the issues I see currently:

EditorProperty#read_only (or any other EditorProperty class properties) doesn't seem to do anything
property_changed signal (or any other signals of EditorProperty class for that matter) is never fired unless you explicitly call EditorProperty#emit_changed()
Of course you can just use add_custom_control() and provide your own Control implementation. But it seems a bit excessive as you'll end up re-implementing EditorProperty* controls for the most part anyway.

Possible solutions (options) I see (not mutually exclusive):

Maybe this specific use case can be solved by making whole inspector auto-update all the properties whenever any of the properties change. I think Godot needs this as well, but it is beyond the scope of EditorProperty usage issue.
Exposing EditorProperty* concrete classes can give a more fine level of control over the editor inspector. But then we need to make sure that all the properties and signals of EditorProperty* classes actually work and define more clearly how should they work.
Changing EditorProperty into a sort of factory class which will give us a correct type implementation depending on the type of the property we attach it to.
...

P.S.: Sorry for the long comment, just seems like a pretty complex issue.
I can add here an example project with the code above to test this on, if you guys need.

nicktgn · 2019-03-07T05:52:12Z

Another option I thought of:
4) Change EditorInspectorPlugin#parse_property to give you a default EditorProperty already created by the editor (since it basically does that under the hood anyway). Something like bool parse_property (EditorProperty editorProperty, Object object, int type, String path, int hint, String hint_text, int usage ) virtual . That way you can have control over how it behaves, but won't need to extend it or reimplement it.

xDGameStudios · 2019-03-07T20:28:26Z

Another option I thought of:
4) Change EditorInspectorPlugin#parse_property to give you a default EditorProperty already created by the editor (since it basically does that under the hood anyway). Something like bool parse_property (EditorProperty editorProperty, Object object, int type, String path, int hint, String hint_text, int usage ) virtual . That way you can have control over how it behaves, but won't need to extend it or reimplement it.

this can be done really easily without exposing all of the EditorProperty* what about something like:

var integerControl = EditorProperty.build_property(TYPE_..., object, path, PROPERTY_USAGE, ...)

this way you wouldn't need to expose all the classes just use for example a static method that will return a EditorProperty and you are good to go.

so it's just necessary to make a EditorProperty static method exposed to the Editor... or even make the method exposed in the EditorInspectorPlugin, it makes more sense

nicktgn · 2019-03-08T01:00:38Z

@xDGameStudios I agree, there is no need of exposing the concrete implementation classes. Static method in EditorProperty that you suggested is pretty much what I meant in the option 3 I wrote in the comments above.
What apparent is that something needs to be done about EditorProperty (or EditorInspectorPlugin) as they are not fully usable in their current state (on the GDScript side of things that is).

xDGameStudios · 2019-03-10T15:08:38Z

Another option I thought of:
4) Change EditorInspectorPlugin#parse_property to give you a default EditorProperty already created by the editor (since it basically does that under the hood anyway). Something like bool parse_property (EditorProperty editorProperty, Object object, int type, String path, int hint, String hint_text, int usage ) virtual . That way you can have control over how it behaves, but won't need to extend it or reimplement it.

The problem with this is that in the source code the EditorProperty editorProperty is only created after the parse_property call as you are intercepting the call before DefaultEditorInspectorPlugin(that is hidden in the source code)... and that needs to be that way because you need to tell the Engine if it needs to create it or if you handled it yourself... that's why you return true (already handled don't handle it under the hood) or false (not handled yet, use the next EditorInspectorPlugin on the list... and eventually this falls to the default DefaultEditorInspectorPlugin)

nicktgn · 2019-03-12T01:58:05Z

@xDGameStudios ok, then I guess we go with the static factory method in EditorProperty. I will open a new issue for that.
Shouldn't be hard to implement that. I can add a PR in a day or two.

nicktgn · 2019-03-28T13:22:11Z

@xDGameStudios, @reduz, @akien-mga
Just now got time to look in the engine code. Couldn't find any the classes exposed to GDScript that have any public static methods in them. And binding static methods doesn't seem to be supported either.

So instead I'm thinking of implementing a separate class EditorPropertyFactory with a single method build_property. Something like this:

class EditorPropertyFactory : public Reference {
	GDCLASS(EditorPropertyFactory, Reference)
protected:
	static void _bind_methods();
public:
	EditorProperty *build_property(Object *p_object, Variant::Type p_type, const String &p_path, PropertyHint p_hint, const String &p_hint_text, int p_usage);
};

Any thoughts regarding this?

(Also, not sure if this is a proper place for the engine development questions, but I thought its easier to ask here within the context of this issue)
I've tried to implement the class above, but stumbled upon a following problem: compiler fails to find appropriate function for binding a method with 6 or more arguments.

void EditorPropertyFactory::_bind_methods() {
	ClassDB::bind_method(D_METHOD("build_property", "object", "type", "path", "hint", "hint_text", "usage"), &EditorPropertyFactory::build_property);
}

./core/class_db.h:230:22: error: no matching function for call to 'create_method_bind'
                MethodBind *bind = create_method_bind(p_method);
                                   ^~~~~~~~~~~~~~~~~~
editor/editor_inspector.cpp:499:11: note: in instantiation of function template specialization 'ClassDB::bind_method<MethodDefinition, EditorProperty
      *(EditorPropertyFactory::*)(int, Variant::Type, const String &, PropertyHint, const String &, int)>' requested here
        ClassDB::bind_method(D_METHOD("build_property", "type1", "type", "path", "hint", "hint_text", "usage"), &EditorPropertyFactory::build_property);
                 ^
./core/method_bind.gen.inc:4089:13: note: candidate template ignored: could not match 'void (type-parameter-0-1, type-parameter-0-2, type-parameter-0-3,
      type-parameter-0-4, type-parameter-0-5) const' against 'EditorProperty *(int, Variant::Type, const String &, PropertyHint, const String &, int)'
MethodBind* create_method_bind( void (T::*p_method)(P1, P2, P3, P4, P5) const ) {
            ^
./core/method_bind.gen.inc:4287:13: note: candidate template ignored: could not match 'type-parameter-0-1 (type-parameter-0-2, type-parameter-0-3,
      type-parameter-0-4, type-parameter-0-5, type-parameter-0-6) const' against 'EditorProperty *(int, Variant::Type, const String &, PropertyHint, const
      String &, int)'
MethodBind* create_method_bind(R  (T::*p_method)(P1, P2, P3, P4, P5) const ) {
            ^
./core/method_bind.gen.inc:3891:13: note: candidate template ignored: failed template argument deduction
MethodBind* create_method_bind(R  (T::*p_method)(P1, P2, P3, P4, P5)  ) {
            ^
./core/method_bind.gen.inc:3306:13: note: candidate template ignored: could not match 'void (type-parameter-0-1, type-parameter-0-2, type-parameter-0-3,
      type-parameter-0-4) const' against 'EditorProperty *(int, Variant::Type, const String &, PropertyHint, const String &, int)'
MethodBind* create_method_bind( void (T::*p_method)(P1, P2, P3, P4) const ) {
            ^
./core/method_bind.gen.inc:3498:13: note: candidate template ignored: could not match 'type-parameter-0-1 (type-parameter-0-2, type-parameter-0-3,
      type-parameter-0-4, type-parameter-0-5) const' against 'EditorProperty *(int, Variant::Type, const String &, PropertyHint, const String &, int)'
MethodBind* create_method_bind(R  (T::*p_method)(P1, P2, P3, P4) const ) {
            ^
./core/method_bind.gen.inc:3693:13: note: candidate template ignored: could not match 'void' against 'EditorProperty *'
MethodBind* create_method_bind( void (T::*p_method)(P1, P2, P3, P4, P5)  ) {
            ^
./core/method_bind.gen.inc:3114:13: note: candidate template ignored: failed template argument deduction
MethodBind* create_method_bind(R  (T::*p_method)(P1, P2, P3, P4)  ) {
            ^
./core/method_bind.gen.inc:2547:13: note: candidate template ignored: could not match 'void (type-parameter-0-1, type-parameter-0-2, type-parameter-0-3)
      const' against 'EditorProperty *(int, Variant::Type, const String &, PropertyHint, const String &, int)'
MethodBind* create_method_bind( void (T::*p_method)(P1, P2, P3) const ) {
            ^
./core/method_bind.gen.inc:2733:13: note: candidate template ignored: could not match 'type-parameter-0-1 (type-parameter-0-2, type-parameter-0-3,
      type-parameter-0-4) const' against 'EditorProperty *(int, Variant::Type, const String &, PropertyHint, const String &, int)'
MethodBind* create_method_bind(R  (T::*p_method)(P1, P2, P3) const ) {
            ^
./core/method_bind.gen.inc:2922:13: note: candidate template ignored: could not match 'void' against 'EditorProperty *'
MethodBind* create_method_bind( void (T::*p_method)(P1, P2, P3, P4)  ) {
            ^
./core/method_bind.gen.inc:2361:13: note: candidate template ignored: failed template argument deduction
MethodBind* create_method_bind(R  (T::*p_method)(P1, P2, P3)  ) {
            ^
./core/method_bind.gen.inc:1812:13: note: candidate template ignored: could not match 'void (type-parameter-0-1, type-parameter-0-2) const' against
      'EditorProperty *(int, Variant::Type, const String &, PropertyHint, const String &, int)'
MethodBind* create_method_bind( void (T::*p_method)(P1, P2) const ) {
            ^
./core/method_bind.gen.inc:1992:13: note: candidate template ignored: could not match 'type-parameter-0-1 (type-parameter-0-2, type-parameter-0-3) const'
      against 'EditorProperty *(int, Variant::Type, const String &, PropertyHint, const String &, int)'
MethodBind* create_method_bind(R  (T::*p_method)(P1, P2) const ) {
            ^
./core/method_bind.gen.inc:2175:13: note: candidate template ignored: could not match 'void' against 'EditorProperty *'
MethodBind* create_method_bind( void (T::*p_method)(P1, P2, P3)  ) {
            ^
./core/method_bind.gen.inc:1632:13: note: candidate template ignored: failed template argument deduction
MethodBind* create_method_bind(R  (T::*p_method)(P1, P2)  ) {
            ^
./core/method_bind.gen.inc:1101:13: note: candidate template ignored: could not match 'void (type-parameter-0-1) const' against 'EditorProperty *(int,
      Variant::Type, const String &, PropertyHint, const String &, int)'
MethodBind* create_method_bind( void (T::*p_method)(P1) const ) {
            ^
./core/method_bind.gen.inc:1275:13: note: candidate template ignored: could not match 'type-parameter-0-1 (type-parameter-0-2) const' against
      'EditorProperty *(int, Variant::Type, const String &, PropertyHint, const String &, int)'
MethodBind* create_method_bind(R  (T::*p_method)(P1) const ) {
            ^
./core/method_bind.gen.inc:1452:13: note: candidate template ignored: could not match 'void' against 'EditorProperty *'
MethodBind* create_method_bind( void (T::*p_method)(P1, P2)  ) {
            ^
./core/method_bind.gen.inc:927:13: note: candidate template ignored: failed template argument deduction
MethodBind* create_method_bind(R  (T::*p_method)(P1)  ) {
            ^
./core/method_bind.gen.inc:414:13: note: candidate template ignored: could not match 'void () const' against 'EditorProperty *(int, Variant::Type, const
      String &, PropertyHint, const String &, int)'
MethodBind* create_method_bind( void (T::*p_method)() const ) {
            ^
./core/method_bind.gen.inc:582:13: note: candidate template ignored: could not match 'type-parameter-0-1 () const' against 'EditorProperty *(int,
      Variant::Type, const String &, PropertyHint, const String &, int)'
MethodBind* create_method_bind(R  (T::*p_method)() const ) {
            ^
./core/method_bind.gen.inc:753:13: note: candidate template ignored: could not match 'void' against 'EditorProperty *'
MethodBind* create_method_bind( void (T::*p_method)(P1)  ) {
            ^
./core/method_bind.gen.inc:246:13: note: candidate template ignored: failed template argument deduction
MethodBind* create_method_bind(R  (T::*p_method)()  ) {
            ^
./core/method_bind.gen.inc:78:13: note: candidate template ignored: could not match 'void' against 'EditorProperty *'
MethodBind* create_method_bind( void (T::*p_method)()  ) {

As I understand such functions do exist in core/method_bind_ext.gen.inc but compiler doesn't see them. Am I missing something here?
Would really appreciate some insights from core devs on this one. Thanks.

xDGameStudios · 2019-04-25T00:23:07Z

I've already tried this before and the only workaround I found was to expect a dictionary from the GDScript side with the correct key values :) "type", "path", "hint", "hint_text", "usage"

nicktgn · 2019-04-25T04:20:10Z

@xDGameStudios Yea, as a workaround dictionary would work, of course. But it is not in line with other APIs, so would not look good.

But the weird part is that I can actually compile it with 5 arguments like this:

ClassDB::bind_method(D_METHOD("build_property", "type", "path", "hint", "hint_text", "usage"), &EditorPropertyFactory::build_property);

But not with 6 arguments:

ClassDB::bind_method(D_METHOD("build_property", "object", "type", "path", "hint", "hint_text", "usage"), &EditorPropertyFactory::build_property);

And I do think this function needs 6 arguments to be in line with the method signature of EditorInspectorPlugin#parse_property(). Also this way we can assign object and path to the EditorProperty* instance inside of this factory method (since those variables of the EditorProperty class are not public).

I think this can be useful if we create a new EditorProperty instance outside of EditorInspectorPlugin#parse_property() to be added as custom control with EditorInspectorPlugin#add_custom_control(). Maybe to display some additional properties of a related object to the one we're currently inspecting.

E.g:

EditorProperty *EditorPropertyFactory::build_property(Object *p_object, Variant::Type p_type, const String &p_path, PropertyHint p_hint, const String &p_hint_text, int p_usage) const {
     EditorProperty *ep = NULL;
     // .... 
     //  create EditorProperty instance based on type and stuff
     // ....

     ep->object = p_object;
     ep->property = p_path;
     ep->property_usage = p_usage;

     return ep 
}

xDGameStudios · 2019-04-28T23:45:01Z

I understand the usage as I said I have already implemented this before and it works... but I used a dictionary because there is no support for exposing methods with more than 5 arguments to the Scripting API.. and since no one seemed interested in the idea.. I kind of left it on the side... and didn't made the PR myself :)

AnidemDex · 2021-12-10T00:43:06Z

Are there any updates on this? I found myself replicating nodes from the source just to make this (and some more complex usages than just categories)

For future reference, seems like is mentioned here: godotengine/godot-proposals#300

akien-mga mentioned this issue Nov 28, 2018

EditorProperty is bound but can't be instanciated #24032

Closed

akien-mga added enhancement discussion topic:editor topic:plugin labels Nov 28, 2018

akien-mga added this to the 3.2 milestone Mar 6, 2019

nicktgn mentioned this issue Mar 12, 2019

Add factory method to EditorProperty to instantiate controls for built-in types #26946

Closed

akien-mga removed this from the 3.2 milestone Dec 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EditorProperty* are bound but are not recognized within Editor #24042

EditorProperty* are bound but are not recognized within Editor #24042

xDGameStudios commented Nov 28, 2018

akien-mga commented Nov 28, 2018

xDGameStudios commented Nov 28, 2018

nicktgn commented Mar 6, 2019 •

edited

xDGameStudios commented Mar 6, 2019 •

edited

nicktgn commented Mar 7, 2019 •

edited

nicktgn commented Mar 7, 2019 •

edited

xDGameStudios commented Mar 7, 2019 •

edited

nicktgn commented Mar 8, 2019

xDGameStudios commented Mar 10, 2019 •

edited

nicktgn commented Mar 12, 2019 •

edited

nicktgn commented Mar 28, 2019 •

edited

xDGameStudios commented Apr 25, 2019

nicktgn commented Apr 25, 2019 •

edited

xDGameStudios commented Apr 28, 2019 •

edited

AnidemDex commented Dec 10, 2021 •

edited

EditorProperty* are bound but are not recognized within Editor #24042

EditorProperty* are bound but are not recognized within Editor #24042

Comments

xDGameStudios commented Nov 28, 2018

akien-mga commented Nov 28, 2018

xDGameStudios commented Nov 28, 2018

nicktgn commented Mar 6, 2019 • edited

xDGameStudios commented Mar 6, 2019 • edited

nicktgn commented Mar 7, 2019 • edited

Possible solutions (options) I see (not mutually exclusive):

nicktgn commented Mar 7, 2019 • edited

xDGameStudios commented Mar 7, 2019 • edited

nicktgn commented Mar 8, 2019

xDGameStudios commented Mar 10, 2019 • edited

nicktgn commented Mar 12, 2019 • edited

nicktgn commented Mar 28, 2019 • edited

xDGameStudios commented Apr 25, 2019

nicktgn commented Apr 25, 2019 • edited

xDGameStudios commented Apr 28, 2019 • edited

AnidemDex commented Dec 10, 2021 • edited

nicktgn commented Mar 6, 2019 •

edited

xDGameStudios commented Mar 6, 2019 •

edited

nicktgn commented Mar 7, 2019 •

edited

nicktgn commented Mar 7, 2019 •

edited

xDGameStudios commented Mar 7, 2019 •

edited

xDGameStudios commented Mar 10, 2019 •

edited

nicktgn commented Mar 12, 2019 •

edited

nicktgn commented Mar 28, 2019 •

edited

nicktgn commented Apr 25, 2019 •

edited

xDGameStudios commented Apr 28, 2019 •

edited

AnidemDex commented Dec 10, 2021 •

edited