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

TypeDB expansion of ClassDB, including ScriptDB/SceneDB, with auto-registration of runtime assets. #17387

Closed
willnationsdev opened this issue Mar 9, 2018 · 14 comments

Comments

@willnationsdev
Copy link
Contributor

willnationsdev commented Mar 9, 2018

Please search existing issues for potential duplicates before filing yours:
https://github.com/godotengine/godot/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+script+identifier

https://github.com/godotengine/godot/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+script+inheritance+view

Godot version:
Godot 3.1+

Issue description:
I've been doing some work on a ScriptDB to solve problems with (#6067), but I think I've stumbled onto an even more powerful, extended solution that would allow for a lot more functionality.

The ClassDB grants C++ classes really powerful, integrated functionality for Godot Engine. C++ classes have "first-party support", such as...

  1. having identifiers for them automatically generated and accessible to the scripting API.
  2. representing types of all kinds (not just Node or Resource, but also Object, Reference, and everything else).
  3. making an entire scene of nodes be represented as a single Node in the engine (for example, when looking at the Tree node in the remote debug scene tree, it has several child nodes generated by the engine).
  4. able to pull up an inheritance view of Node and Resource types in the Editor.

However, all of this is only supported for types defined at compile time, that are part of the ClassDB.

Our closest approximation of this, custom types, do not currently support any of this functionality.

  1. No identifiers or namespaces are created for custom types (this would be a scripting language-specific issue, so it isn't appropriate to solve it with the ClassDB).
  2. The Editor will add custom types to the Node and Resource creation tools, but because the Editor doesn't have (or need) similar tools for other types, there's nothing to do here for custom types that extend those other C++ classes. That is, a script won't be able to exploit point 1 because the Editor doesn't need it, not because games don't need it.
  3. Custom types allow you to bind scripts, not scenes, to a name (and the name is only pertinent to the creation tools from point 2, so it doesn't actually give us point 1 either).
  4. The custom types support does not currently give inheritance between custom types, only the inheritance from the immediate relationship to the C++ type. Now, this is actually a simple bug that could be fixed, so it's not quite as big of an issue. However, even if you were to do this, you still cannot define abstract types that aren't instance-able, but which still show up in the inheritance hierarchy (things like BaseButton).

Also, each custom type must be manually registered in order for custom type functionality to be engaged in the first place. There are other problems with the custom type system (as mentioned in the other Issue), and while some parts of that Issue overlap, the concept of creating script constraints for Object types (and the possible ramifications thereof) isn't the subject of this Issue.

I did attempt to create a runtime-generated (so no manual registration required) substitute for points 2 and 4 with the creation of a plugin for Godot, the Inheritance Dock, but it has to deal with a core problem that doesn't scale well: in order to view the inheritance information about any given script or scene, you have to load the resource into memory. Because it rebuilds the entire Tree in the dock every time the filesystem is changed, and checks the inheritance of every single file by loading them all into memory every single time, there is an immense amount of work being done for very little gain that is exacerbated as the size of project increases. Ideally, we would be able to check this type/inheritance information without ever having to load any files.

Proposed solution:
Create a TypeDB class that is analogous to the ClassDB, but which is purely for storing

  1. filepath
  2. namespace
  3. typename (an UpperCamelCase rendition of the file name)
  4. inherited type's data (a pointer to its filepath, namespace, and typename)
  5. inherited type's name (in cases where there is no runtime-defined parent for the type, this would be the name of the ClassDB entry). This allows us to preserve inheritance hierarchies across the various DBs.

The TypeDB would be a separate .h/.cpp in core that includes class_db.h/.cpp and script_language.h/.cpp. It would mimic much of ClassDB's methods for CRUD-ing type and inheritance information, but it would function more like GDScript's global_map, just creating a two-way mapping (so 2 HashMaps) between the namespace+typename and the filepath.

Assuming much of the TypeDB's functionality is defined via macros (like Object), it could then define a ScriptDB and SceneDB in the same file with common functionality that grants localized storage for both scripted types and scene types. Calling the common functions on the TypeDB would then aggregate the data from ClassDB, ScriptDB, and SceneDB.

Types' full names could mimic filepaths with slash delimiters (/). A front-facing slash could be used to refer to non-C++ types defined in the local project (not part of a plugin), e.g. "/MyType".

# filesystem, res://
panel_container.gd # editor auto-registers /PanelContainer in ScriptDB.
panel_container.tscn # editor auto-registers /PanelContainer in SceneDB.
addons/my-plugin/panel_container.tscn # editor auto-registers /MyPlugin/PanelContainer in SceneDB.

# some .gd file
func _ready():
    TypeDB.type_exists("/PanelContainer", TYPE_CPP & TYPE_SCRIPT & TYPE_SCENE) # returns true
    # ^ automatically returns true for CPP types if the name is the same, like a fallback
    TypeDB.instance("/PanelContainer", TYPE_*) # instances the desired type. Returns null if multiple types requested.
    TypeDB.instance("/PanelContainer", [TYPE_SCENE, TYPE_SCRIPT, TYPE_CPP]) # instances a scene if it exists. If not, then a script. If not, then a C++ type.  If not, then reports an error.
    
    # Because things are registered, typenames can now be directly accessed
    PanelContainer.new() # returns C++ PanelContainer instance, no change from before
    var type = /PanelContainer # returns an instance of a TypeRecord C++ type that provides an interface for interacting with the "type" regardless of whether it knows about a C++ type, script or scene.
    # if panel_container.gd/.tscn weren't defined, then this would default to the C++ type again (so it has an in-engine fallback)
    var type2 = /MyPlugin/PanelContainer # gets the TypeRecord for the file at res://addons/my-plugin/panel_container.gd/.tscn.
    
    # can have fetching and instantiation follow a scene > script > C++ type priority
    type.create() # returns PackedScene.instance() if a scene exists, else Script.new(), else ClassDB.instance(typename). Else null
    type.new() # returns load("panel_container.gd").new() or, if there is no script, ClassDB.instance(typename). Else null
    type.instance() # returns load("panel_container.tscn").instance(). Else null.
    type.fetch(TYPE_*) # returns null in null/TYPE_CPP cases, Script and PackedScene for the others
    type.fetch([TYPE_SCENE, TYPE_SCRIPT]) # returns PackedScene or, if null, Script, etc. (maybe this is the default value?)

With something of this nature, the editor could then incorporate a Type Dock that supplies an inheritance view of ALL types, WITHOUT having to explicitly load any resources (solving #13357). From the dock, users could then open, instantiate, or extend any of the types (for which those actions are possible) by clicking on icons in the row, similar to the Scene dock.

In addition, the "Add a node" window could display C++ types, scripts, and scenes (based on the root node), providing some kind of visible differentiation between each (color coded? icons? etc.) along with allowing users to define editor metadata to be associated with the associated DB (an icon, a brief description, a class API XML file, etc.). This would then also enable a nearly automatic fix for #17093 by changing the editor's code to refer to the TypeDB rather than the ClassDB.

Anyway, just throwing thoughts out there. Good/bad ideas? All of it, some of it? Please let me know since I already have about half of the ScriptDB implemented. XD

Edit:

Also, because I know it'll come up, I recognize that namespacing and identifiers are generally not a concern for non-GDScript/non-VisualScript languages, so one might claim that this is a scripting-language specific problem. And that is true for the registration of scripts, but if you used this technique and then exposed the TypeDB to the in-engine scripting API, then you'd even be able to share namespaces between all languages by unifying them in the TypeDB (perhaps handle script registrations slightly differently, letting them specify a language to clarify any inter-language typename conflicts?). Not in a literal fashion of course (a C# script isn't gonna suddenly access MyScript class defined in my_script.gd), but through TypeDB.fetch(name), you could start to approximate it.

It's also possible that we may have different ideas about how an individual script could be auto-registered (flags itself or its subclasses as exported, declares a particular namespace / tyepname for itself or subclasses, etc.).

@willnationsdev
Copy link
Contributor Author

Seems this is is also related to solving #7402 . Gotta thank karroffel for bringing it to my attention.

@reduz
Copy link
Member

reduz commented Mar 9, 2018 via email

@reduz
Copy link
Member

reduz commented Mar 9, 2018 via email

@willnationsdev
Copy link
Contributor Author

@reduz Well, all of the namespacing and typename stuff from 7402 and this COULD be done in the Editor. We'd just have to have the editor auto-replace the typenames and such with paths to the script files during compilation of the scripts so that the runtime content is the same as what it currently is.

However, the 6067 proposal to introduce script constraints for Objects that are custom types WOULD require core changes (just add getter/setter for a custom_script property and modify set_script/set_script_and_instance to take it into account). If you don't do that, then it defeats the whole purpose of defining custom types in the first place.

@willnationsdev
Copy link
Contributor Author

So the question comes down to whether you want to allow script constraints or not. If not, then the entire concept of custom types can be done away with entirely when the related Editor changes are made and scripts are registered automatically at design-time.

@reduz
Copy link
Member

reduz commented Mar 9, 2018 via email

@willnationsdev
Copy link
Contributor Author

Hmm..ok, understood.

Any possibility of a plugin approach to any of this seeming like a problem to you? For example, if GDScript left it open for a plugin to define identifiers as well (would just need to expose the global_map, if I understand correctly).

I don't intend any sort of disrespect. I earnestly just want to have confidence as to whether I should trash this idea entirely, or if there is some way I can get the editor to allow for the functionality I'm seeking from it, without having to resort to just independently developing a fork of the engine of my own that supports it (for personal use). Would really rather not do all that work and have it only benefit me cause I know there are many others interested in the feature(s).

@reduz
Copy link
Member

reduz commented Mar 9, 2018

When you approach a problem, you first should discuss the use case, not go straight to a solution. I still don't see an use case or even a problem for this, so discussing a solution imo is not what should be going on here

@willnationsdev
Copy link
Contributor Author

Well, the "problem" in this case is more of a usability / convenience thing. It's not that people can't use file paths. They are simply inconvenient and that's it. And this is simply solving the inconvenience by mapping file paths to names with namespaces to prevent collisions.

@reduz
Copy link
Member

reduz commented Mar 10, 2018

This is not an usability/convenience thing. You rarely assign scripts to nodes, so all this work is not justified.

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Mar 31, 2018

FYI, what I've been doing for this.

  1. EditorData now has TypeDB class. It stores information about all types defined in a project (and not just custom types, but all manner of scripts and scenes). Type names are automatically assigned based on PascalCase-ifying the basename of a file (scenes get Scn appended to their type names). Type names are namespaced one level under a plugin name (if they are included in /addons/some_plugin/) or the project name. Each entry in the TypeDB holds the path, name, namespace, and its inherited asset's entry and namespace+name (if any).

  2. If a script/scene is a custom type defined by a plugin, then the TypeDB entry includes the script/scene Ref, whether or not its abstract (can be inherited but can't be instantiated from the TypeDB), and a link to any XML documentation it references. The fact that it only includes the script/scene if it's a custom type means that we don't force the engine to store a loaded reference to every script/scene constantly.

  3. For creating references to ALL scripts/scenes in GDScript safely (no constant loaded references), I plan to have the engine create a global "GD" GDScript instance to be stored in the .godot file. When using the editor, upon updates to the filesystem's scripts/scenes, it can update the source code for the script global by generating its content from the TypeDB's records. An example of what it might look like...

extends Node

const API = {}

class MyPlugin:
    var MyNode = "res://addons/my_plugin/my_node.gd" setget set_MyNode, get_MyNode
    func set_MyNode(p_value): MyNode = p_value
    func get_MyNode(): return load(MyNode)
    
    var MyNodeScn = "res://addons/my_plugin/my_node.tscn" setget set_MyNodeScn, get_MyNodeScn
    func set_MyNodeScn(p_value): MyNodeScn = p_value
    func get_MyNodeScn(): return load(MyNodeScn)

class MyProject:
    var MyNode = "res://my_node.gd" setget set_MyNode, get_MyNode
    func set_MyNode(p_value): MyNode = p_value
    func get_MyNode(): return load(MyNode)

func _ready():
    API["MyProject"] = MyProject.new()
    API["MyPlugin"] = MyPlugin.new()

An example usage in a regular GDScript file might be...

# sample.gd
extends Node
func _ready():
    # I want to get an instance of the types without name collision
    var project_node = GD.API.MyProject.MyNode.new()
    var plugin_node = GD.API.MyPlugin.MyNode.new()
    # I want to change which script is associated with the type
    GD.API.MyPlugin.MyNode = "res://path/to/new/script.gd"

What do you guys think?

@ghost
Copy link

ghost commented Apr 1, 2018

From the perspective of a lurker, it looks like this issue has turned from solving a relatively straightforward problem (nicely summed up by Akien) to adding a set of features.

The original issue is quite a specific scope, and reduz suggested an elegant fix (third paragraph). Occam's Razor suggests the simplest solution is nearly always the best one. I think this fix is the simplest and clearest. It can be implemented to solve the current problem. Then the new features being suggested can be opened separately, and discussed from the perspective of adding a new, independent feature. Currently, the discussion is about adding features as a fix to a problem, which IMHO is backwards.

Just my two cents.

@willnationsdev
Copy link
Contributor Author

This is effectively what I am doing @RodeoMcCabe, only this Issue is essentially my personal tracking for a variety of other related issues, and I'm creating a single foundation that will be able to support each of them. In addition to the things akien mentioned, there is also...

  1. Allowing users to reference scripts by name rather than by filepath in GDScript (and not just custom types, but to have the editor automatically bind a name to any created script or scene).

  2. Having the editor prevent the user from breaking a custom type script inheritance hierarchy on a node created with that script (does not block it in real-time).

  3. Allowing users to define custom type scenes in addition to Individual nodes.

  4. Allowing users to define XML documentation for their custom types.

  5. Allowing users to define whether a custom type is abstract and having that affect the CreateDialog's displayed content.

  6. Fixing a bug that prevents the custom type icon from updating when you change the type of a node (or even when you remove the custom type script currently, though that will become obsolete when these changes are made).

The most efficient way to handle points 1 and 2 is to cache the names and inheritance hierarchies of files as they are edited (so that we don't re-load every script and scene in the entire resource folder every time the file system changes). Hence, the TypeDB. And by default, the only thing it stores is the name, namespace, and inheritance hierarchy. It only stores an actual script/scene or documentation path when you specifically register them manually in an EditorPlugin.

The complexity here is due to a need for optimization. And I know it's needed because I've basically already attempted to track inheritance hierarchies in the file system with my Inheritance Dock plugin, and it suffers from the same problem. Constant tly reloading every script/scene to re-evaluate its Inheritance relationships is very costly, and furthermore, if a file has errors on it, then you can easily start flooding the console with repetitive, undesirable error logs since every time you save or move one file, every file gets re-evaluated.

To solve THAT problem, setting up the TypeDB really is the easiest and most effective solution.

@willnationsdev
Copy link
Contributor Author

Based on the introduction of script classes, the future addition of annotations in GDScript, and the eventual ability to generate documentation data for a script (which I believe might be planned), there isn't really any reason to keep up this stuff. Also...

Based on the discussion in #21187, there is no need to continue working on this. Anything that can assist both the editor (can't be a module, must be in core or editor) and runtime (can't be in editor) would have to be in core, and any and all core changes that would enable a user-defined type system will not be approved by reduz.

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