godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.17k stars 98 forks source link

Add script class utility methods to ScriptServer #68

Open willnationsdev opened 5 years ago

willnationsdev commented 5 years ago

Describe the project you are working on: I am attempting to implement #18, #22, and #43 which involve changes to both /editor and various /modules/* directories.

Describe how this feature / enhancement will help your project: None of those codebases should be dependent on each other. All should only be aware of the engine core (/core, /scene, etc.). I am finding myself duplicating certain script-class-related algorithms across EditorData / other editor classes and the various ScriptLanguage implementations in modules. It would simplify things greatly if all of these utility algorithms could be sourced from the actual core class that is responsible for managing script classes, so the code can be shared between several contexts (which, ya know, would actually make sense).

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

Thus far I have code in the following places that has been duplicated:

/modules/gdscript/gdscript_parser.cpp
/modules/visual_script/visual_script.cpp
/modules/visual_script/visual_script_nodes.cpp
/modules/editor/create_dialog.cpp
/modules/editor/editor_properties.cpp
/modules/editor/editor_data.cpp

I haven't even approached C# script classes, nor tried to create a global access point for script class references in NativeScript, both of which could benefit from the utility algorithms I'm using.

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

Add the following three utility methods to ScriptServer:

// Loop through the available languages and get the name/base_type/icon_path
// based on a given path.
// Edit: Would probably want to have this code also determine the type based on p_path
// and add an if check for `lang->handles_global_class_type(the_type)`
StringName ScriptServer::get_global_class_name(const String &p_path, String *r_base_type = NULL, String *r_icon_path = NULL) {
    for (int i = 0; i < get_language_count(); i++) {
        ScriptLanguage *lang = get_language(i);
        StringName class_name = lang->get_global_class_name(p_path, r_base_type, r_icon_path);
        if (class_name != StringName()) {
            return class_name;
        }
    }
    return StringName();
}
// Given a name, give the script.
// This single line of code happens very frequently.
// It should be centralized and an error should be reported anytime something
// would go wrong here.
Ref<Script> ScriptServer::get_global_class_script(const StringName &p_class) {
    ERR_FAIL_COND_V(!global_classes.has(p_class), NULL);
    Ref<Script> script = ResourceLoader::load(ScriptServer::get_global_class_path(p_class), "Script");
    ERR_FAIL_COND_V(script.is_null(), NULL);
    return script;
}
// The ClassDB::instance equivalent for ScriptServer's script classes.
// The editor's CreateDialog and soon-to-be EditorPropertyResource's PopupMenu
// as well as a VisualScript graphnode that I'm working on would all make use of this 
// functionality (could also make it available to NativeScript in the future and a planned 
// GDScript global function to class/script-class agnostically instantiate a type by name). 
// Several use cases spread across the source code. Likewise, it would be better for it
// to be centralized with explicit error messages appearing at each step of the process.
Object *ScriptServer::instantiate_global_class(const StringName &p_class) {
    ERR_FAIL_COND_V(!global_classes.has(p_class), NULL);
    String native = get_global_class_native_base(p_class);
    ERR_FAIL_COND_V(!ClassDB::class_exists(native), NULL);
    Object *obj = ClassDB::instance(native);
    Ref<Script> script = ScriptServer::get_global_class_script(p_class);
    ERR_FAIL_COND_V(script.is_null(), NULL);
    obj->set_script(script.get_ref_ptr());
    return obj;
}

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

It is engine related, not script related.

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

It is engine related, not script related.

astrale-sharp commented 4 years ago

Im supporting this, this would be great!

sairam4123 commented 3 years ago

This issue is blocking me from implementing saves, because currently, I can't get all scripts and call a function.

willnationsdev commented 3 years ago

@sairam4123 Just so that you are aware, PRs godotengine/godot#44879 (for 3.x) and godotengine/godot#48201 (for 4.0) include this change.

Also, implementation of these methods should not be a "blocker" of any sort. This data is already accessible from the ProjectSettings singleton via hidden properties in the project.godot file.

You can see the serialization process, and which variables are being written to for which purposes, here. You can then see the way the data is deserialized back into the ScriptServer here. You can replicate this code in GDScript easily enough. For example, Godot Next has a ClassType script which has utility methods to fetch this information. This method for example fetches all of the global script classes and binds their name to a Dictionary containing all of their information (such as "language" name, file "path", "class" name, and "base" class name), so you can, iirc, load the script using load(script_map["MyClass"]["path"]), etc.

If you need to iterate through all scripts in the res:// directory rather than just the global script classes, then you can do that yourself using the File class, as Godot Next also demonstrates in its FileSearch class.