`take_over_path` and extending the same named class twice

[KANAjetzt]

Godot version

v4.1.2.stable.official [399c9dc39] v4.2.beta1.official [b1371806a]

System information

Godot v4.1.2.stable - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3080 (NVIDIA; 31.0.15.3699) - AMD Ryzen 7 5800X 8-Core Processor (16 Threads)

Issue description

func _init() -> void:
    var extend_0 = load("res://extends/extend0.gd")
    extend_0.new()
    extend_0.take_over_path("res://base.gd")
    var extend_1 = load("res://extends/extend1.gd")
    extend_1.new()
    extend_1.take_over_path("res://base.gd")

Running this code causes a game crash with no error message in the editor output.

The following error is printed if the editor is run with the console:

ERROR: FATAL: Condition "!exists" is true.
   at: operator[] (./core/templates/hash_map.h:504)

This pattern was used to build an "inheritance chain" for modding in Godot 3.5.

In Godot 4.1, after loading the first script and taking over the path of the named class, it crashes the game as soon as the second script that extends the same-named class is loaded, in the above example:

var extend_1 = load("res://extends/extend1.gd")

This all works as expected if the class_name Base is removed from res://base.gd.

Steps to reproduce

1. Create a script with a class_name:

   
   class_name Base
   extends Node

func _ready() -> void: print("base")

func do_stuff() -> void: print("do_stuff")

2. Create 2 scripts that extend the named class:

