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

[4.x] Duplicating a global named script class (i.e. in script_extension.gd) causes an error. #345

Open
Tracked by #340
ColdCalzone opened this issue Oct 10, 2023 · 7 comments
Labels
4.x bug Something isn't working
Milestone

Comments

@ColdCalzone
Copy link

In the process of extending a script, this is run:

# 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())

This causes an error, Parser Error: Class "ClassName" hides a global script class."

@ColdCalzone ColdCalzone changed the title Duplicating a global script class (i.e. in script_extension.gd) causes an error. [4.x] Duplicating a global script class (i.e. in script_extension.gd) causes an error. Oct 10, 2023
@KANAjetzt KANAjetzt added bug Something isn't working 4.x labels Oct 15, 2023
@KANAjetzt KANAjetzt added this to the 4.x - 6.2.0 milestone Oct 15, 2023
@KANAjetzt
Copy link
Collaborator

is this the cause for #338?

@KANAjetzt
Copy link
Collaborator

KANAjetzt commented Oct 15, 2023

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_script

Results 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)

var parent_script: Script = load(extension_path)

Log dump
Godot Engine v4.1.2.stable.official.399c9dc39 - https://godotengine.org
Vulkan API 1.3.242 - Forward+ - Using Vulkan Device #0: NVIDIA - NVIDIA GeForce RTX 3080
 
INFO ModLoader:Store: Applying options override with feature tag "editor".
DEBUG ModLoader: Autoload order
[
  "ModLoaderStore",
  "ModLoader",
  "Utils",
  "DebugService",
  "SceneManager",
  "TextManager",
  "PartService",
  "EffectService",
  "AudioManager",
  "RunData",
  "PlayerManager",
  "OnlineService",
  "ControllerIcons",
  "InputService",
  "MultiplayerInput"
]
INFO ModLoader: game_install_directory: res://
  Can't open mod folder res://mods
