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 categories to GDScript exported variables #10303

Closed

Conversation

bojidar-bg
Copy link
Contributor

Closes #4378 via the 4th proposal, as it is easiest to implement
Supports:

category xyz:
    export var x = 2
category "xyz/test":
    export var y = 5
category(test):
    export var z = 6
category("test/xyz"):
    export var w = 8
export var a = 7 # Would appear before the categorised variables because of limitations
func _ready():
    print(x + y + z + w)

Does not yet support:

category a:
    category b:
        export var c = 2
# can be faked using `category "a/b":`

But, most of all, needs some extra testing, as I might have broken things.

@Geequlim
Copy link
Contributor

New keyword

@neikeq
Copy link
Contributor

neikeq commented Aug 13, 2017

I seriously consider GDScript would benefit from attributes to avoid so many new keywords.

EDIT:

For example, if #9469 was merged, it would simply be:

@category xyz
export var x = 2
@category xyz/test
export var y = 5

No modifications to the language, you would just check the field attributes. Less complexity. Even export could be an attribute if necessary.

Closes godotengine#4378 via the 4th proposal, as it is easiest to implement
@bojidar-bg bojidar-bg force-pushed the 4378-gdscript-categories branch from f1916c5 to cfe71c0 Compare August 15, 2017 11:06
@juan-garcia-m
Copy link

juan-garcia-m commented Aug 23, 2017

@neikeq exactly, that could had been a good use for annotations.

@bojidar-bg this is cool, thanks!

@kubecz3k
Copy link
Contributor

kubecz3k commented Nov 6, 2017

Well we don't have attributes in gdscript, but this pr (ability to create exported var categories) is still cool.
@reduz could you give us a hint on how you see this?

@akien-mga
Copy link
Member

Needs rebase after #12969, sorry :D

@Geequlim
Copy link
Contributor

Please stop adding new keywords to GDScript!

@bojidar-bg
Copy link
Contributor Author

bojidar-bg commented Nov 17, 2017

I think I will not be rebasing this right now, since the community doesn't seem to like having more keywords in GDScript.

What might get implemented is the proposal in #10799 (comment), which is to have the script's name instead of "Script Variables", thus splitting variables from extended scripts.

You might react with 👍 if this proposal sounds better to you, with 👎 if you think it won't be any better than current master, and with 😕 if you think this proposal is worse than the one currently implemented in this PR (you might want to also write a comment with why you think so).

@Zireael07
Copy link
Contributor

Derived scripts being separate is fine and dandy but I often find myself with so many variables in a single script that I need categories within this single script. (Mostly a consequence of the fact that I use tool scripts A LOT)

@bojidar-bg
Copy link
Contributor Author

@Zylann Somewhat related to #12837, one day we would be able to separate tool scripts away.

Note that this PR is not required for making categories via GDScript, since you can just use _get_property_list to populate them.

@reduz
Copy link
Member

reduz commented Nov 22, 2017

I agree, too many keywords, we should have annotation support at this point.

@reduz
Copy link
Member

reduz commented Nov 22, 2017

Afte thinking about it, i would do

export_category

so only text like

export_category "hello" would be good, and that's it.

@reduz
Copy link
Member

reduz commented Nov 22, 2017

also do not accept indentifiers in there, it's really confusing

@bojidar-bg
Copy link
Contributor Author

I think I will close this PR for now, maybe to be reopened some time after 3.1 starts. Sorry to everyone who needs it now, but, seems like more GDScript enchancements would come later 😃

@bojidar-bg bojidar-bg closed this Nov 22, 2017
@neikeq
Copy link
Contributor

neikeq commented Nov 22, 2017

@juan-garcia-m's PR was perfect for this IMO. The only thing I would add to it is to optionally use parenthesis for the parameters, this way the attribute can be in the same line as the declaration.

@juan-garcia-m
Copy link

@neikeq I did indeed added parenthesis at first, but I removed them as it seem more in the style of GDScript.
Sadly, I deleted the repo for the PR... Not sure I have the code somewhere in local, but it should be outdated by now.
I am now implementing Java support for Godot externally, and all the ideas I had for the annotations I will do them in Java instead.
But if the community is still interested on having GDScript annotations I could help though.

@kubecz3k
Copy link
Contributor

@juan-garcia-m you are implementing java externally? That is interesting. What VM will you use?

@neikeq
Copy link
Contributor

neikeq commented Nov 23, 2017

@juan-garcia-m Your commits are still available in #9469 :)

@juan-garcia-m
Copy link

@neikeq Oh! I see :)

@kubecz3k IKVM.NET, abusing the fact that we have official Mono support. You will still need to have Mono installed (and a JDK of course), but even Godot itself will be downloaded by the Gradle plugin I am writing. Having addons as dependencies, automatic typed scenes and Eclipse support is on the way. Hopefully I will publish an Alpha in the coming weeks.

@vnen vnen mentioned this pull request Jul 21, 2018
@Xrayez
Copy link
Contributor

Xrayez commented Apr 24, 2020

@Zylann Somewhat related to #12837, one day we would be able to separate tool scripts away.

Note that this PR is not required for making categories via GDScript, since you can just use _get_property_list to populate them.

@bojidar-bg I confirm the ability to create both categories and groups with this method (3.2):

Annotation 2020-04-24 152130

Those prefixes are still visible though, not sure how to hide them to make less verbose in the inspector this way. Here's how I do it:

func _get_property_list():
	var props = []
	add_category("Arms", props)
	add_group("Left", props)
	add_property("left_layer_ofs_1", TYPE_REAL, 1 / 3.0, props)
	add_property("left_layer_ofs_2", TYPE_REAL, 2 / 3.0, props)
	add_group("Right", props)
	add_property("right_layer_ofs_1", TYPE_REAL, 1 / 3.0, props)
	add_property("right_layer_ofs_2", TYPE_REAL, 2 / 3.0, props)
	return props


func add_group(p_name, r_properties: Array):
	r_properties.push_back({
			name = p_name,
			type = TYPE_NIL,
			hint = PROPERTY_HINT_NONE,
			usage = PROPERTY_USAGE_GROUP | PROPERTY_USAGE_SCRIPT_VARIABLE,
		}
	)


func add_category(p_name, r_properties: Array):
	r_properties.push_back({
			name = p_name,
			type = TYPE_NIL,
			hint = PROPERTY_HINT_NONE,
			usage = PROPERTY_USAGE_CATEGORY | PROPERTY_USAGE_SCRIPT_VARIABLE,
		}
	)


func add_property(p_name, p_type, p_value, r_properties: Array):
	r_properties.push_back({
			name = p_name,
			type = p_type,
			value = p_value
		}
	)

@bojidar-bg
Copy link
Contributor Author

@Xrayez In ClassDB::add_property_group, it is done by passing the prefix for the variables inside the group as the hint string of the group.

godot/core/class_db.cpp

Lines 924 to 931 in 26ecd92

void ClassDB::add_property_group(StringName p_class, const String &p_name, const String &p_prefix) {
OBJTYPE_WLOCK;
ClassInfo *type = classes.getptr(p_class);
ERR_FAIL_COND(!type);
type->property_list.push_back(PropertyInfo(Variant::NIL, p_name, PROPERTY_HINT_NONE, p_prefix, PROPERTY_USAGE_GROUP));
}

So, something like this will work:

func _get_property_list():
	…
	add_group("Left", "left_", props)
	…
	add_group("Right", "right_", props)
…
func add_group(p_name, p_prefix, r_properties: Array):
	r_properties.push_back({
			…
			hint_string = p_prefix,
			…
		}
	)
…

@Zireael07
Copy link
Contributor

This is lovely, can one of the snippets go somewhere in documentation? At last, my exported variables won't be a total mess in debugger <3 <3

@Xrayez
Copy link
Contributor

Xrayez commented Apr 24, 2020

Yep, works like a charm!

Annotation 2020-04-24 160742

@Zireael07 _get_property_list method certainly lacks documentation in the built-in docs. There are also related _set and _get virtual methods which need to be overridden in some cases combined with _get_property_list.

For instance, those hint and hint_string act depending on the context, including usage enum constants... Like float ranges for the slider which is wrapped in export(float, 0, 100) syntax on the script level. So those need to be documented per each Variant compatible type.

EDIT: this info could likely be added to http://docs.godotengine.org/en/latest/getting_started/scripting/gdscript/gdscript_exports.html

@Zylann
Copy link
Contributor

Zylann commented Apr 24, 2020

It would be really nice if inspector groups were not something embedded inside _get_property_list (which then affects loading times and reflection calls when executed in exported games)

@3top1a
Copy link

3top1a commented Sep 18, 2020

@Xrayez This works, but how do you access the variables?

@Shadowblitz16
Copy link

why hasn't this been merged in 4 years?
the only problem was what? too many keywords?
we still don't have this feature and annotations still haven't been released.

@SirPigeonz
Copy link
Contributor

why hasn't this been merged in 4 years?
the only problem was what? too many keywords?
we still don't have this feature and annotations still haven't been released.

Check mentions above it was implemented differently and is merged and used for GDScript 2.0

@Zylann
Copy link
Contributor

Zylann commented Apr 19, 2021

I had a look at the PR linked from this issue to the GDScript 2.0 docs, and it seems it doesnt mention category annotations anywhere. The 3.x way of doing it was erased with no replacement: https://github.com/godotengine/godot-docs/pull/4247/files
Was it added later or is it still not implemented?

@Xrayez
Copy link
Contributor

Xrayez commented Apr 19, 2021

See godotengine/godot-docs#3623 (comment), perhaps there might be a thing like dynamic annotations? Who knows, there's no GDScript 2.0 specification out there publicly available... 😛

@Xrayez This works, but how do you access the variables?

Late to respond but usually dynamically created variables can be accessed with set() and get() methods explicitly. But GDScript seems to handle even dynamically created properties via plain . operator (if those properties are not declared with var).

And yeah, you'll have to either:

  • override _set and _get callbacks for them to work
  • declare those properties in script with var, without export keyword (_get_property_list() will act as a configuration method for those properties).

@nathanfranke
Copy link
Contributor

Proposal: godotengine/godot-proposals#1255
Superseding PR: #62707

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

Successfully merging this pull request may close these issues.

Add a way to add categories to the script variables section from GDScript