Implement load_resource_pack destination support

[nonchip]

Example usage:

func dir_contents(path, ind =""):
	var dir = DirAccess.open(path)
	if dir:
		dir.list_dir_begin()
		var file_name = dir.get_next()
		while file_name != "":
			if dir.current_is_dir():
				print(ind+"Found directory: " + file_name)
				dir_contents(path+"/"+file_name, ind+"  ")
			else:
				print(ind+"Found file: " + file_name)
			file_name = dir.get_next()
	else:
		print("An error occurred when trying to access the path.")

func _ready():
	print("ZIP:")
	ProjectSettings.load_resource_pack("res://test.zip",false,0,"blah/")
	dir_contents("res://blah")

	print("PCK:")
	ProjectSettings.load_resource_pack("res://test.pck",false,0,"blub/")
	dir_contents("res://blub")

	# test actual loading
	print("LOAD:")
	print(load("res://blub/icon.svg"))

Output:

ZIP:
Found directory: .godot
  Found directory: imported
	Found file: icon.svg-218a8f2b3041327d8a5756f3a245f83b.ctex
  Found directory: exported
	Found directory: 133200997
	  Found file: export-3070c538c03ee49b7677ff960a3f5195-main.scn
  Found file: global_script_class_cache.cfg
  Found file: uid_cache.bin
Found file: icon.svg.import
Found file: main.gd
Found file: main.tscn.remap
Found file: icon.svg
Found file: project.binary
PCK:
Found directory: .godot
  Found directory: exported
	Found directory: 133200997
	  Found file: export-3070c538c03ee49b7677ff960a3f5195-main.scn
  Found directory: imported
	Found file: icon.svg-218a8f2b3041327d8a5756f3a245f83b.ctex
  Found file: global_script_class_cache.cfg
  Found file: uid_cache.bin
Found file: icon.svg
Found file: icon.svg.import
Found file: main.gd
Found file: main.tscn.remap
Found file: project.binary
LOAD:
<CompressedTexture2D#-9223372013014350661>

• Bugsquad edit, closes: Allow using ProjectSettings.load_resource_pack() into a subfolder godot-proposals#7769

[nonchip]

note this does not imply the resource cache actually working for subfolders: load("res://blub/icon.svg") still wants to reference res://.godot/imported/icon.svg.* (instead of the expected res://blub/.godot/......). but it at least allows to mount a mod/etc without overriding /; you just kinda have to treat the files as "dumb like in usr://".

[jams3223]

note this does not imply the resource cache actually working for subfolders: load("res://blub/icon.svg") still wants to reference res://.godot/imported/icon.svg.* (instead of the expected res://blub/.godot/......). but it at least allows to mount a mod/etc without overriding /; you just kinda have to treat the files as "dumb like in usr://".

It seems like one of the checks has failed.

[nonchip]

note this does not imply the resource cache actually working for subfolders: load("res://blub/icon.svg") still wants to reference res://.godot/imported/icon.svg.* (instead of the expected res://blub/.godot/......). but it at least allows to mount a mod/etc without overriding /; you just kinda have to treat the files as "dumb like in usr://".

It seems like one of the checks has failed.

yup it keeps doing that and incrementally telling me i forgot to do a single maintenance thing :P first it was godot --doctool, now it's something about the api dump ("no compatibility function provided" is a lie tho, i merely added an optional argument, so C++ provides the "compatibility function"), not actually sure how to fix that, sorry :/

tried to godot --dump-extension-api but that doesn't change anything...

[AThousandShips]

Check the instructions in: misc/extension_api_validation /4.1-stable.expected

[nonchip]

@AThousandShips that file almost makes sense to me, except the bit about what the GH-NUMBER lines actually mean, should i insert the PR number there?

[AThousandShips]

Yes

[nonchip]

yeah um, i think the check might've broken? it complains about not knowing what an enum is, nothing about my PR tho, if i read that right :P

[AThousandShips]

The relevant issue is highlighted in red:

Error: Validate extension JSON: Error: Hash changed for 'classes/ProjectSettings/methods/load_resource_pack', from B2EAA0DF to 167E5FB9. This means that the function has changed and no compatibility function was provided.

You need to add a compatibility method

[nonchip]

um, how, given that's literally the job of the optional parameter? 0.o

[nonchip]

that one was my editor deciding to randomly start using 8 spaces for a tab somehow, despite my tabs literally everywhere being tabs and 2 wide. sorry this PR is such a pain :/

[nonchip]

AARGH now i undid your space again ;_;

[AThousandShips]

AARGH now i undid your space again ;_;

You will need to pull the branch before making changes locally or any suggestions applied here will vanish

[nonchip]

s/will/would've/; yeah i did a late fetch&rebase because i forgot the pull and messed up in the manual merge :P

[rsubtil]

note this does not imply the resource cache actually working for subfolders: load("res://blub/icon.svg") still wants to reference res://.godot/imported/icon.svg.* (instead of the expected res://blub/.godot/......). but it at least allows to mount a mod/etc without overriding /; you just kinda have to treat the files as "dumb like in usr://".

Without this though, this functionality is extremely limited. Trying to load any scene will fail because all scripts and resources will try to fetch from res:// when actually they are now stored in res://blah, for example. Even loading assets directly doesn't seem to work correctly because the resource is remapped to what's inside res://.godot, and not res://blah/.godot, and thus doesn't use the new content under the pack.

However, I wonder if it's enough to actually account for the existence of multiple .godot folders and finding the correct one to use when loading resources, to finally implement this long-requested feature.

[nonchip]

However, I wonder if it's enough to actually account for the existence of multiple .godot folders and finding the correct one to use when loading resources, to finally implement this long-requested feature.

@rsubtil i think then you're still left with the (potentially bigger) issue of any resource paths being prefixed now, and how to specify whether res:// means from the game's or pck's point of view.

i didn't feel really "comfortable enough" to mess with core workflows / design decisions that deeply, but i feel even just getting the option to open a pck file without potentially nuking res:// somehow (i mean, there isn't even an API for a "manually driven" PCKUnPacker, the "best" option so far was essentially bytewise parsing) is worth a merge, especially since this change was relatively straightforward.

and i mean technically this does now allow to do any of the things you described, just not builtin yet, by extensive filtering of the PackedScene onload while manually loading the assets (either by following the export path rewrite, or by using "importing" functions like Image.load_from_file etc).

[Jade-TheCat]

Perhaps instead of mangling res:// by prefixing them, we could have a separate location for those resources that acts like res:// for loaded packs?

So in the pack you would use pack:// for all pack-internal references and res:// for all of the base game resources. Then in project settings there could be a setting for where to load non-overriding packs by default (let's say it defaults to res://packs/<pack_file_name>), and there could be an alternative to load_resource_pack() (say, load_resource_pack_no_override()) that would load the pack to the path set in project settings. The engine would have to keep track of what pack a file is in, but I think that could be easy by traversing up the tree from its position in res://packs/... and maybe placing a special .packreference file or something in the pack's directory.

Of course, this would still allow for packs to be loaded into res://, say for updates or texture packs, but there would be the option to put the pack contents somewhere it doesn't mangle stuff for mods. I believe there's a way to specify what types of files get loaded by load_resource_pack() but if not that may be something to consider as well. That way you can specify per-pack whether it should override stuff and what it should override.

I'm not really sure how one would implement this as I have only looked at the engine code a little, but I might take a look into it when I have more time and if someone else doesn't pick up this or another suggestion.

[nonchip]

yeah all pretty good ideas imo, but probably best to put those in proposals instead of the PR, i guarantee they'll get lost otherwise ;)

[Jade-TheCat]

yeah all pretty good ideas imo, but probably best to put those in proposals instead of the PR, i guarantee they'll get lost to time otherwise ;)

oh true, forgot this wasn't proposals lmao. I'll go ahead and do that

[rsubtil]