INFO ModLoader: No zipped mods found
SUCCESS ModLoader: DONE: Setup 4 mods
DEBUG ModLoader:UserProfile: Updated the active state of all mods, based on the current user profile "default"
INFO ModLoader:ModData: Loading mod_manifest (manifest.json) for -> GodotModding-ModConfig
DEBUG ModLoader:ModData: GodotModding-ModConfig loaded manifest data -> { "name": "ModConfig", "namespace": "GodotModding", "version_number": "0.1.0", "description": "Example Mod for User Profiles", "website_url": "https://github.com/GodotModding", "dependencies": [], "extra": { "godot": { "authors": ["GodotModding"], "optional_dependencies": [], "compatible_game_version": [], "compatible_mod_loader_version": ["6.0.0"], "incompatibilities": [], "load_before": [], "tags": [], "config_schema": { "title": "Config", "description": "Config for this Mod", "type": "object", "properties": { "material_settings": { "title": "Material Settings", "type": "object", "properties": { "square_scale": { "type": "number", "title": "Square Scale", "minimum": 0, "maximum": 0.3, "multipleOf": 0.001, "default": 0.1 }, "animate": { "type": "boolean", "title": "Animate", "default": false }, "bluer_amount": { "type": "number", "title": "Bluer Amount", "minimum": 0, "maximum": 10, "multipleOf": 0.1, "default": 5.1 }, "mix_amount": { "type": "number", "title": "Mix Amount", "minimum": 0, "maximum": 1, "multipleOf": 0.01, "default": 0.71 }, "color": { "type": "array", "title": "Color Over", "prefixItems": [{ "type": "number", "minimum": 0, "maximum": 255 }, { "type": "number", "minimum": 0, "maximum": 255 }, { "type": "number", "minimum": 0, "maximum": 255 }, { "type": "number", "minimum": 0, "maximum": 255 }], "items": false, "minItems": 3, "maxItems": 4, "default": [0, 0, 0, 255] } } } } }, "description_rich": "", "image": <null> } } }
DEBUG ModLoader:UserProfile: Set the "current_config" of "GodotModding-ModConfig" to "default" in user profile "default" 
INFO ModLoader:ModData: Loading mod_manifest (manifest.json) for -> GodotModding-UserProfile
DEBUG ModLoader:ModData: GodotModding-UserProfile loaded manifest data -> { "name": "UserProfile", "namespace": "GodotModding", "version_number": "0.2.0", "description": "Example Mod for User Profiles", "website_url": "https://github.com/GodotModding", "dependencies": [], "extra": { "godot": { "authors": ["GodotModding"], "optional_dependencies": [], "compatible_game_version": [], "compatible_mod_loader_version": ["6.0.0"], "incompatibilities": [], "load_before": [], "tags": ["test1", "test2"], "config_schema": { "$schema": "https://json-schema.org/draft/2020-12/schema", "title": "Config", "description": "Config for this Mod", "type": "object", "properties": { "select_profile_text": { "title": "Select profile text:", "type": "string", "default": "Select Profile" }, "material_settings": { "title": "Material Settings", "type": "object", "properties": { "square_scale": { "type": "number", "title": "Square Scale", "minimum": 0, "maximum": 0.3, "multipleOf": 0.001, "default": 0.1 }, "animate": { "type": "boolean", "title": "Animate", "default": false }, "blur_amount": { "type": "number", "title": "Blur Amount", "minimum": 0, "maximum": 10, "multipleOf": 0.1, "default": 5.1 }, "mix_amount": { "type": "number", "title": "Mix Amount", "minimum": 0, "maximum": 1, "multipleOf": 0.01, "default": 0.71 }, "color": { "type": "string", "title": "Color Over", "format": "color", "default": "#f7000000" } } } } }, "description_rich": "", "image": <null> } } }
DEBUG ModLoader:UserProfile: Set the "current_config" of "GodotModding-UserProfile" to "test1" in user profile "default" 
INFO ModLoader:ModData: Loading mod_manifest (manifest.json) for -> KANA-VersionNumber
DEBUG ModLoader:ModData: KANA-VersionNumber loaded manifest data -> { "dependencies": [], "description": "Description of your mod...", "extra": { "godot": { "authors": ["KANA"], "compatible_game_version": ["0.0.1"], "compatible_mod_loader_version": ["6.2.0"], "config_schema": {  }, "description_rich": "", "image": <null>, "incompatibilities": [], "load_before": [], "optional_dependencies": [], "tags": [] } }, "name": "VersionNumber", "namespace": "KANA", "version_number": "0.0.1", "website_url": "https://github.com/exampleauthor/examplemod" }
INFO ModLoader:ModData: Loading mod_manifest (manifest.json) for -> KANA-VersionNumber2
DEBUG ModLoader:ModData: KANA-VersionNumber2 loaded manifest data -> { "dependencies": ["KANA-VersionNumber"], "description": "Description of your mod...", "extra": { "godot": { "authors": ["KANA"], "compatible_game_version": ["0.0.1"], "compatible_mod_loader_version": ["6.2.0"], "config_schema": {  }, "description_rich": "", "image": <null>, "incompatibilities": [], "load_before": [], "optional_dependencies": [], "tags": [] } }, "name": "VersionNumber2", "namespace": "KANA", "version_number": "0.0.1", "website_url": "https://github.com/exampleauthor/examplemod" }
SUCCESS ModLoader: DONE: Loaded all meta data
DEBUG ModLoader:Dependency: Checking dependencies - mod_id: GodotModding-ModConfig optional dependencies: []
DEBUG ModLoader:Dependency: Checking dependencies - mod_id: GodotModding-UserProfile optional dependencies: []
DEBUG ModLoader:Dependency: Checking dependencies - mod_id: KANA-VersionNumber optional dependencies: []
DEBUG ModLoader:Dependency: Checking dependencies - mod_id: KANA-VersionNumber2 optional dependencies: []
DEBUG ModLoader:Dependency: Checking dependencies - mod_id: GodotModding-ModConfig required dependencies: []
DEBUG ModLoader:Dependency: Checking dependencies - mod_id: GodotModding-UserProfile required dependencies: []
DEBUG ModLoader:Dependency: Checking dependencies - mod_id: KANA-VersionNumber required dependencies: []
DEBUG ModLoader:Dependency: Checking dependencies - mod_id: KANA-VersionNumber2 required dependencies: ["KANA-VersionNumber"]
DEBUG ModLoader:Dependency: Required dependency -> KANA-VersionNumber importance -> 1
INFO ModLoader: mod_load_order -> 1) KANA-VersionNumber
INFO ModLoader: mod_load_order -> 2) GodotModding-ModConfig
INFO ModLoader: mod_load_order -> 3) GodotModding-UserProfile
INFO ModLoader: mod_load_order -> 4) KANA-VersionNumber2
INFO ModLoader: Initializing -> KANA-VersionNumber
DEBUG ModLoader: Loading script from -> res://mods-unpacked/KANA-VersionNumber/mod_main.gd
DEBUG ModLoader: Loaded script -> <GDScript#-9223371983570336508>
DEBUG ModLoader: Adding child -> KANA-VersionNumber:<Node#53636760837>
INFO ModLoader: Initializing -> GodotModding-ModConfig
DEBUG ModLoader: Loading script from -> res://mods-unpacked/GodotModding-ModConfig/mod_main.gd
DEBUG ModLoader: Loaded script -> <GDScript#-9223371982999911158>
INFO GodotModding-ModConfig: Init
DEBUG ModLoader: Adding child -> GodotModding-ModConfig:<Node#54391735563>
INFO ModLoader: Initializing -> GodotModding-UserProfile
DEBUG ModLoader: Loading script from -> res://mods-unpacked/GodotModding-UserProfile/mod_main.gd
DEBUG ModLoader: Loaded script -> <GDScript#-9223371982194604783>
INFO GodotModding-UserProfile: Init
DEBUG ModLoader: Adding child -> GodotModding-UserProfile:<Node#55280928019>
INFO ModLoader: Initializing -> KANA-VersionNumber2
DEBUG ModLoader: Loading script from -> res://mods-unpacked/KANA-VersionNumber2/mod_main.gd
DEBUG ModLoader: Loaded script -> <GDScript#-9223371981305412328>

*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
Copy link
Author

is this the cause for #338?

Nope. I can recreate that error when directly using take_over_path in a test project.

@KANAjetzt
Copy link
Collaborator

KANAjetzt commented Oct 18, 2023

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
Copy link
Collaborator

KANAjetzt commented Oct 18, 2023

I opened an issue, let's hope we find answers there 👀
godotengine/godot#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.

@KANAjetzt KANAjetzt changed the title [4.x] Duplicating a global script class (i.e. in script_extension.gd) causes an error. [4.x] Duplicating a global named script class (i.e. in script_extension.gd) causes an error. Oct 23, 2023
@ZackeryRSmith
Copy link
Contributor

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
Copy link

kakaroto commented Apr 11, 2024

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants