Script hierarchies for Inspector

[willnationsdev]

Fixes #10799

Demonstration:

Here are some various screenshots. These are from an environment in which I have the following structure:

# class_c.gd
extends "res://class_b.gd"
export(bool) var c = true
export(bool) var c2 = false

# class_b.gd
extends "res://class_a.gd"
var b = true

# class_a.gd
extends "res://class_2.gd"
export var a = "a"
export var a2 = "a2"

# class_2.gd
extends "res://class_1.gd"
export var three = 3
export var four = 4

# class_1.gd
extends Node
export var one = 1
export var two = 2

The only variable/class we should NOT see show up is class_b.gd with the single, un-exported b property with value true.

If I attach class_c.gd to a Node, I see this before/after:

If I have a builtin script with exported builtin1 and non-exported builtin2, then I get this before/after (script_test.tscn is the name of the scene and the built-in script extends a Node type):

If I have class_c open, and I change it to extend from class_2 instead of class_b, but I HAVEN'T saved it yet, then I will see this before/after (looks the same as the saved version, which was the goal):

In the course of doing this, I did find another small bug for the unsaved scripts (going from less derived to more derived base class in an unsaved script. In this case, I went from class_1 to class_a, so class_2 and class_a's variables are showing up under class_1`'s), so it seems it still needs some more work.

Edit: Oh wait! No, that's a purposeful "bug". If you ADD scripts and properties to the inheritance hierarchy, but the script hasn't been saved, the p_list ends up with all of these extra properties and doesn't yet know about the new scripts, so it just bundles them all underneath the most relevant script it can find. It gets revised when you actually save the script.

(after saving:)