@Jade-TheCat Just to point out most of those ideas (choosing what files can be overriden, and custom resource paths) are already being discussed in godotengine/godot-proposals#2689 and godotengine/godot-proposals#6307.

@nonchip

and i mean technically this does now allow to do any of the things you described, just not builtin yet, by extensive filtering of the PackedScene onload while manually loading the assets (either by following the export path rewrite, or by using "importing" functions like Image.load_from_file etc).

That's the issue though: it works technically, and requires a lot of manual stuff in order to get the functionality you desire. There has been an attempt at doing this before (#35261) which was subsequently reverted (#38035, #25815) precisely because the remapping issue isn't trivial.

I agree, it's not pretty to dump everything on res:// and I myself also want some solution to this as my project also relies heavily on user content. But I believe that, to get this accepted, it should be a simple mechanism to use and not need any extensive manual tricks to be useful.

[nonchip]

@rsubtil

in order to get the functionality you desire

the functionality i desire is pretty much just to be able to open a pck at all without risking res://, but yeah it's only the first step to the functionality most people desire.

i would really like this first step to be a thing though until we figure out the "it just works" part, because right now it's literally impossible to do anything like that. like, there isn't even an ls for pck files so you can load metadata from your mods without enabling them. maybe an alternative api that exposes the underlying pack accessor so you could treat it more like a DirAccess? potentially even just something like "make PCKPacker readable instead of write-only"? even like something really dumb with a Dictionary get_directory_tree and a File open_file (or, if that's annoying with random access, maybe just a PackedByteArray get_file_contents) would be better than no introspection at all.

that latter idea sounds like a GDExtension actually, i might take another look at that and release an addon :)

[Jade-TheCat]

@nonchip

like, there isn't even an ls for pck files so you can load metadata from your mods without enabling them.

I mean if you're using ZIPs there might be a way to do this but idk if it's available in GDScript. In other languages, you could open the .zip into a variable and play with its filesystem that way. That's how Minecraft mods load metadata afaik. That may require a GDExtension for Godot though.

[nonchip]

@Jade-TheCat there's a ZIPReader yeah, but no PCKReader.

[Jade-TheCat]

ah, right. I'm not super familiar with PCK files, are they a data blob like DOOM WADs?

[nonchip]

or like ZIPs, yeah.

[Jade-TheCat]

I thought ZIPs had more proper directory trees though, whereas WADs are a flat filespace with names acting for paths

[nonchip]

nope, and also out of scope of this PR

[rsubtil]

i would really like this first step to be a thing though until we figure out the "it just works" part, because right now it's literally impossible to do anything like that. like, there isn't even an ls for pck files so you can load metadata from your mods without enabling them. maybe an alternative api that exposes the underlying pack accessor so you could treat it more like a DirAccess? potentially even just something like "make PCKPacker readable instead of write-only"? even like something really dumb with a Dictionary get_directory_tree and a File open_file (or, if that's annoying with random access, maybe just a PackedByteArray get_file_contents) would be better than no introspection at all.

Indeed, that's a good first step. There's currently no good solution to prevent this, other than loading a pack with replace_files disabled. I just think we need a better way to do this without introducing it in a way users are likely to interpret as a fully working solution for resource remapping, and then complaining that the feature is "not working".

There's actually FileAccessPack/DirAccessPack implementations in https://github.com/godotengine/godot/blob/master/core/io/file_access_pack.h already. I've successfully used it in another PR in order to fetch a pack file directly without it overriding any existing files. These do not seem to be used anywhere else in the codebase and are not exposed to GDScript, so I think these could be exposed and are a good way to support your use-case.

[scgm0]

How is the progress of this pr?

[nonchip]

@scgm0 as you can clearly read above, halted indefinitely.

[scgm0]

@scgm0 as you can clearly read above, halted indefinitely.

What a pity...

[nonchip]

@scgm0 not at all, for the reasons seen above. what is tho is spam.

[scgm0]

@scgm0 not at all, for the reasons seen above. what is tho is spam.

No, this PR is very useful for my mod function, because my mod will not use godot resources.