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

Add a tagging system for organizing properties, methods, and signals #8752

Open
nlupugla opened this issue Dec 30, 2023 · 6 comments
Open

Add a tagging system for organizing properties, methods, and signals #8752

nlupugla opened this issue Dec 30, 2023 · 6 comments

Comments

@nlupugla
Copy link

nlupugla commented Dec 30, 2023

Describe the project you are working on

Godot engine.

Describe the problem or limitation you are having in your project

There are often properties, methods, or signals which share similar properties, but the options available for organizing these in a way the editor understands are limited. Properties can be organized to some extent with @export_group and @export_subgroup, but this only works for @exported properties and enforces a tree-like structure. On the other hand, Nodes can be organized into groups, which are essentially tags, and have proven to be extremally useful.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

My idea is to introduce a tag system for properties, methods, and signals that works in much the same way as Node groups. Here are some example use cases:

  1. Tag properties as "serializable" to make saving or networking your game easier.
  2. Tag methods as "ability" to easily get a list of abilities a character has.
  3. Tag signals as "one_shot" to make it clear that certain signals are intended to be connected using the CONNECT_ONE_SHOT flag.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

The most straightforward way to add tags would be using a new GDScript annotation like @tag. For example

signal health_changed
signal mana_changed

@tag("one_shot") # tag applies to both signals below
signal spawned
signal despawned

@tag("serializable")
var id

@tag("serializable", "signal_on_change") # tags apply to both variables below
var health
var mana


@tag("ability")
func punch():
    #punches


@tag("ability")
func kick():
    #kicks


func serialize():
    var dict = {}
    var properties = get_tagged_properties("serializable")
    for property in properties:
        dict[property.name] = get(property.name)
    return dict

It should also be possible to interact with and manage tags in the editor. I'm not great at UI mock ups, but I think it's not too hard to imagine some way of adding a tag filter to this menu:

image

In a complicated project, tags could be organized using a menu like the Group Manager:

image

Edit:

I think that @tag would also cover a large percentage of the use case for the fully custom annotations proposed here: #1316.

If this enhancement will not be used often, can it be worked around with a few lines of script?

The best alternative I can think of is to write a plugin that abuses Object metadata, but that would be pretty messy and certainly more than a few lines of script.

Is there a reason why this should be core and not an add-on in the asset library?

It would be hard to achieve this level of editor and GDScript integration with an add-on.

@Calinou Calinou changed the title Tag system for organizing properties, methods, and signals Add a tagging system for organizing properties, methods, and signals Jan 1, 2024
@vnen
Copy link
Member

vnen commented Jan 2, 2024

Two things I don't understand from the example, in the same place:

@tag("serializable", "signal_on_change")
var health
var mana

First, it seems like this intends the tag to apply to both health and mana properties, but that's not how annotations work. They would only apply to health in this case. There are proposals to make them work in block, but nothing implemented yet.

Finally, the signal_on_change seems odd to me. There's nothing to make this actually emit a signal on change unless you add a custom setter, which does not exist in this example, and you cannot do it externally. So I don't understand what this is supposed to mean.

Regarding the overall proposal, I see pros and cons when compared with full custom annotations.
Pros:

  • It would be simple to implement.
  • It could be well integrated in the editor out-of-the-box.
    Cons:
  • Limited use cases (e.g. can't pass arguments to the tags, can't add any custom behavior).
  • It is prone to typos.
  • It can create conflict between different plugins using the same tag, with nothing to warn you of the conflict (with annotations, it would fail when two are registered with the same name).

Also, with custom annotations you could create a plugin that adds a @tag annotation as proposed here. Plugins can also include UI in the editor.

@nlupugla
Copy link
Author

nlupugla commented Jan 2, 2024

Thanks for the feedback Vnen :)

First, it seems like this intends the tag to apply to both health and mana properties, but that's not how annotations work. They would only apply to health in this case. There are proposals to make them work in block, but nothing implemented yet.

