KANAjetzt
commented
10 months ago is this the cause for #338?
Open ColdCalzone opened 11 months ago
KANAjetzt
commented
10 months ago is this the cause for #338?
KANAjetzt
commented
10 months ago commenting this part out:
static func apply_extension(extension_path: String) -> Script:
# Check path to file exists
if not FileAccess.file_exists(extension_path):
ModLoaderLog.error("The child script path '%s' does not exist" % [extension_path], LOG_NAME)
return null
var child_script: Script = ResourceLoader.load(extension_path)
# Adding metadata that contains the extension script path
# We cannot get that path in any other way
# Passing the child_script as is would return the base script path
# Passing the .duplicate() would return a '' path
child_script.set_meta("extension_script_path", extension_path)
# Force Godot to compile the script now.
# We need to do this here to ensure that the inheritance chain is
# properly set up, and multiple mods can chain-extend the same
# class multiple times.
# This is also needed to make Godot instantiate the extended class
# when creating singletons.
# The actual instance is thrown away.
child_script.new()
var parent_script: Script = child_script.get_base_script()
var parent_script_path: String = parent_script.resource_path
# We want to save scripts for resetting later
# All the scripts are saved in order already
# if not ModLoaderStore.saved_scripts.has(parent_script_path):
# ModLoaderStore.saved_scripts[parent_script_path] = []
# # The first entry in the saved script array that has the path
# # used as a key will be the duplicate of the not modified script
# ModLoaderStore.saved_scripts[parent_script_path].append(parent_script.duplicate())
#
# ModLoaderStore.saved_scripts[parent_script_path].append(child_script)
ModLoaderLog.info(
"Installing script extension: %s <- %s" % [parent_script_path, extension_path], LOG_NAME
)
child_script.take_over_path(parent_script_path)
return child_scriptResults in a silent crash for me, in the console this error is logged:
ERROR: FATAL: Condition "!exists" is true.
at: operator[] (./core/templates/hash_map.h:504)*Edit
It crashes with the second script_extension in _check_inheritances -> var b_stack := cached_inheritances_stack(extension_b)
https://github.com/GodotModding/godot-mod-loader/blob/d5f07336c93c942a4df81265d1688629bed6ec8c/addons/mod_loader/internal/script_extension.gd#L65
*Edit 2: Basically, as soon as a second script is loaded that extends the same named class as a previous "script_extension," it crashes.
ColdCalzone
commented
10 months ago is this the cause for #338?
Nope. I can recreate that error when directly using take_over_path in a test project.
KANAjetzt
commented
10 months ago Here is a minimal reproduction project that replicates the issue: Godot v4.1.2.stable.official Script Extending.zip
As described above, loading the second script that extends the same-named class causes a crash, with the only hint being this error if the editor is run with the console window:
ERROR: FATAL: Condition "!exists" is true.
at: operator[] (./core/templates/hash_map.h:504)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")*Edit: Same result in Godot v4.2.beta1.official
KANAjetzt
commented
10 months ago I opened an issue, let's hope we find answers there 👀 https://github.com/godotengine/godot/issues/83542
I will hunt for a workaround in the meantime. I don't expect that this behavior will change anytime soon. Not being able to extend a script two times renders pretty much the whole mod loader useless.
ZackeryRSmith
commented
5 months ago I will hunt for a workaround in the meantime.
I have limited Godot knowledge but I thought: "why not mess around with this". I'm wondering how feasible it is to modify the file causing errors at runtime. For example modifying extender.gd slightly to remove a line at runtime, fixes the crash.
func _init() -> void:
var file = FileAccess.open("res://base.gd", FileAccess.READ_WRITE)
file.seek(0)
file.store_string(' '.repeat(len("class_name Base")))
file.close()
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")However this does create an annoying popup in the editor, but other than that the file seems unaffected. As far as adding back in the functionality of class_name I'm a bit unsure on. You may have to modify the last extension being made to include the class_name? Regardless, I hope this could be of some use!
As an extra note I'm using Godot v4.1.3 Script Extending Workaround.zip
kakaroto
commented
4 months ago The specific issue here doesn't seem to be related to take_over_path but rather to calling parent_script.duplicate() because duplicating the script causes a new (duplicate) script with the same class_name statement, which is why we get the Parser Error: Class "ClassName" hides a global script class." error.
I tried to dynamically update the script's source code, which does fix the duplicate issue. I wanted to assign the class_name to the new child_script, but that causes the same error, since it looks like the global class doesn't get deregistered once we reload the script with the class_name directive.
This is the new code:
if not ModLoaderStore.saved_scripts.has(parent_script_path):
ModLoaderStore.saved_scripts[parent_script_path] = []
# If the parent script has a global class name, then remove it and set it on the child script instead
# This prevents a "Class X hides a global script class" error when calling duplicate()
# and (potentially) fixes a take_over_path bug in Godot 4.
if parent_script.has_source_code() and parent_script is GDScript:
# Regexp to find a GDScript class name
var class_name_regex : RegEx = RegEx.create_from_string(r"(?m)^\s*class_name\s+(\S+)")
var source: String = parent_script.source_code
var _match: RegExMatch = class_name_regex.search(source)
if _match:
var global_name = _match.get_string(1) # Godot 4.3 has parent_script.get_global_name(), but we also don't need to know it
source = class_name_regex.sub(source, "#$0")
parent_script.source_code = source
parent_script.reload()
# Set the class_name directive on the child instead
# This doesn't work because the global class is still registered in the engine
#if child_script.has_source_code():
#source = _match.get_string(0) + "\n" + child_script.source_code
#child_script.source_code = source
#child_script.reload()
# The first entry in the saved script array that has the path
# used as a key will be the duplicate of the not modified script
ModLoaderStore.saved_scripts[parent_script_path].append(parent_script.duplicate())The idea was to always keep the topmost script with the class_name directive in its source code, and registered as the global class, so remove_specific_extension_from_script would restore things (removing the comment on the line).
Note that Godot 4 doesn't seem to use ProjectSetting _global_script_classes setting anymore, but only has a get_global_class_list method, so we can't register/deregister classes via an API (as far as I could see).
(Unrelated, but this also means that ModLoaderMod.register_global_classes_from_array doesn't work anymore, i.e: #335)
In the process of extending a script, this is run: https://github.com/GodotModding/godot-mod-loader/blob/e3e795bd040de157c4b996ed50d4bd670132b8b6/addons/mod_loader/internal/script_extension.gd#L100-L106 This causes an error,
Parser Error: Class "ClassName" hides a global script class."