godot icon indicating copy to clipboard operation
godot copied to clipboard

[Godot 4] `Object.get_property_list()` and `Script.get_script_property_list()` contain `Category`, `Group` and `SubGroup`.

Open Daylily-Zeleen opened this issue 3 years ago • 11 comments

Godot version

4.0 beta 5

System information

Windows 11, Vulkan, GTX1060

Issue description

Object.get_property_list() and Script.get_script_property_list() will get some element which should not be a property. For example

       {
		"class_name": "",
		"hint": 0,
		"hint_string": "",
		"name": "Node",
		"type": 0,
		"usage": 128
	},


	{
		"class_name": "",
		"hint": 0,
		"hint_string": "res://test_class.gd",
		"name": "test_class.gd",
		"type": 0,
		"usage": 128
	},


	{
		"class_name": "",
		"hint": 0,
		"hint_string": "process_",
		"name": "Process",
		"type": 0,
		"usage": 64
	},

In 3.x, this will not happen.

Steps to reproduce

A simple script, named test.gd :

extends Node

A EditorScript scipt :

@tool
extends EditorScript
	
func _run() -> void:
	var s = preload("test.gd")
	var tmp = s.new()
	print(JSON.stringify(tmp.get_property_list(),"\t"))
	print("=======")
	print(JSON.stringify((s as GDScript).get_script_property_list(),"\t"))
	tmp.queue_free()
	return

Run it, you will see some elements which should not be a property for a object.

Minimal reproduction project

No response

Daylily-Zeleen avatar Nov 17 '22 11:11 Daylily-Zeleen

I'm not sure. This is how categories and groups are registered. I suppose it is unexpected behavior, to an extent, but it renders making use of _get_property_list and the PROPERTY_FLAGS for them more unusual, because it would be as if the engine doesn't work the same way as the exposed method, from the user's perspective.

Then, if they are excluded, a new issue could be reported in the future, claiming the method excluding its own groups from get_property_list to be the bug, instead.

Mickeon avatar Nov 17 '22 12:11 Mickeon

I'm not sure. This is how categories and groups are registered. I suppose it is unexpected behavior, to an extent, but it renders making use of _get_property_list and the PROPERTY_FLAGS for them more unusual, because it would be as if the engine doesn't work the same way as the exposed method, from the user's perspective.

Then, if they are excluded, a new issue could be reported in the future, claiming the method excluding its own groups from get_property_list to be the bug, instead.

Maybe we should provide a bool argument object_properties_only for Object.get_property_list() and Script.get_script_property_list(), to decide exclude these wrong properties or not.

Daylily-Zeleen avatar Nov 17 '22 12:11 Daylily-Zeleen

While it sounds good on paper, it also needs some concrete examples, as it is a bit of a feature request.

Mickeon avatar Nov 17 '22 12:11 Mickeon

In 3.x, this will not happen.

Nothing changed since Godot 3.x in that regard. The property list has always contained the entire list of items as you see it in the Inspector dock, and that includes categories, groups, etc.

Here's a quick test from 3.5.1:

tool
extends EditorScript

func _run() -> void:
	var c := Node.new()
	print(c.get_property_list())
	

Note the first line, that's a category:

[
{class_name:, hint:0, hint_string:, name:Node, type:0, usage:256}, 
{class_name:, hint:20, hint_string:, name:editor_description, type:4, usage:1048578}, 
{class_name:, hint:0, hint_string:, name:_import_path, type:15, usage:1048581}, 
{class_name:, hint:3, hint_string:Inherit,Stop,Process, name:pause_mode, type:2, usage:7}, 
{class_name:, hint:3, hint_string:Inherit,Off,On, name:physics_interpolation_mode, type:2, usage:7}, 
{class_name:, hint:0, hint_string:, name:name, type:4, usage:0}, 
{class_name:, hint:0, hint_string:, name:unique_name_in_owner, type:1, usage:5}, 
{class_name:, hint:0, hint_string:, name:filename, type:4, usage:0}, 
{class_name:Node, hint:19, hint_string:Node, name:owner, type:17, usage:0}, 
{class_name:MultiplayerAPI, hint:19, hint_string:MultiplayerAPI, name:multiplayer, type:17, usage:0}, 
{class_name:MultiplayerAPI, hint:19, hint_string:MultiplayerAPI, name:custom_multiplayer, type:17, usage:0}, 
{class_name:, hint:0, hint_string:, name:process_priority, type:2, usage:7}, 
{class_name:Script, hint:19, hint_string:Script, name:script, type:17, usage:7}
]

Judging by your proposals, you want to get a list of serializable properties. All properties marked with the export keyword (or the @export annotation in 4.0) have usage flag PROPERTY_USAGE_STORAGE. You can do a bitwise check to filter them out. You can also use PROPERTY_USAGE_EDITOR to filter them based on the fact that they appear in the Inspector dock. Neither categories, nor groups would go past this filter.

You can also use PROPERTY_USAGE_SCRIPT_VARIABLE for script properties.

tool
extends EditorScript

func _run() -> void:
	var c := Node.new()
	
	for prop in c.get_property_list():
		if prop.usage & PROPERTY_USAGE_STORAGE:
			print(prop.name)
	
	print("-----")
	for prop in c.get_property_list():
		if prop.usage & PROPERTY_USAGE_EDITOR:
			print(prop.name)
_import_path
pause_mode
physics_interpolation_mode
unique_name_in_owner
process_priority
script
-----
editor_description
pause_mode
physics_interpolation_mode
process_priority
script

There may be other situations where a property should not be serialized, and it's not just for "editor hacks", as one of your proposal suggests. So this filtering approach is what you should use, both in 3.x and in 4.x.

YuriSizov avatar Nov 18 '22 10:11 YuriSizov

I reconfirm the behavior of 3.5, it will return category, too.

But still have something different.

Here is my test: A simple script named "test.gd"

extends Node

In 3.5:

tool
extends EditorScript

func _run() -> void:
	var s = preload("test.gd")
	print(JSON.print((s as GDScript).get_script_property_list(),"\t"))

will print:

[

]

In 4.0:

@tool
extends EditorScript

func _run()->void:
	var script = preload("test.gd")
	print(JSON.stringify((script as GDScript).get_script_property_list(),"\t"))

Will print:

[
	{
		"class_name": "",
		"hint": 0,
		"hint_string": "res://test.gd",
		"name": "test.gd",
		"type": 0,
		"usage": 128
	}
]

I still think it is important that provide a way to obtain real properties of object and script by Godot core, it means that Godot specify the concept of property.

Daylily-Zeleen avatar Nov 18 '22 10:11 Daylily-Zeleen

In 4.0:

@tool
extends EditorScript

func _run()->void:
	var script = preload("test.gd")
	print(JSON.stringify((script as GDScript).get_script_property_list(),"\t"))

Will print:

[
	{
		"class_name": "",
		"hint": 0,
		"hint_string": "res://test.gd",
		"name": "test.gd",
		"type": 0,
		"usage": 128
	}
]

Right, this was added in https://github.com/godotengine/godot/pull/63712 so that you can detect without hacks which script the property comes from in a complex inheritance chain.

I still think it is important that provide a way to obtain real properties of object and script by Godot core, it means that Godot specify the concept of property.

All these are real properties, they just have different usage flags. That's how our language-agnostic API is exposed. You shouldn't have any problem filtering out only the properties you need based on their usage flags.

YuriSizov avatar Nov 18 '22 11:11 YuriSizov

All right,let this issue to be a suggestion.

Not necessary,but I think it is one of improvment.

The implement of 68781 is not breake change, if it have possibility of merger to master, we should talk about how to filter exactly.

Daylily-Zeleen avatar Nov 18 '22 11:11 Daylily-Zeleen

Now I provide a int argument usage_flags to filter proerties. It should be more elegent and better performance.

for prop in get_property_list():
    if prop.usage & PROPERTY_USAGE_STORAGE or prop.usage & PROPERTY_USAGE_SCRIPT_VARIABLE:
        print(prop.name)

will be replace by

for prop in get_property_list(PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_SCRIPT_VARIABLE):
    print(prop.name)

And left argument empty will perform old behavior( all properies).

Daylily-Zeleen avatar Nov 21 '22 12:11 Daylily-Zeleen

I suppose it may not hurt to be optionally able to provide a usage_filter_flags argument, but there needs to be more reasons to justify its existence.

Mickeon avatar Nov 21 '22 13:11 Mickeon

I'm gonna comment here, even though my question isn't the same as this problem here, just related. I personally appreciate that category, group and subgroup are in get_script_property_list() as I need them for a plugin I'm working on (dialogic).

However is there a reason why get_property_list() and get_script_property_list() return the properties in an order where the custom groups and subgroups are at the end, basically making it impossible for me to know what properties are in those groups? Is there a better way to know what exported properties are in what exported groups?

Answers are much appreciated.

Jowan-Spooner avatar Jan 29 '23 13:01 Jowan-Spooner

Groups should always come before the properties they include, otherwise even our own Inspector would be confused. If this is not happening, there is a bug. But since there are no reported issues like that for the inspector, then perhaps it's an issue with passing data to GDScript.

YuriSizov avatar Jan 29 '23 13:01 YuriSizov

I suppose it may not hurt to be optionally able to provide a usage_filter_flags argument, but there needs to be more reasons to justify its existence.

I would even go a step further and provide filtering options for the expected type/class_name too if things would move in this direction. I'm fine with doing the filtering myself, but if there's built-in functionality for it I would definitely use it. This specific issue is also a great example of how filters could be useful to the majority of users; in many cases you wouldn't need to know about variable grouping unless you're working on an editor plugin.

robbertzzz avatar Oct 12 '23 12:10 robbertzzz