Incidentally, if you add a script to a node that has nothing else in the script hierarchy, and the script is not yet saved (so it doesn't have a name yet), AND if it has export variables that show up in the Inspector, then it starts autonaming the category blocks with "Class 0", "Class 1", etc. for every script that isn't yet saved. If anywhere in the script hierarchy there is a saved script, then all of the unsaved scripts' properties are assumed to belong to the saved script, until those other scripts are saved. This is why we see the "buggy" behavior above. Here's what this algorithm results in for the objects with no saved scripts:

Regardless of whether this ends up as part of the 3.0 release or not, I might actually start extending this out into a full on overhaul of custom types in general, targeting a 3.1 release. This would be a critical change to have for a full custom type simulation across the engine.

Edit:

Here are some various screenshots. These are from an environment in which I have the following structure:

# class_c.gd
extends "res://class_b.gd"
export(bool) var c = true
export(bool) var c2 = false

# class_b.gd
extends "res://class_a.gd"
var b = true

# class_a.gd
extends "res://class_2.gd"
export var a = "a"
export var a2 = "a2"

# class_2.gd
extends "res://class_1.gd"
export var three = 3
export var four = 4

# class_1.gd
extends Node
export var one = 1
export var two = 2

The only variable/class we should NOT see show up is class_b.gd with the single, un-exported b property with value true.

If I attach class_c.gd to a Node, I see this before/after:

If I have a builtin script with exported builtin1 and non-exported builtin2, then I get this before/after (script_test.tscn is the name of the scene and the built-in script extends a Node type):

If I have class_c open, and I change it to extend from class_2 instead of class_b, but I HAVEN'T saved it yet, then I will see this before/after (looks the same as the saved version, which was the goal):

In the course of doing this, I did find another small bug for the unsaved scripts (going from less derived to more derived base class in an unsaved script. In this case, I went from class_1 to class_a, so class_2 and class_a's variables are showing up under class_1`'s), so it seems it still needs some more work.

Edit: Oh wait! No, that's a purposeful "bug". If you ADD scripts and properties to the inheritance hierarchy, but the script hasn't been saved, the p_list ends up with all of these extra properties and doesn't yet know about the new scripts, so it just bundles them all underneath the most relevant script it can find. It gets revised when you actually save the script.

(after saving:)

Incidentally, if you add a script to a node that has nothing else in the script hierarchy, and the script is not yet saved (so it doesn't have a name yet), AND if it has export variables that show up in the Inspector, then it starts autonaming the category blocks with "Class 0", "Class 1", etc. for every script that isn't yet saved. If anywhere in the script hierarchy there is a saved script, then all of the unsaved scripts' properties are assumed to belong to the saved script, until those other scripts are saved. This is why we see the "buggy" behavior above. Here's what this algorithm results in for the objects with no saved scripts:

Regardless of whether this ends up as part of the 3.0 release or not, I might actually start extending this out into a full on overhaul of custom types in general, targeting a 3.1 release. This would be a critical change to have for a full custom type simulation across the engine.

[willnationsdev]

List::insert_before and List::insert_after used to not properly hook up the previous / following node to the new neighbor. Had been running into problems doing a forward loop after calling insert_before, but now it works.

[akien-mga]

You broke indentation all over the file :)
See http://docs.godotengine.org/en/latest/community/contributing/code_style_guidelines.html#using-clang-format-locally

[willnationsdev]

oh gosh. Yeah, thanks for this. Hopefully I can fix it up. XD

[willnationsdev]

I'm gonna have to make some modifications to this because I've encountered a bug where my code is crashing the engine.

[akien-mga]

It might be worth splitting your List fix in a separate commit and another PR so that it can be reviewed seprately.

[willnationsdev]

@akien-mga Is that a good idea if the code relies on it? I mean, if I separate it out, I won't be able to test the script changes locally without changing the code to not use insert_before at all. Would have to re-organize it into push_back uses and re-implement everything all over again.

[willnationsdev]

I suppose I could separate it and then keep the change, but just not have it recorded by git in the script-var-categories branch...

[akien-mga]

You can make that commit in a new branch and a new PR, and rebase this PR on that other branch (so you would have two commits here).

[willnationsdev]

oh, so that sounds good. Will get on that.

[akien-mga]

@willnationsdev Could you now rebase again since #13143 was merged but the related commit here has a different hash? (also you might want to remove the + of the commit log ;))

[willnationsdev]

Yes, I can do that later. I will likely be away from a computer for the next day or two. Will ping you when I have it ready.

[akien-mga]

No worry, I can also merge it manually myself when it's approved if need be.

[willnationsdev]

That works too. I can ping you early if I get a chance to work on it sooner rather than later. Otherwise, I'll see the merge notification. Thanks! :-)

[willnationsdev]

I still won't get to work on this until later, but keep in mind that currently, there seems to be an issue when you have a single script with only one exported variable. Something is causing the script_vars_start variable to be null in some cases. It ends up crashing the engine when you save the script after adding the export variable.

[mhilbrunner]

Can you test this with the new inspector?

[willnationsdev]

Maybe. I'm pressed for time in a lot of ways for the next two weeks, but if I get a chance I will.

[toger5]

How hard would it be to Introduce proper coustom class names.
So you can do 'extends MyCoustomClass' instead of: 'extends "res://path/to/file/myCoustomClass.gd''

[willnationsdev]

@Toger that's actually a project I've been working on for a while. Been trying to unify the custom type system API with the ability to register namespaces and typenames for scripts. There is a TypeDB issue I created about it before.

Basically, once it's done, you should be able to do extends Scripts.namespace.typename.

You'll also be able to register scenes as custom types, so there will be an analogous Scenes.namespace.typename access.

[toger5]

Nice!!! This is amazing and something i was hoping for since a long time.
How far could you go with providing type information? Would inheriting from a scene let the intellisense know about the node structure in it?

[willnationsdev]

I don't really know how the Intellisense works (haven't looked at it), but it's theoretically possible. All I will really be doing under the hood is keeping a HashMap of namespaces typenames to file paths for each resource. It will be a slash-delimeted typename mapped to a file path, stored in ProjectSettings. Then I am planning to detect the use of "Scripts" or "Scenes", begin parsing a dot-delimited set of names, take the whole of those names, replacing the dots with slashes, and then plugging it in to get the path to the appropriate resource, returning the loaded resource. This keeps us from having to constantly keep custom type resources loaded in memory.

Now, if the resource were a scene, then you would be able to get the SceneState of the PackedScene, analyze it's contents, and then fill an auto completion popup with the node paths in the scene to somehow get a reference to that node. You'd probably have to store the SceneState alongside the variable data so that it'd be accessible when attempting.pt to get a node from that variable.

