ogapo commented 1 year ago

Godot version

4.1.1.stable

System information

Godot v4.1.1.stable - macOS 13.4.1 - Vulkan (Mobile) - integrated Apple M1 Pro - Apple M1 Pro (10 Threads)

Issue description

Issue

When mounting DLC content (via PCK or ZIP archive) the loading code mounts the files, but does not re-index any global caches (such as .godot/global_script_class_cache.cfg and .godot/uid_cache.bin).

For global_script_class_cache.cfg (included in PCK files by default) this results in any DLC class that exposes class_name MyBaseControl not being resolvable by other code unless that script is explicitly loaded first. This makes it impossible to use named DLC classes via extends MyBaseControl or as a resource class in a .tres file. This appears to be the case no matter if the use is within the same PCK or within another one that depends on it.

Workaround: For extends it's still possible to use the res:// syntax instead of the global class name. However, for the .tres case it always fails because the fallback from script_class is uid which also does not work (see below).

uid_cache.bin is also included in all PCK files by default, but appears to be ignored during mounting as well. This mostly results in warnings being emitted as seen below.

W 0:00:01:0163   open: res://dlc/frontend/frontend-main.tscn: In external resource #2, invalid UID: uid://glss85r4a4qa - using text path instead: res://dlc/frontend/ui/location.tscn
  <C++ Source>   core/io/resource_format_binary.cpp:1053 @ open()

Most cases using UIDs have a fallback to full path (which still works, albeit less efficiently) but a notable exception is the .tres file format which uses the global class name followed by UID without a res field (perhaps this is another issue?).

[gd_resource type="Resource" script_class="MyResourceClass" load_steps=2 format=3 uid="uid://bl1m18r3skrb8"]

NOTE: after examining get_forced_export_files in C++ I think there may be a similar issue with the internationalization files, but I don't know enough about that system to say for sure.

Assessment:

If these caches aren't meant to be used then we shouldn't pack them into PCK files generated by --export-pack since those are not meant to be main pck files (project.binary is also included which is just bloat in these DLC files, probably everything in EditorExportPlatform::get_forced_export_files should be examined). However, I don't think things like class_name will work without them so it would make more sense to make merging the values with the existing runtime global class map and UID cache part of mounting.

Proposed Fix:

After mounting a PCK, load the UID, global_class, and Internationalization files again and merge anything found with what's already in memory.

Steps to reproduce

Repro:

Create an initial project initial-proj with nothing in it. This represents the initial (shipped) state of the project.
- add a new scene called main_scene.tscn
  - give it a custom script main_scene.gd which extends Control
- add a default scene called start_scene.tscn (set this as the default for the project)
  - add code to load res://DLC.pck (if it exists) and then change to main_scene.tscn (whether DLC was found or not)
Copy your initial project to another directory called dlc-proj. This will represent the post-release (DLC) version of the project.
In the copied DLC project
- create a Control class (exposed with class_name MyBaseControl) called my_base.gd
- change main_scene.gd to extend MyBaseControl instead of Control
- create an export config for your current platform
  - restrict the export config to export main_scene.tscn and dependencies.
  - use the GUI to export DLC.pck and place it in initial-proj
Go back to the initial project and run it (now with DLC) (minimal reproduction project attached)

Expected Result

The DLC should load properly in initial-proj and the main_scene.tscn should spawn using the updated version (derived from MyBaseControl). This should be the same behavior as observed when running dlc-proj.

Actual Result

When running dlc-proj everything works.

However, upon running initial-proj (with DLC.pak present) the main_scene.gd script fails to load with the error:

Parser Error: Could not find base class "MyBaseClass".

Minimal reproduction project

pck-issue.zip

ogapo commented 1 year ago

I'm interested in submitting a PR for this, but would welcome confirmation from a dev that I'm not just using the system wrong somehow :-) and whether my assessment of the fix makes sense.

ghmart commented 1 year ago

I've also encountered similar problem while developing modding system and found workaround:

Generate list of resource ids for .tscn, .tres and files like .png, ogg, etc. and store them in text file.
Include this text file in exported pck.
After loading pck in main application restore resource ids for all those files back.

Would be great if this issue can be fixed to get rid of this workaround.

ogapo commented 1 year ago

I've also encountered similar problem while developing modding system and found workaround:

Generate list of resource ids for .tscn, .tres and files like .png, ogg, etc. and store them in text file.

Include this text file in exported pck.

After loading pck in main application restore resource ids for all those files back.

Would be great if this issue can be fixed to get rid of this workaround.