```gdscript
# extend0.gd
extends "res://base.gd"

func _ready() -> void:
    super._ready()
    print("extend0 - _ready()")

func do_stuff() -> void:
    super.do_stuff()
    print("extend0 - do_stuff()")

# extend1.gd
extends "res://base.gd"

func _ready() -> void:
    super._ready()
    print("extend1 - _ready()")

3. Create an autoload with the following code:

func _init() -> void:
    var extend_0 = load("res://extends/extend0.gd")
    extend_0.new()
    extend_0.take_over_path("res://base.gd")
    var extend_1 = load("res://extends/extend1.gd")
    extend_1.new()
    extend_1.take_over_path("res://base.gd")

Minimal reproduction project description

res://base.gd

• The named class (class_name Base) that is extended and taken over by res://extends/extend0.gd and res://extends/extend1.gd.
• The class_name Base can be commented out to toggle between working and not working.

res://extends/extend0.gd and res://extends/extend1.gd

• These scripts can be viewed as two mods that want to modify the same vanilla game script.

res://singletons/extender.gd

• An autoload that handles the extending of the script. Each "script extension" is loaded, and take_over_path is used to take over the base script.

res://VanillaThing.gd

• Represents a vanilla game script that calls the modified do_stuff method on the Base class.
• If everything works as intended, calling do_stuff from here should print:

do_stuff
extend0 - do_stuff()

Minimal reproduction project

Script Extending.zip

[KANAjetzt]

https://github.com/godotengine/godot/blob/09946f79bd8215b2c6332de8821737580909a91c/modules/gdscript/gdscript.cpp#L1092-L1125

After some digging, it looks like find_class is returning null when preparing to compile extend1.gd.

When the class_name is commented out in base.gd, first is set to U"res://base.gd". In this case, it enters the following block of code:

else if (p_qualified_name.begins_with(get_root_script()->path)) {
    // Script path could have a class path separator("::") in it.
    class_names = p_qualified_name.trim_prefix(get_root_script()->path).split("::");
    result = get_root_script();
}

With the class_name in base.gd, first is set to U"Base". It's looking for "Base" in extend0.gd, where the global_name is (obviously) [empty], causing it to run through without finding anything.

Of course, I have a very limited understanding of the code base, so I'm not sure if this observation tracks.

I was able to fix the MRP with this dodgy code:

GDScript *GDScript::find_class(const String &p_qualified_name) {
    String first = p_qualified_name.get_slice("::", 0);
    String second = path.get_slice("::", 0);

    Vector<String> class_names;
    GDScript *result = nullptr;
    // Empty initial name means start here.
    if (first.is_empty() || first == global_name) {
        class_names = p_qualified_name.split("::");
        result = this;
    } else if (p_qualified_name.begins_with(get_root_script()->path)) {
        // Script path could have a class path separator("::") in it.
        class_names = p_qualified_name.trim_prefix(get_root_script()->path).split("::");
        result = get_root_script();
    } else if (second.begins_with(get_root_script()->path)) {
        class_names = second.trim_prefix(get_root_script()->path).split("::");
        result = get_root_script();
    }

    [...]

[KoBeWi]

Stack trace for the crash:

CrashHandlerException: Program crashed
Engine version: Godot Engine v4.2.beta.custom_build (f497156e0b37fc4c33ce11c285a8b318b319f7cc)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[0] HashMap<StringName,int,HashMapHasherDefault,HashMapComparatorDefault<StringName>,DefaultTypedAllocator<HashMapElement<StringName,int> > >::operator[] (C:\godot_source\core\templates\hash_map.h:538)
[1] GDScriptCompiler::_prepare_compilation (C:\godot_source\modules\gdscript\gdscript_compiler.cpp:2650)
[2] GDScriptCompiler::compile (C:\godot_source\modules\gdscript\gdscript_compiler.cpp:3183)
[3] GDScript::reload (C:\godot_source\modules\gdscript\gdscript.cpp:772)
[4] GDScriptCache::get_full_script (C:\godot_source\modules\gdscript\gdscript_cache.cpp:303)
[5] ResourceFormatLoaderGDScript::load (C:\godot_source\modules\gdscript\gdscript.cpp:2699)
[6] ResourceLoader::_load (C:\godot_source\core\io\resource_loader.cpp:261)
[7] ResourceLoader::_thread_load_function (C:\godot_source\core\io\resource_loader.cpp:319)
[8] ResourceLoader::_load_start (C:\godot_source\core\io\resource_loader.cpp:499)
[9] ResourceLoader::load (C:\godot_source\core\io\resource_loader.cpp:416)
[10] GDScriptUtilityFunctionsDefinitions::load (C:\godot_source\modules\gdscript\gdscript_utility_functions.cpp:241)
[11] GDScriptFunction::call (C:\godot_source\modules\gdscript\gdscript_vm.cpp:2133)
[12] GDScript::_create_instance (C:\godot_source\modules\gdscript\gdscript.cpp:180)
[13] GDScript::instance_create (C:\godot_source\modules\gdscript\gdscript.cpp:396)
[14] Object::set_script (C:\godot_source\core\object\object.cpp:906)
[15] Node::set_script (C:\godot_source\scene\main\node.cpp:3573)
[16] Main::start (C:\godot_source\main\main.cpp:3192)
[17] widechar_main (C:\godot_source\platform\windows\godot_windows.cpp:179)
[18] _main (C:\godot_source\platform\windows\godot_windows.cpp:204)
[19] main (C:\godot_source\platform\windows\godot_windows.cpp:218)
[20] WinMain (C:\godot_source\platform\windows\godot_windows.cpp:232)
[21] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[22] <couldn't map PC to fn name>
-- END OF BACKTRACE --

[KANAjetzt]

Two MRPs that demonstrate that only one take_over_path is needed to cause this crash.

Script Extending.zip

• base.gd has the class_name Base
• vanilla_thing_extends_base.gd extends Base
• extends/extend0.gd extends "res://base.gd" or extends Base ( same result )
• extender.gd is the autoload that loads extend0.gd and executes extend_0.take_over_path("res://base.gd")

Stack trace

``` CrashHandlerException: Program crashed Engine version: Godot Engine v4.3.dev.custom_build (780e1a50408360cf0cf93c0b55b59e9d7b4ad0b1) Dumping the backtrace. Please include this when reporting the bug to the project developer. [0] GDScriptCompiler::_prepare_compilation (C:\Users\Kai\Documents\godot\godot\modules\gdscript\gdscript_compiler.cpp:2661) [1] GDScriptCompiler::compile (C:\Users\Kai\Documents\godot\godot\modules\gdscript\gdscript_compiler.cpp:3198) [2] GDScript::reload (C:\Users\Kai\Documents\godot\godot\modules\gdscript\gdscript.cpp:803) [3] GDScriptCache::get_full_script (C:\Users\Kai\Documents\godot\godot\modules\gdscript\gdscript_cache.cpp:347) [4] ResourceFormatLoaderGDScript::load (C:\Users\Kai\Documents\godot\godot\modules\gdscript\gdscript.cpp:2863) [5] ResourceLoader::_load (C:\Users\Kai\Documents\godot\godot\core\io\resource_loader.cpp:268) [6] ResourceLoader::_thread_load_function (C:\Users\Kai\Documents\godot\godot\core\io\resource_loader.cpp:326) [7] ResourceLoader::_load_start (C:\Users\Kai\Documents\godot\godot\core\io\resource_loader.cpp:529) [8] ResourceLoader::load (C:\Users\Kai\Documents\godot\godot\core\io\resource_loader.cpp:446) [9] GDScriptUtilityFunctionsDefinitions::load (C:\Users\Kai\Documents\godot\godot\modules\gdscript\gdscript_utility_functions.cpp:241) [10] GDScriptFunction::call (C:\Users\Kai\Documents\godot\godot\modules\gdscript\gdscript_vm.cpp:2161) [11] GDScriptInstance::callp (C:\Users\Kai\Documents\godot\godot\modules\gdscript\gdscript.cpp:1978) [12] Node::_notification (C:\Users\Kai\Documents\godot\godot\scene\main\node.cpp:213) [13] Object::notification (C:\Users\Kai\Documents\godot\godot\core\object\object.cpp:902) [14] Node::_propagate_ready (C:\Users\Kai\Documents\godot\godot\scene\main\node.cpp:269) [15] Node::_propagate_ready (C:\Users\Kai\Documents\godot\godot\scene\main\node.cpp:258) [16] Node::_propagate_ready (C:\Users\Kai\Documents\godot\godot\scene\main\node.cpp:258) [17] Node::_set_tree (C:\Users\Kai\Documents\godot\godot\scene\main\node.cpp:3181) [18] OS_Windows::run (C:\Users\Kai\Documents\godot\godot\platform\windows\os_windows.cpp:1669) [19] widechar_main (C:\Users\Kai\Documents\godot\godot\platform\windows\godot_windows.cpp:181) [20] _main (C:\Users\Kai\Documents\godot\godot\platform\windows\godot_windows.cpp:208) [21] main (C:\Users\Kai\Documents\godot\godot\platform\windows\godot_windows.cpp:220) [22] __scrt_common_main_seh (d:\agent\_work\4\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288) [23] -- END OF BACKTRACE -- ================================================================ ```

There is also a third, more exotic way of causing this crash:

Script Extending - Inner Class.zip

• base.gd has no class_name but class BaseInner:
• VanillaThing.gd extends "res://base.gd".BaseInner
• base.gd and VanillaThing.gd are attached to a node in the main scene.

Stack trace

``` ================================================================ CrashHandlerException: Program crashed Engine version: Godot Engine v4.3.dev.custom_build (780e1a50408360cf0cf93c0b55b59e9d7b4ad0b1) Dumping the backtrace. Please include this when reporting the bug to the project developer. [0] GDScriptCompiler::_prepare_compilation (C:\Users\Kai\Documents\godot\godot\modules\gdscript\gdscript_compiler.cpp:2661) [1] GDScriptCompiler::compile (C:\Users\Kai\Documents\godot\godot\modules\gdscript\gdscript_compiler.cpp:3198) [2] GDScript::reload (C:\Users\Kai\Documents\godot\godot\modules\gdscript\gdscript.cpp:803) [3] GDScriptCache::get_full_script (C:\Users\Kai\Documents\godot\godot\modules\gdscript\gdscript_cache.cpp:347) [4] ResourceFormatLoaderGDScript::load (C:\Users\Kai\Documents\godot\godot\modules\gdscript\gdscript.cpp:2863) [5] ResourceLoader::_load (C:\Users\Kai\Documents\godot\godot\core\io\resource_loader.cpp:268) [6] ResourceLoader::_thread_load_function (C:\Users\Kai\Documents\godot\godot\core\io\resource_loader.cpp:326) [7] ResourceLoader::_load_start (C:\Users\Kai\Documents\godot\godot\core\io\resource_loader.cpp:529) [8] ResourceLoaderText::load (C:\Users\Kai\Documents\godot\godot\scene\resources\resource_format_text.cpp:476) [9] ResourceFormatLoaderText::load (C:\Users\Kai\Documents\godot\godot\scene\resources\resource_format_text.cpp:1685) [10] ResourceLoader::_load (C:\Users\Kai\Documents\godot\godot\core\io\resource_loader.cpp:268) [11] ResourceLoader::_thread_load_function (C:\Users\Kai\Documents\godot\godot\core\io\resource_loader.cpp:326) [12] ResourceLoader::_load_start (C:\Users\Kai\Documents\godot\godot\core\io\resource_loader.cpp:529) [13] ResourceLoader::load (C:\Users\Kai\Documents\godot\godot\core\io\resource_loader.cpp:446) [14] Main::start (C:\Users\Kai\Documents\godot\godot\main\main.cpp:3810) [15] widechar_main (C:\Users\Kai\Documents\godot\godot\platform\windows\godot_windows.cpp:179) [16] _main (C:\Users\Kai\Documents\godot\godot\platform\windows\godot_windows.cpp:208) [17] main (C:\Users\Kai\Documents\godot\godot\platform\windows\godot_windows.cpp:220) [18] __scrt_common_main_seh (d:\agent\_work\4\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288) [19] -- END OF BACKTRACE -- ================================================================ ```

[pirey0]

After talking with Kana i've been debugging this extensively The issue is not GDScript::find_class returning a wrong value. That is just a symptom. (which invalidates the pr fix attempts in https://github.com/godotengine/godot/pull/84148)

This is the sequence of events that leads to the problem:

1. extend0.gd is compiled
2. extend0 takes over the path res://base.gd (GDScriptCache::set_path with p_take_over true)
3. extend1.gd starts getting compiled
4. GDScriptAnalyzer::resolve_class_inheritance gets called for extend1.gd
5. gdscript_analyzer.cpp:403 calls parser->get_depended_parser_for("res://base.gd")
6. this calls GDScriptCache::get_parser("res://base.gd") <- (this returns a parser for "res://base.gd", which should probably already be remapped and return a parser for "res://extend0.gd") ⚠️
7. which leads to GDScriptCache::get_source_code("res://base.gd") <- (Reads the source code from res://base.gd, the wrong file) ⚠️
8. base.gd gets (incorrectly) re-parsed as the base-type of extend1
9. gdscript_compiler.cpp:2667 calls GDScriptCompiler::_gdtype_from_datatype passing the re-parsed "res://base.gd" as base type
10. GDScriptCache::get_shallow_script("res://base.gd") is called which returns the extend0.gd type
11. script->find_class(p_datatype.class_type->fqcn) is called (gdscript_compiler.cpp:167) with script referring to extend0.dg, but _pdatatype to base.gd ❗ ❗ ❗
12. find_class "correctly" returns null because extend0 is not the type with fqcn "Base"
13. crash! because "empty" is returned in gdscript_compiler.cpp:2669 and we assume a valid base type

[pirey0]

This bug breaks how modding was approached in Godot 3.5, so I would like to get this fixed quite soon or find an alternative.

(In my personal case, over at Bippinbits we are trying to port Dome Keeper to Godot 4, but without this issue getting addressed we would lose modding support/ would have to rebuild how modding is supported.)

[otDan]

I guess we wait for 4.4 😔

[funnbot]

I've done some further investigating and came up with more questions than answers.

Following what pirey0 found, I compared the execution of the broken + working MRP, that is, with class_name Base in 'res::/base.gd' and without. where: B = broken MRP, W = working MRP.

3. "extend1.gd" starts getting compiled
4. GDScriptAnalyzer::resolve_class_inheritance gets called for 'res://extend1.gd'
5. B and W take this path, GDScriptAnalyzer::resolve_class_inheritance
6. parser->get_depended_parser_for(p_class->extends_path), 'res://base.gd' will not be in depended_parsers map, so this calls GDScriptCache::get_parser('res://base.gd')
   • B and W create a new GDScriptParser/GDScriptParserRef instance because 'res://base.gd' has been removed from GDScriptCache::parser_map
7. which leads to GDScriptCache::get_source_code('res://base.gd') <- B and W both read the original source from filesystem at 'res://base.gd'.
8. base = ext_parser->get_parser()->head->get_datatype();, this GDScriptParser::DataType, which will be Kind::CLASS for both, will be used later in GDScriptCompiler::_gdtype_from_datatype.
   • Notably, nothing else from this parser is kept.
   • B: base->class_type->fqcn = 'Base'
   • W: base->class_type->fqcn = 'res://base.gd'
9. GDScriptCompiler::_gdtype_from_datatype calls GDScriptCache::get_shallow_script('res://base.gd') which returns 'res://extend0.gd' from GDScriptCache::full_gdscript_cache, and then
   • B: find_class('Base') fails,
   • W: find_class('res://base.gd') succeeds.

So this raises the question, why does GDScriptCache::get_source_code read the source from filesystem again

• called by GDScriptParserRef::raise_status(Status::EMPTY)

when in all cases, GDScript::source has already been set by GDScript::load_source_code. Meaning all scripts are read from filesystem twice.

[Cranzor]

As I am unsure of when this will be resolved and wanted to implement modding support for my project, I created a plugin (Inheritance Chain Mender) to alleviate this issue for the time being. It automatically converts your GDScript files at export to no longer use class_name. Note that it may not account for all edge cases at present. Hope it can be of use to others.