[toger5]

Makes sense. I thought the custom type would become part of the classDB.
I think this is what the intellisense right now uses for querying functions/properties.

Are there anybissues when saving the coustom type to file connection in the ProjectSettings when moving files from one project to another to reuse them?

How hard would it be to use classDB.
Class db also could add the advantage of letting godot create a docfile for all your classes api. (Even functikn descriptikns could be added via comment. Which would be amazing for plugin documentation.)

[willnationsdev]

@toger5

I thought the custom type would become part of the classDB. I think this is what the intellisense right now uses for querying functions/properties.

How hard would it be to use classDB.

I would actually be much easier to use the ClassDB, and in fact, that's what I did at first. But reduz didn't want what should really only be editor-time type checking (for custom type reinforcement) and GDScript-module-specific parser/compiler changes affect the core of the engine (rightly so). So I refactored all the code out of the ClassDB and inserted it into a TypeDB in the EditorData class. This keeps it globally accessible through the EditorNode singleton, but keeps all of the dependencies strictly in the Editor or language-specific module space.

I've actually refactored the whole thing about 3 different times. That's the only reason it's taken so long to get this far. Started this project around the beginning of the year.

Are there anybissues when saving the coustom type to file connection in the ProjectSettings when moving files from one project to another to reuse them?

There are no issues with moving custom types. The EditorNode has a callback for when the filesystem is changed. It catalogues which files are new / have been modified, and I pass those files to the TypeDB which re-registers any existing files with updated typenames / file locations if those ever change.

Class db also could add the advantage of letting godot create a docfile for all your classes api. (Even functikn descriptikns could be added via comment. Which would be amazing for plugin documentation.)

Yes, this would be great! And luckily, as it turns out, you don't need the ClassDB to do any of that. There is a DocData class associated with the Editor that allows you to search a directory for XML files and generate documentation from those files. I plan on exposing this method to the EditorPlugin class so that users can directly have their plugin generate updated documentation for their content.

[toger5]

this all sounds really promising! Thank you! for taking the time to give me an insight on how it works.

I plan on exposing this method to the EditorPlugin class so that users can directly have their plugin generate updated documentation for their content.

maybe a second function for parsing doc information to DocData could be created which parses it directly from the script file itself. with custom comments which than represent the documentation for a fucniton/member. how feasible do you think this would be?

This Is also discussed in a different issue?

[willnationsdev]

@toger5

maybe a second function for parsing doc information to DocData could be created which parses it directly from the script file itself. with custom comments which than represent the documentation for a fucniton/member. how feasible do you think this would be?

You are probably looking for the annotations feature that has been discussed somewhere (I can't seem to find the related Issue(s)?). At one point, we were throwing around the idea of adding @-based annotations in GDScript so that you could add metadata about class content for documentation and autocompletion features. So, something like...

@param p_a: variant, The first parameter.
@param p_b: variant, The second parameter.
@return variant, The sum.
func add(p_a, p_b):
    return p_a + p_b

[willnationsdev]

@mhilbrunner I just tested this after a fresh pull / rebase onto master. Everything in my branch seems to be working as far as I can tell.

All this PR does is insert PROPERTY_USAGE_CATEGORY properties into the property_list before it gets passed to the Inspector. As long as the Inspector implementation is still evaluating the property list similarly to how it did before, then it should have no integration issues.

[willnationsdev]

@mhilbrunner Don't C# scripts allow for inheritance too? I may have to test this with a C# script as well, but I haven't messed around with that at all.

[neikeq]

get_base_script() is not yet implemented in C#.

[willnationsdev]

@neikeq ah, thanks. I'll have to do that.

[mhilbrunner]

As discussed in the IRC meeting, the idea behind this is good and welcome, but needs to be implemented in a solid way.

Notes from IRC:

reduz> I still think bojidar_bgs PR for script categories was the best solution to this problem
too bad it was closed
the approach was bad, but the idea was good
a keyword export_category "Something" would be great
Akien> I think we're starting to have a need for annotations support in GDScript finally :)
Akien>We could do stuff like:
#[category "Something"]
export var foo