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

Simplify scripting API "type" and "icon" data acquisition for scripts and scenes #103

Open
willnationsdev opened this issue Sep 25, 2019 · 4 comments

Comments

@willnationsdev
Copy link
Contributor

willnationsdev commented Sep 25, 2019

This is related to #68.

Describe the project you are working on:

I would like to make an Instantiation Pallete toolbar. That is, an EditorPlugin (that may one day be merged as a PR if lots of people like it? Who knows. I will definitely use it for my projects though) where users get a vertical toolbar on the left-hand side of the CanvasItem/Spatial viewport that displays frequently used, favorited, and/or suggested types and allows users to drag-n-drop them into the world. It would make for a much easier workflow than constantly flipping to/from the CreateDialog, searching, adding, then positioning, and going back again for a different type, repeatedly. Currently, it's a huge level design hassle. Anyway, that's not the problem I'm addressing today.

Describe how this feature / enhancement will help your project:

As I'm thinking about how I would design this plugin, I'm realizing that the only way to display the icon associated with a scene (if I wanted to drag-n-drop a scene too - which I do) would be to load the PackedScene resource in its entirety, then get the root node, and then check if it has a script / get the icon for the type, etc. BUT, I don't want to have to load every single scene in my project every single time the user updates the file system or brings the editor into focus, etc. I would much rather have the information cached somewhere and accessible from script code.

In addition, the workflow for gathering information about a script class is extremely cumbersome on the scripting side whereas there are plenty of ways to get this information inside the engine:

Access to this information would need to be exposed to the scripting API.

In addition, to prevent users from having to load scenes to get at the information they need, the EditorFileSystem would need to start caching root node information inside just like it does for script class name/icon/extends. You could probably even re-use the same fields - though you'd have to update the API and refactor things; you might include a bool as to whether it's a script class, a scene, or neither.

Show a mock up screenshots/video or a flow diagram explaining how your proposal will work:

So, anytime someone changes the filesystem, I have to check to see if they've updated any of the script class names or icons, as well as see if the icon associated with a scene has also updated (based on its root node). Ideally, I wouldn't have to load any resources to get this information. I should be able to get everything I need just from the file path and by querying cached data hidden behind singletons/facades/etc. Keep in mind, much of this logic must be executed for every single script/scene file I iterate over. For larger projects, this could become obscenely slow.

Get the name of a script

Before: O(N) time where N is the number of script classes in the project.

var filepath := get_from_somewhere()
var name := ""
var script_classes := ProjectSettings.get_setting("_global_script_classes") as Array
for a_class in script_classes:
    if a_class.path == filepath:
        name = a_class.class
        break

After: O(1) time based on a single HashMap lookup.

var filepath := get_from_somewhere()
var name := ScriptServer.get_global_class_name(filepath) # relies on changes from #68.

Get the icon for a script

Before: O(N) time where N is the number of script classes in the project. Also adds two HashMap lookups.

var filepath := get_from_somewhere()
var name := ""
var script_classes := ProjectSettings.get_setting("_global_script_classes") as Array
for a_class in script_classes:
    if a_class.path == filepath:
        name = a_class.class
        break
var script_class_icons := ProjectSettings.get_setting("_global_script_class_icons") as Dictionary
var script_icon := script_class_icons[name] as Texture

After: O(1) time based on a constant number of HashMap lookups (at least, once custom types are deprecated and removed).

var filepath := get_from_somewhere()
var name := ScriptServer.get_global_class_name(filepath)
var icon := get_editor_interface().get_class_icon(name) # from EditorNode

get the icon for a scene

Before: O(N + M) time. Performance is dependent on the number of properties on the nodes (N), the number of script classes that exist (M).