My intention is for the tag to apply to both properties, as you suggested. I'll add a comment to my example to make that clear. Also, isn't the @export_group family of annotations multiline like this?

Finally, the signal_on_change seems odd to me. There's nothing to make this actually emit a signal on change unless you add a custom setter, which does not exist in this example, and you cannot do it externally. So I don't understand what this is supposed to mean.

You're right that in this example, the signal_on_change tag doesn't do anything in the sense that it doesn't make the code run any differently. However, it is useful information for developers reading and writing the code. That said, an I do have an idea that would make a tag like this actually useful at run time.

I'm not exactly sure how the syntax would work, but it's not too hard to imagine supplying @tag with values as well as keys. For example:

@tag(serializable, signal_on_change = health_changed)
var health
@tag(serializable, signal_on_change = mana_changed)
var mana

func _set(property : StringName, value) -> bool:
    if property_has_tag(property, "signal_on_change"):
        property_get_tag_value(property, "signal_on_change").emit()

As for being prone to typos, I think it should be possible for the editor to autocomplete tags that you've used before.

@vnen
Copy link
Member

vnen commented Jan 2, 2024

Also, isn't the @export_group family of annotations multiline like this?

They aren't, it is just a quirk of how the inspector deals with groups and categories, so the only way to end a group is to start a new one. The spacing between the declaration makes no difference at all, since you can add multiple lines between the declarations and the effect is the same. Those are also "standalone" annotations, so they do not actually apply to any property.


However, it is useful information for developers reading and writing the code.

About this, note that when writing code you are rarely looking at the source of the other side. For instance, if you are using a plugin API, you are probably looking at its documentation, not its source code. For the doc itself the tag is not special, as you could just have it written in the description.

This would only be truly useful if the tags are shown in the code completion, so you could see them while writing the code. Even then, if the completion could show the documentation then the tags are not that important anymore, as those could just be part of the description.

For reading the code itself it could help, but could also be just a comment.

I see the filtering of properties by tag useful. For anything else I don't see much difference than documentation/comment in the code.

I'm not exactly sure how the syntax would work, but it's not too hard to imagine supplying @tag with values as well as keys. For example:

@tag(serializable, signal_on_change = health_changed)
var health
@tag(serializable, signal_on_change = mana_changed)
var mana

func _set(property : StringName, value) -> bool:
    if property_has_tag(property, "signal_on_change"):
        property_get_tag_value(property, "signal_on_change").emit()

I can see that, but it's still a partial solution. There's nothing to set the types of the arguments for instance. If you need multiple arguments, you have to use an array or multiple tags, which is not a very convenient syntax.

I think this proposal should be looked at its own merit. Trying to replace all the cases of custom annotations will just create a worse system that is bad at serving its own purpose and fail at actually replacing custom annotations. My point about it is just that with custom annotations this would be redundant or it could just be implemented as a custom annotation itself.

BTW, this example wouldn't work as expected because _set() is only called for properties that aren't declared.

@nlupugla
Copy link
Author

nlupugla commented Jan 2, 2024

This would only be truly useful if the tags are shown in the code completion

Totally! For example, in VS code, when I hover over a symbol for a few seconds, I get a popup with some information about it. It's easy to imagine all the tags for that symbol fitting towards the top of such a popup.

image

How exactly that popup would work in Godot is beyond the scope of this proposal, but my point is that the editor can be much more helpful to the user if it has structured data to work with in the form of tags than if it just has unstructured comments to work with.

I see the filtering of properties by tag useful. For anything else I don't see much difference than documentation/comment in the code.

It would also be nice for filtering methods. One could imagine putting a little filter button next to the spyglass button in the method explorer

image

As for signals, filtering is probably less useful as it's rare for an object to have more than a handful of signals.

I think this proposal should be looked at its own merit. Trying to replace all the cases of custom annotations will just create a worse system that is bad at serving its own purpose and fail at actually replacing custom annotations.

