Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Object metadata. What to do with it? #18591

Closed
reduz opened this issue May 3, 2018 · 80 comments
Closed

Object metadata. What to do with it? #18591

reduz opened this issue May 3, 2018 · 80 comments

Comments

@reduz
Copy link
Member

reduz commented May 3, 2018

As you guys probably know, all objects in Godot can have metadata. This is accessed from code as:

obj.set_meta("key",value)
obj.has_meta("key")
obj.get_meta("key")

This metadata is even serialized, but currently it's usage by the community is rather unclear to us developers. In the editor itself, It's mostly unused. There remain a few cases where the 2D editor puts metadata into nodes (for locking and marking as bones) but this could be moved away.

Metadata can't be edited by users from the UI. This would be doable, but then editor should not store data in there anymore. This is also easy to do.

I'm also not sure if anyone uses these functions from code in games.

So, we were discussing with other devs, what should we do with it? I see three paths for this:

  1. Move the remaining usages of it away from it and deprecate the feature
  2. Move the remaining usages of it away from it, but keep the feature AND make metadata available for editing in inspector
  3. Move the remaining usages away from it and do nothing.

Feedback very welcome on this!

@ElfEars
Copy link

ElfEars commented May 3, 2018

2 definately. This is a really useful feature for distinguishing stuff and 2 makes it so much more viable.

@ignaloidas
Copy link

ignaloidas commented May 3, 2018

  1. It's a feature I would definitely use if it would be made more available, and I think it would be a waste to deprecate this feature

@EliseoDiegoViola
Copy link

EliseoDiegoViola commented May 3, 2018

3 For coherence
2 If you want to take an extra step.

The feature is extremely useful for plugins which import assets into the game.

Also, since we are talking about metadata , I would like to bring into the table the chance to define metadata in tiles.

@ghost
Copy link

ghost commented May 3, 2018

I use it all the time when I want to quickly store data in nodes that don't have script attached (so I have no control over their member vars). Anything will do, just don't remove it. :P

@adeluiz
Copy link

adeluiz commented May 3, 2018

If 1 makes Godot faster, then 1. Else 2

@pchasco
Copy link

pchasco commented May 3, 2018

I think in most cases where someone would use metadata, an export var would be more appropriate. That does require the additional step of creating a script and entering a line of code, so that is somewhat inconvenient.

One usage of meta that I can see is if another node wants to modify the metadata of a node. For example, to mark the node as having been seen in a stealth-type game. This obviously breaks all kinds of best practices for software design, but we’re just making games here. A finished game is the goal; the most elegant scripts are worth nothing if the game is never finished.

I say leave it in, but don’t use it for any internal Godot functions.

@BeayemX
Copy link
Contributor

BeayemX commented May 3, 2018

In the high level multiplayer tutorial it is used to store the network_peer

http://docs.godotengine.org/en/3.0/tutorials/networking/high_level_multiplayer.html

@waldson
Copy link
Contributor

waldson commented May 3, 2018

I use it to store Tiled's custom properties.

@willnationsdev
Copy link
Contributor

willnationsdev commented May 3, 2018

In the custom type system, it is used to store the overriding _editor_icon that shows the custom type's icon in the Scene Dock / CreateDialog. In the stuff I'm working on in the editor, I'm currently using it to store the custom type's fully qualified typename (including namespace). This allows me to track editor-specific data without needing to modify core to add unnecessary, use-case-specific properties to the Object class.