This is good info! I assume this was specifically to address the UID caching part of the bug? I was going to try something like this first (in the PR).

ghmart commented 1 year ago

This is good info! I assume this was specifically to address the UID caching part of the bug? I was going to try something like this first (in the PR).

Yes. It was so annoying to see all those warnings about "invalid UID".

Here's some chunks of code from this workaround:

To generate array of ids and filenames:

var uids = []
func generate_uids(path):
var dir = DirAccess.open(path)
dir.list_dir_begin()
var filename = dir.get_next()
while filename != "":
    if dir.current_is_dir() and filename != "global_share":
        generate_uids(dir.get_current_dir() + "/" + filename)
    else:
        var cur_dir = dir.get_current_dir()
        var id = ResourceLoader.get_resource_uid(cur_dir + "/" + filename)
        if id != -1:
            var relative_path = cur_dir.replace(Events.path + "modules/" + $Module.text, "")
            uids.append(str(id))
            uids.append("res:/" + relative_path + "/" + filename)
    filename = dir.get_next()

Stored txt file looks like this:

358396934393139503
res://icon.svg
7721873721324996179
res://cfg/icon.png
4962929528344609547
res://hotcold/enemy.tscn
3952460360499408664
res://hotcold/hud.tscn
6396805370724544074
res://hotcold/main.tscn
2816660160389940632
res://hotcold/label.tres
8738893336698734136
res://hotcold/player.tscn

Restore uids after loading pck:


func run(module):
ProjectSettings.load_resource_pack(module.get_meta("file") + "/game.pck")
Events.set_script(null)
Events.set_script(ResourceLoader.load("res://events.gd", "", ResourceLoader.CACHE_MODE_IGNORE))
update_uids()

func update_uids(): if not FileAccess.file_exists("res://uids.txt"): return

var file = FileAccess.open("res://uids.txt", FileAccess.READ)
while not file.eof_reached():
    var id = int(file.get_line())
    if ResourceUID.has_id(id):
        ResourceUID.set_id(id, file.get_line())
    else:
        ResourceUID.add_id(id, file.get_line())



Maybe I've missed some details here (it's from old project) but this could give general idea.

dioptryk commented 1 year ago

There seems to be something fundamentally wrong here (at least on 4.1.1 mono).

I have multiple PCKs which I used to split the project into smaller resource packs. This worked without problems in Godot 3. Now, a single PCK with a single model immediately shows this warning when trying to load the related resource.. And the worst thing is, everything seems correct - the UID is the same in the .import file in the PCK project, in the exported binary PCK, and also ResourceLoader reports the same UID when querying about the model path, after the PCK is loaded. So why is there a warning at all?

And since the UIDs are the same, the workaround does not work, because I get an error about already existing UID.

Everything is working, the model shows up and all, but when I migrate all my PCKs I am potentially looking at 1000+ warnings.

teddybear082 commented 8 months ago

I think I have this same problem, when using ProjectSettings.load_resource_pack("path/to/my/zip") with a zip created by exporting using the proper tool from the editor, custom classes in the zip files are not recognized by the project and I get errors like:

USER SCRIPT ERROR: Parse Error: Could not find type "XRToolsPointerEvent" in the current scope.
   at: GDScript::reload (res://xr_viewport_2d_in_3d.gd:205)
ERROR: Failed to load script "res://xr_viewport_2d_in_3d.gd" with error "Parse error".
   at: load (modules/gdscript/gdscript.cpp:2775)
USER SCRIPT ERROR: Parse Error: Could not find type "XRToolsPointerEvent" in the current scope.
   at: GDScript::reload (res://xr_viewport_2d_in_3d_body.gd:56)

Meanwhile, I confirmed the zip file does have a .godot folder and inside that is a godot_script_class_cache.cfg that has the following contents:

list=Array[Dictionary]([{
"base": &"Node3D",
"class": &"XRToolsFunctionPointer",
"icon": "",
"language": &"GDScript",
"path": "res://xr_pointer.gd"
}, {
"base": &"RefCounted",
"class": &"XRToolsPointerEvent",
"icon": "",
"language": &"GDScript",
"path": "res://xr_pointer_event.gd"
}])

godotengine / godot

Mounting PCK/ZIP files does not appear to load/merge uid_cache.bin or global_script_class_cache.cfg #82061

Godot version

System information

Issue description

Issue

Assessment:

Proposed Fix:

Steps to reproduce

Repro:

Expected Result

Actual Result

Minimal reproduction project