Absolutely, very well said! I didn't include the key/value idea in my OP precisely because it felt overcomplicated and unwieldy.

@HolonProduction
Copy link

HolonProduction commented Jan 3, 2024

I think that @tag would also cover a large percentage of the use case for the fully custom annotations proposed here: #1316.

I don't think so. Multiple people on the original proposal stated that they needed parameters. This also applies to the overlapping use case you presented here. serializable will often take additional parameters (although optional). Things that come to mind:

  • viable range for numbers (to prevent loading manipulated save data that might make problems)
  • should this property be saved locally or synchronized with the cloud
  • is this property global for the installation or tied to a specific save slot

Your other example ability will also require some additional info in a real world example:

  • name
  • description
  • mana points or other game specific stuff

I don't want to say that @tag alone won't be useful in some scenarios. But I think the majority of use cases will still need parameters. Either from the beginning on or as a project evolves.


You also showed the usage to document things like signal_on_change or one_shot. This is were the real strength of this proposal lies IMO. However in the way you showed it I think users will get confused (as perfectly demonstrated by vnen in his first reply). If a user sees this:

@tag("one_shot")
signal foo

they will naturaly assume that the annotation is not only for information, but that it will do something (in this case make connections one shot by default). That is what they learned from other annotations after all. This will lead to confusion and probably some bug reports because someone spotted this code out in the wild and copied it assuming it would do something.


The fact that most use cases will require arguments and the cons that were already listed e.g.:

  • no safeguard against collisions
  • prone to typing errors
    make me lean against adding them as something that should be available for scripting.

But I see value in the documentation capabilities and the possible improvements in searching. I therefore could imagine adding them as a documentation-only feature which goes inside of a doc comment.

## Does some magic.
##
## @tag(one_shot)
signal foo

In this way, no user should come with the expectation that they have an effect. The tags could still be parsed and displayed in the editor to give hints on how you intended usage. Although editing through the editor would not be possible.

Seeing that we have some predefined tags for doc comments like @deprecated or @experimental I could imagine, that it would be useful to be able to add custom tags that fit your projects and teams needs.

Cons Tradeoffs of this approach:

  • no editing in the editor
  • no introspection at runtime (IMO this is the field of annotations though)
  • currently there is no way to have autocompletion in doc comments, but I think there was a proposal on this?

Also aside from my opinion the following needs discussion:

  • What are valid tag names? Do they need to be valid identifiers?
    I have no real opinion on that

TL;DR / Keypoints:

  • The use cases for tag alone are very few compared to custom annotations. That combined with the lack of collision awareness makes annotations the superior solution. They could in theory coexist, but I don't think it is worth the effort.
  • Using @tag for documentation purposes is a valid use case, but it would confuse users. Therefore @tag should be placed inside a doc comment to make clear it is intended for documentation purposes.

Also little site note: you should probably escape annotations as inline code when writing them. Otherwise you may ping some unsuspecting Github users.

@nlupugla
Copy link
Author

nlupugla commented Jan 4, 2024

Thanks for the detailed feedback!

Overall, I actually don't have a strong opinion about how tags manifest inside GDScript, whether they be inside doc comments or an @tag annotation. To me, the important thing is nice editor integration so you can get filtering etc. I was even thinking about bugging @AThousandShips to change the label from GDScript to Editor or Core as my vision for tags is really more akin to Node groups, which are an engine feature.

Marking up a script with tags seemed like the most economical way for a user to create them, but one could imagine adding some widgets to the property inspector to allow adding tags from a GUI. Another approach could be something akin to how signal connections show up in the script editor. In the same way that clicking on the arrow icon in the gutter brings up a window displaying information about a signal connection, one could imagine a little tag icon which brings up a menu for editing tags. I'm not really sure what the best approach would be as I'm not much of a UI person, but I appreciate that there is some positive feedback towards the idea of tags, even if not as a GDScript language feature.

image image

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

4 participants