var filepath := get_from_somewhere()
var scene := load(filepath) as PackedScene
var state := scene.get_state()
var the_type := state.get_node_type(0)
var the_script: Script = null
var the_icon: Texture = null
for i in range(state.get_node_property_count(0):
    if (state.get_node_property_name(0, i) == "script":
        the_script = state.get_node_property_value(0, i)
        break
if not the_script:
    the_icon = get_editor_interface().get_base_control().get_icon(the_type, "EditorIcons") # done!
else:
    var script_classes := ProjectSettings.get_setting("_global_script_classes") as Array
    for a_class in script_classes:
        if a_class.path == filepath:
            name = a_class.class
            break
    var script_class_icons := ProjectSettings.get_setting("_global_script_class_icons") as Dictionary
    the_icon = script_class_icons[name] as Texture # done! (finally)

After: O(1) time based on two HashMap lookups (get cache, lookup in Dictionary).

var filepath := get_from_somewhere()
var file_cache := get_editor_interface().get_editor_filesystem().get_file_cache(filepath) as Dictionary
var icon := load(file_cache.class_icon) as Texture

Describe implementation detail for your proposal (in code), if possible:

  1. Add a method to get the name of a script by filepath in ScriptServer (included in Add script class utility methods to ScriptServer #68).
  2. Add the EditorNode get_class_icon and get_object_icon methods to the EditorInterface class.
  3. Create a script-accessible method to fetch a FileCache struct as a Dictionary from the EditorFileSystem
    1. The returned Dictionary would have an additional field for the actual icon Texture that already figures out for us what the appropriate class icon is based on the "extends" and the "icon_path" values.
    2. You would likely need to refactor the names to go from script_class_name, script_class_extends, etc. to just be class_name, class_extends, etc. if they start describing information for any arbitrary root node in a scene. Icon paths for non script types would just be empty.
  4. A script-accessible _ScriptServer singleton would need to be registered to the ClassDB to expose access to the ScriptServer utility methods.

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

There is no performance-sensitive means by which to do this in the scripting API. This proposal is about updating the scripting API's capabilities to accommodate the performance problems that can occur at scale.

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

It requires direct modifications to the engine core (expose ScriptServer) and the editor (EditorNode icon methods, EditorFileSystem FileCache exposure).

@Calinou Calinou changed the title Simplify scripting API "type" and "icon" data acquisition for scripts and scenes. Simplify scripting API "type" and "icon" data acquisition for scripts and scenes May 4, 2020
@Shatur
Copy link

Shatur commented Feb 11, 2021

Get the icon for a script
Get the name of a script

I also found that NativeScript have script_class_name and script_class_icon_path properties. Maybe it would be nice to have same methods in GDScript class too? This would allow to just use MyClass.script_class_icon_path and MyClass.script_class_name.

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Feb 11, 2021

While it isn't as good for usability as the above proposal, godotengine/godot#44879 introduces a ScriptServer singleton that has methods to expose access to all ScriptServer information about classes (the class name, language, resource path, and base or native type). The icon however is tracked by the Editor. To get that, you'd either have to a Control with the Editor's theme to get the icon or you'd have to explicitly get ProjectSettings value, as other linked Issues cover.

I think Script and PackedScene methods to acquire this information would improve usability a lot.

Edit: Now that I look at the proposal in-depth, my PR already does a lot of it, save for the icon bit. The main missing thing is the Script/PackedScene bindings for methods that would go through the ScriptServer for the name and the ProjectSettings for the icon.

@ripperdave
Copy link

@willnationsdev thanks for the details.
This scenario is related to the proposal. It would be great to reach easily such core information.

I import many full game resource to my game launcher project with ProjectSettings.load_resource_pack("res://fullGame1.pck", true)
The game launcher supposed to start many Godot games as part of a collection.

load_resource_pack() won't import class_name data from pck. That is ok. It is a strange use-case as Project settings are imported.

So I added global script class information manually.
Godot won't consider this information. Cannot resolve/parse class names.
You might have some hints. I might refresh ScriptServer somehow from GDScript.

func load_config():
	var config = ConfigFile.new()
	
	var err = config.load("res://settings.tres")
	assert(err == OK)
	
	if config.has_section_key("","_global_script_classes"):
		var gsc = config.get_value("","_global_script_classes")
		ProjectSettings.set_setting("_global_script_classes", gsc)

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Aug 25, 2021

@ripperdave As it is designed currently, the global variables that are present in GDScript are all setup during the initialization of the GDScriptLanguage singleton which happens during the initialization of the Godot Engine app process overall. That is, at app startup, ProjectSettings is loaded, ProjectSettings is read by ScriptServer which initializes the script classes inside it, and then GDScriptLanguage boots up, reads from ScriptServer, and sets up global variables, before ever parsing/loading any GDScript files. As a result, you can't add global variables to the GDScriptLanguage after the app startup has already completed.

With the changes my PR will make, you would be able to look up a script by name and get the script or create an instance of it, but you wouldn't be able to just statically reference the script name with a direct language symbol and have the GDScript language recognize it. To do that, you'd have to make changes to the GDScriptLanguage singleton so that it straight up never used its built-in global variable system to handle script classes. It would instead need to add a manual check against the ScriptServer for globally defined types whenever it attempts to analyze an identifier/symbol in the script code (so a separate case statement in the switch that handles parsing of identifiers within a block of code). The global script identifiers are currently handled in the same case statement that evaluates global variables because....they are global variables in GDScript at the moment. So, rather than importing the global classes at startup and making them global variables, it would just constantly be doing fresh queries against the ScriptServer whenever a script was evaluated (which isn't really a significant cost or anything I don't think - it's just programmed differently than how it is now).

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