If you DO go with option 2, you might(?) wanna make it so that meta properties can be hidden from the Editor view that edits the values (perhaps only displaying values that don't have a leading underscore?).

I would recommend 2 or 3. I generally think that using metadata for general properties on a node is a bad idea though. My use case just works well because the alternative would be adding code-bloat to the Object class in core.

@FabienDeshayes
Copy link

I see the main usage being importers adding their own custom properties / data structure. For example the tiled-importer.

I'm in favor of 3, just because I don't see much value in making this editable: if the data is imported via an addon, I treat is as read-only and will change it via the source only. It would be nice to see it but I don't think of it as important.

@Bauxitedev
Copy link

Bauxitedev commented May 3, 2018

I vote for 2 simply because the Tiled importer I used half a year ago for a game of mine put custom properties into the metadata. Without metadata that would've been a big headache to deal with.

Additionally, being able to edit metadata from the Inspector would be very useful for storing "material properties" inside of SpatialMaterials, comparable to $surfaceprop in the Source engine. Basically that means some materials have different friction/decals/sounds when you step on them or particles when you shoot them. Metadata is an ideal way to store this.

@Norrox
Copy link
Contributor

Norrox commented May 3, 2018

2!

@neikeq
Copy link
Contributor

neikeq commented May 3, 2018

One thing to keep in mind is that the metadata field makes the Object class a bit fatter, so there is a price for keeping it.
An alternative to removing it could be improving it. I find the implementation being a Variant Dictionary to be unnecessarily slow. Why not something like an array of components? That would fit the needs of those using it the way the tiled-importer does, and improve performance.

@reduz
Copy link
Member Author

reduz commented May 3, 2018

I find the implementation being a Variant Dictionary to be unnecessarily slow. Why not something like an array of components?

Probably would be better as an actual hash map from StringName to Variant, keeping insertion order. This way it's only a pointer when unused.

@CodeChomper
Copy link

2 please that way I don't have to make a script to attach data to a scene.

@willnationsdev
Copy link
Contributor

Is there any way to attach metadata to a scene itself? I wouldn't think so since it'd have to be saved into the scene file.

@neikeq
Copy link
Contributor

neikeq commented May 3, 2018

@willnationsdev Either to the PackedScene, the SceneState or the root Node. There's nothing else closer to be considered a scene.

@willnationsdev
Copy link
Contributor

@neikeq Right, but if you unloaded and then reloaded the PackedScene (and if you didn't want to instance the scene to store additional data about it), then that data would be lost, wouldn't it? Or would the PackedScene's metadata be stored inside the .tscn file too?

@neikeq
Copy link
Contributor

neikeq commented May 3, 2018

@willnationsdev Not sure if I understand the cases you mention. But if the PackedScene resource has the metadata asigned at the time it's saved to disk, then it should be stored in the .tscn.

@vinterskog
Copy link

Didn't even know about this! Could this be used to store additional per-tile data in TileMap and GridMap? I guess probably not since all they store are simple IDs, right?
Anyway, I think this is a useful feature and shouldn't be removed. One question though: It seems as if for every object in Godot, an additonal dictionary is stored for this. Does this mean that there is some memory overhead to objects because of this? Even if the dictionary isn't actually used?

@willnationsdev
Copy link
Contributor

@vinterskog I suspect so based on reduz's reply earlier:

Probably would be better as an actual hash map from StringName to Variant, keeping insertion order. This way it's only a pointer when unused.

And the only way you'd be able to use the TileMap's metadata to store tile data would be to use the tile ID as a key to a sub-HashMap or something like that...if I'm not mistaken. I know there is another Issue about that topic already.

@LikeLakers2
Copy link
Contributor

2, most definitely. We already have many many ways to configure, sort, and interact with our nodes -- what's one more, especially since it's arbitrary data that the dev can parse to their needs?

@Zylann
Copy link
Contributor

Zylann commented May 3, 2018

I never needed this, even when it comes to adding properties to objects without a script. It's more a last-resort quick-feature for cases where time lacks or every other design patterns fail to address, because it is more convenient in the long term to attach a script in order to declare them (be it a Node or a PackedScene). So even if they were editable in the inspector it would be potentially error-prone, especially when there are many properties. I'm fine with 1 or 2 if most people like the feature. Definitely the editor shouldn't pollute this unless absolutely necessary.

The only time I started needing it was in GDNative to store type tags when they weren't implemented, but that was an old workaround we got rid of.

@eon-s
Copy link
Contributor

eon-s commented May 3, 2018

Is unused due to the lack of information about it, it can be useful if gets better access (like inspector) and can reduce some complexity in certain nodes, also used on scenes that do not need a script and for "data nodes".

so, 2 too.

@akien-mga akien-mga added this to the 3.1 milestone May 3, 2018
@ol-smaug
Copy link

ol-smaug commented May 3, 2018

It's definitely a convenient feature and is nice to have available in certain scenarios. Either 2 or 3 would be fine so it doesn't go away. :)

@CowThing
Copy link
Contributor

CowThing commented May 3, 2018

For a long time I didn't even know the metadata existed, and I'm still not completely sure what its use is. If it is kept there should probably be a page in the docs describing it and giving examples.

@Ranoller
Copy link
Contributor

Ranoller commented May 3, 2018

I don´t know if this concerns metadata of TreeItems and similar, but if it is, all my plugins use that (and there are some of they), and i don´t find other way to do that.
In game i remember used .set_meta .get_meta in any project... but currently, only in plugins.

So 2. If it include controls
And 1. If exposed metadata to gdscript- TreeItem and similar is respected.

@SuperDIMMaX
Copy link

SuperDIMMaX commented Dec 19, 2018

Access meta in editor?
Maybe it will be usefull... anyone :/

export var meta : Dictionary setget meta_set, meta_get

func meta_set(v):
	self.__meta__ = v

func meta_get():
	return self.__meta__

small tip
get_meta() and set set_meta() == self.__meta__

@nobuyukinyuu
Copy link
Contributor

@SuperDIMMaX
Copy link

SuperDIMMaX commented Dec 19, 2018

@SuperDIMMaX

https://docs.godotengine.org/en/3.0/classes/class_object.html#class-object-get-meta

I know. but self.__meta__ - faster for me )
I.e. a = 1 == set("a", 1)

@SuperDIMMaX
Copy link

metadata is simple dictionary in object named __meta__ .... I guess ....

@willnationsdev
Copy link
Contributor

Don't we just need to make a new EditorPropertyMetadata that's identical to the Dictionary one but hardcoded to have Strings for keys rather than any Variant? And then you just have to add __meta__ to the Object properties in the inspector with some sort of property hint. No?

@SuperDIMMaX
Copy link

SuperDIMMaX commented Dec 20, 2018

Don't we just need to make a new EditorPropertyMetadata that's identical to the Dictionary one but hardcoded to have Strings for keys rather than any Variant? And then you just have to add meta to the Object properties in the inspector with some sort of property hint. No?

I use this variant - temporary ... i see, someone need access metadata in editor - this temporary variant.
Or you mean write editor plugin?

@SuperDIMMaX
Copy link

And i see strange with get access to meta - self.__meta__ working __meta__ not work.

@willnationsdev
Copy link
Contributor

I am referring to just exposing it as a property visible to the Inspector (since I've never seen it there, and I'm therefore guessing it doesn't have PROPERTY_USAGE_EDITOR), defining an EditorProperty object to render it similarly to a Dictionary, but only with String keys allowed, and then handling all the calls on the backend via _get, _set, and _get_property_list.

I'm not sure what you mean by "temporary variant".

@SuperDIMMaX
Copy link

SuperDIMMaX commented Dec 20, 2018

now, in inspector you not see metadata of node, in my case, you see and can modify metadata in editor...
export

export var meta : Dictionary setget meta_set, meta_get

func meta_set(v):
	self.__meta__ = v

func meta_get():
	return self.__meta__

with remote debug :)

@SuperDIMMaX
Copy link

2nd variant:

tool
extends Node2D

func _get_property_list():
	return  [{"hint": "Metadata", "usage": PROPERTY_USAGE_DEFAULT, "name": "Metadata", "type": TYPE_DICTIONARY}]

func _get(p):
	if p == "Metadata":
		return self.__meta__
func _set(p, v):
	if p == "Metadata":
		self.__meta__ = v
		return true

Write plugin or not....

@willnationsdev
Copy link
Contributor

Yes, I was suggesting that something similar to this be done, but on the editor C++ side, so that it's built into the engine.

@SuperDIMMaX
Copy link

1 question: Why meta - self.__meta__ working __meta__ not work (node.__meta__ - working)?

@ArdaE
Copy link
Contributor

ArdaE commented Dec 20, 2018

@SuperDIMMaX, what you’re doing is a nice attempt but it’ll only work in cases where metadata is not really needed in the first place. (If you were able to and have already created a script for the node, why not add your own dictionary to it?).

In more detail: Metadata solves a problem that occasionally arises when one uses someone else’s framework, where extra data needs to be attached to built-in types. Many frameworks solve this by providing a generic Tag you can use however you want, while others — including Godot — go a step further and provide a pre-made dictionary for it (for ease of use at the expense of some minor performance/memory penalty for some use cases). The problems that metadata solves only exist when there’s a distinction between framework (engine) code and user code. If you’re able to attach a script to a node type like you do in your case, that means you could have created your own dictionary to start with, and that you really didn’t need Godot’s metadata feature.

If metadata is to be displayed in Godot’s editor, support for it should be implemented within the editor. (Unless Godot supports extension of existing classes with new functions, in which case your code could be used by attaching it to the Object class. I don’t know if Godot has support for such a concept or not, though technically it should be able to support it, considering its general design).

@SuperDIMMaX
Copy link

In my case, need dynamically create nodes with unknown number of variables (and functions)...
I can write:

class foo extends Node:
    var a = {}

var t = foo.new()
t.a = {"j":100, "k":200}
add_child(t)

but, its slowly and need more ram...
P.s. set_meta("a",100) faster, than self.__meta__["a"] = 100

@akien-mga akien-mga modified the milestones: 3.1, 3.2 Jan 17, 2019
@mysticfall
Copy link
Contributor

mysticfall commented Jan 27, 2019

Currently, I'm using it to store additional data (i.e. keys) for TreeItem. But it can be replaced with something else, so I don't have a strong opinion about the matter.

On a side note, I wish it didn't throw an error when an unserializable value is given, so that it can treat such values as transient.

The reason is, I want to use it as a way to overcome such a case where you need to add a feature to a built-in node type but cannot extend it.

In C#, there's a concept called 'extension method' with which you can 'attach' a new method to a given type without actually modifying its source. But you can't add any property that way (I heard it's planned for a future release though) so object metadata could be used as a backing store for such ad-hoc properties, if it supported unserializable values.

It could be quite a corner case, but still I think it can be also useful in other scenarios as well, like for storing an actual game object in a TreeItem rather than just its key, for example.

@jonbonazza
Copy link
Contributor

Could this be used (maybe with some QoL improvements from the editor) to easily support Saved Node State? (For example, for saving games).

@nobuyukinyuu
Copy link
Contributor

Probably, but node state can already be saved with PackedScene into a serialized format. It might be more useful as a feature request to be able to have that output to a string if you want to save such things into metadata.

@slapin
Copy link
Contributor

slapin commented Feb 22, 2019

nice feature to add simple marks and data to objects when groups are not enough.
that would be great if I knew about this feature before... So many useless scripts would be eliminated.

@slapin
Copy link
Contributor

slapin commented Feb 22, 2019

I'd be fine with 2 or 3, also would be nice to have instance-specific metadata too.

@mhilbrunner
Copy link
Member

Closing this, as the consensus seems to be to definitely keep object metadata. I opened a new issue for exposing metadata to the editor: #29081

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests