Treat loaded resource packs separately from each other #2689
Comments
|
Edit: Hey there! Just want to let anyone reading this, that I'm thinking of reworking this proposed implementation. Quite honestly, this one feels a bit over-engineered to me. I think I can do a lot better. Below is my proposal for how I believe this could be implemented. If you have your own proposal for how this proposal should be implemented, feel free to base it on this, or make your own wildly different implementation if you desire. I believe this functionality should be exposed under a new singleton. The name is up in the air, but for ease, I'll be calling it Function signatures
I propose the following additional items:
Notes and Implementation DetailsThis also includes details on the Click to expandDisabling/enabling packsUsing When a pack is disabled, it is not searched for resources when using NOTE: Disabling a pack does not affect any already-loaded resources in any way, nor does it reverse any effects those resources may have had. (Un)Trusted packsWhen loading a pack, an optional argument to When a pack is untrusted, no scripts can be loaded from that pack, but resources from that pack can still use scripts from trusted packs. The ability to set a pack as untrusted is useful for developers who desire mod support where the mods are not allowed to add/change code -- for example, with texture packs or custom levels. To be more specific on how this works: When a pack is untrusted, any resources within are subject to the following restrictions when using any functions that return a
If a developer wishes to change the trusted status of a pack at runtime, they should unload the resource pack and reload it using NOTE: If you previously had a resource pack marked as trusted, then decided to reload it as untrusted, be aware that any already-loaded resources coming from that pack will not be affected in any way. Be careful if you decide to change a pack's trusted status at runtime. ResourceFilesystemSource
This class (as I describe it here) is meant as a base class, and should be inherited by subclasses that handle the different types of sources we can accept. For example:
and so on. I believe each source should manage its own resource cache, rather than the engine managing one large resource cache. This makes it easier for the engine to continue using a cache for speed, while not requiring the engine to recreate the cache every time a mod is loaded or unloaded. Additionally, I propose that the following information be exposed through this class:
Lastly, while I do list Versioned File PathsFile paths, such as those in However, in some cases, it may be desired that a file path be able to point to a specific version of a file. For example, let's say that a mod does not want to use any version of To handle this, I propose the concept of "versioned file paths." These file paths will always refer to one specific file from one specific loaded pack, and never to any other pack. Ideally, these file paths would still be Strings, just as normal file paths are. However, these strings are formatted specially to refer to one pack. I am not sure what format these "versioned file paths" should be in, however. Ideally, a "versioned file path" would refer to a pack even if that pack becomes unloaded, and perhaps could be accepted even if the source pack was never loaded before. With all that said, to round out the example given above (a mod wanting to load its own enemies list, and nobody else's), let's assume that these file paths are formatted like P.S. I understand this "versioned file paths" proposal may warrant being a separate proposal from the rest of this proposal. If desired, I can move this to a new proposal. Other Wanted/Needed ChangesClick to expandChanges to ProjectSettings
Additionally, Misc changesDue to the nature of my proposed implementation, some functions may need to be changed or removed:
GDScript Reference CodeResourceFilesystemServerextends Object
class ResourceFilesystemLoadedPack:
var enabled: bool = true
var trusted: bool = false
var source: ResourceFilesystemSource
# The resource cache for this loaded pack. Putting it in this instead of
# having a global resource cache means it's easier for the cache to be
# "updated" when a pack is loaded/unloaded... since the resource cache for
# each pack will get deleted when that pack gets unloaded.
#
# As an aside, this `ResourceCache` class is entirely ficticious, and the
# name is used here as a placeholder for however we actually implement the
# resource cache.
#
# For now, assume this has two functions: `has(path: String)` for checking if
# a resource is cached, and `add(path: String, resource: Resource)` for adding
# a resource to the cache.
var resource_cache: ResourceCache
var loaded_packs: Array # Array<ResourceFilesystemLoadedPack>
################################################################################
func is_loaded(
path: String
) -> bool:
for pack in loaded_packs:
if pack.source.get_path() == path:
return true
return false
func load_resource_pack(
path: String,
trusted: bool = false,
load_position: int = -1
) -> bool: # We can also use Error here, if we don't mind adding a few constants
# First, check if this path is already loaded:
if is_loaded(path):
return false
# The path supplied isn't loaded, so let's load it!
var new_source = ResourceFilesystemLoadedPack.new()
# <magic goes here for figuring out and instantiating the correct `ResourceFilesystemSource*` class>
# This is also probably where we'd return any IO errors that we encounter (file not existing, for example)
# Make sure to set the trusted state!
new_source.trusted = trusted
# We want to respect `load_position` here.
if (load_position > 0) and (load_position <= loaded_packs.size()):
# `load_position` was specified and is valid (not 0, not negative, not above the size of the array)
loaded_packs.insert(load_position, new_source)
elif load_position == -1:
# `load_position` was not specified, or was specified as -1
loaded_packs.push_back(new_source)
else:
printerr("Error loading pack: Invalid load_position specified")
return false
return true
func unload_resource_pack(
idx: int
):
if idx == 0:
# We don't want to unload the main game's resources.
printerr("Main resource pack (index 0) cannot be unloaded")
return
if (idx < 0) or (idx >= loaded_packs.size()):
# Out of bounds index
printerr("Attempted to unload resource pack at out-of-bounds index " + str(idx))
return
loaded_packs.remove(idx)
func inspect_pack(
pack_path: String
) -> Variant:
# We most likely wouldn't use the normal loading procedure here. Instead, we'd probably
# use a function that returns a `ResourceFilesystemLoadedPack` that hasn't been added to
# the `loaded_packs` array. However, for simplicity, I'm going to use `load_resource_pack()`
# here. Just do me a favor and assume that the pack we specify will always end up at the
# end of the `loaded_packs` array.
#
# (I'll probably clean up this code later!)
load_resource_pack(pack_path)
# This has the side effect of unloading it, but ideally we'd get the `ResourceFilesystemLoadedPack`
# from whatever function we use to load it.
var temporary_pack = loaded_packs.pop_back()
# Get `pack.json` and parse it into a Dictionary
if temporary_pack.source.file_exists("pack.json"):
var json_byte_array = temporary_pack.source.read_file("pack.json")
var json_string = json_byte_array.get_string_from_utf8()
var pack_info_dict = parse_json(json_string)
return pack_info_dict
else:
return ERR_FILE_NOT_FOUND
################################################################################
func get_loaded_pack(
idx: int
) -> ResourceFilesystemSource:
return loaded_packs[idx].source
func get_loaded_pack_count() -> int:
return loaded_packs.size()
func get_loaded_packs() -> Array:
return loaded_packs
################################################################################
func get_resource(
path: String
) -> Variant:
# We want to check the loaded packs in reverse order. Easiest way is to invert
# an array returned from range().
var inverted_pack_order = range(loaded_packs.size())
inverted_pack_order.invert()
var returned_resource = null
for i in inverted_pack_order:
if loaded_packs[i].enabled:
var returned_resource = _get_resource_from_pack(
i,
path
)
# If we got a resource, return it
if returned_resource:
return returned_resource
# We didn't get a resource, so we return `null`.
return null
func get_resource_from_pack(
pack_idx: int,
path: String
) -> Variant:
return _get_resource_from_pack(
pack_idx,
path
)
# Internal function, not exposed to GDScript
func _get_resource_from_pack(
pack_idx: int,
path: String
) -> Variant:
var current_pack = self.loaded_packs[pack_idx]
var loaded_resource = null
# So, for this function, I'm going to be using pseudo-code for the purposes of summarizing. If you spot
# a function being called that doesn't exist in this reference code, assume it does what I describe
# in the comments just above it.
# Firstly, we should check the resource cache:
if current_pack.resource_cache.has(path):
loaded_resource = current_pack.resource_cache.get(path)
else:
# Since it doesn't exist in the resource cache, we need to load it.
# If the pack is untrusted, and the requested path's extension may refer to a Script resource (for
# example, if the filename ends in `.gd`), we preemptively return `null`.
if !(current_pack.trusted) and path_extension_refers_to_script_resource(path):
return null
# Otherwise, we continue on by checking for the file's existance:
if current_pack.source.file_exists(path):
# Since the file supposedly exists, we grab the file. Then we load it using the appropriate
# ResourceFormatLoader. We should ensure that any ResourceFormatLoader we call here knows
# if a resource comes from an untrusted pack. This allows the ResourceFormatLoader to know
# if it should ignore built-in scripts.
loaded_resource = load_this_content_to_a_resource(
current_pack.source.read_file(path),
current_pack.trusted
)
# As one final precaution before we return the resource, we check if the current pack is trusted, and if the
# loaded resource is a Script. If both of those conditions are true, we instead return `null`.
if !(current_pack.trusted) and (loaded_resource is Script):
return null
# Finally, we add the loaded resource to the resource cache (if there was even a resource loaded to begin with:
if loaded_resource and !(current_pack.resource_cache.has(path)):
current_pack.resource_cache.add(
path,
loaded_resource
)
return loaded_resource
################################################################################
func set_loaded_pack_enabled(
idx: int,
enabled: bool
):
if idx != 0:
loaded_packs[idx].enabled = enabled
func get_loaded_pack_enabled(
idx: int
) -> bool:
return loaded_packs[idx].enabled
func get_loaded_pack_trusted(
idx: int
) -> bool:
return loaded_packs[idx].trustedResourceFilesystemSourceextends Reference
################################################################################
func get_file_list() -> PackedStringArray:
# Must be implemented by inheriting this class.
pass
func get_path() -> String:
# Must be implemented by inheriting this class.
pass
################################################################################
func file_exists(
path: String
) -> bool:
# Must be implemented by inheriting this class.
pass
func read_file(
path: String
) -> PackedByteArray:
# Must be implemented by inheriting this class.
pass |
|
If every resource pack is loaded as untrusted and Scripts are simply ignored, what happens to all the missing Script resources and subresources when I change the pack to trusted? No matter how much I think about it, I can't come up with any (good) way to load them after the fact, especially when some scenes have already been instantiated (and maybe modified) without the Script. Instead of allowing users to change the trusted status of a resource pack at runtime, I propose that this is set directly when the pack is loaded via an additional optional argument (defaulting to not trusted). In addition, some sort of inspect_pack(path: String) function that reads a pack.json file in a pack's root directory would be useful. This way devs could get both generic (e.g. authors, trusted_required, dependencies, name, version,...) and game-specific information about packs before (and after) they load them. |
Calinou
changed the title
@Ansraer Okay, this is a good point. I had left it as I did thinking that there might be some reason we would want to trust a pack after the fact, but in hindsight I hadn't actually tried to think of any reasons. And now I imagine any situations I do think up would be few and far between. I think I'll take your suggestion:
and change my proposed implementation to match.
Good suggestion! Though I'm not sure if I want to change my proposed implementation to include this, as then we would want to include facilities in the editor to specify what info goes into this |
I think you will have to. Without it there is no way for people who use gdscript to get information about provided packs. They would either have to load, get info, unload the entire pack (significant performance overhead) or write a hacky workaround with the ZipUtils (which might break when the pack is signed/encrypted). Plus, having additional information should allow us to catch certain bugs before they happen while also providing more in depth crash logs. |
|
@Ansraer Before I tackle that issue, I want to go back to the issue of handling trusted state. There is something important I missed making my first reply. I did some thinking, and I actually realized one issue with changing my implementation to match what you described (trusted state is set at the time of loading rather than at any time). Specifically, requiring that the developer unload and reload the pack, it can cause issues with load order. In its current state, newly-loaded packs are checked first for resources -- but if we're simply changing the trusted state, we may not want to change where in the list that mod is. That said, I think it might be an easy fix: Adding a |
|
Oh, yeah. Totally forgot about that. An optional load order argument sounds like a good solution. Should we ever get around to including additional information in packs we could also provide a list of dependencies, which could be used to try and automatically figure out the correct load order. |
|
@Ansraer Not a bad idea, to be honest -- but that means we'd have to deal with how to specify that list of dependencies. Unfortunately, I'm not sure we can do that in a game-agnostic manner. But even if we could, I think it would be best figured out down the road. Anyways, I'll add It's likely that we'll want to change that return value to something else -- perhaps it returns a |
|
I tackled this problem over the last few days and came up with a simple solution with very few changes. Now this hasn't been battle-tested nor I'm not knowledgeable enough to ascertain if this is a proper approach to this but, nonetheless, it currently works with what I expect to use this feature for. I hope this sparks better ideas on how to implement this. From now on I will refer to resource packs loaded in this manner as DLC. In a similar way I have named the classes like this too. I welcome suggestions for more general purpose class names. Here it is in a single commit: ChangesI added 3 classes to core Godot. PackedDLCData, DLCPackResourceLoader and DLC. PackedDLCDataPackedData is the filesystem of a Godot game or program. It contains (hashed) mappings of filesystem paths to offsets to Godot data files. Godot uses a single PackedData object. If it loads a new resource pack and such resource pack contains duplicated file paths, it may replace the old or drop the new file references. PackedDLCData comes into this by creating additional filesystems for each DLC the game loads. That means the conflict of repeated file paths among DLC packs is eliminated however, DLC packs can't replace the game's filesystem paths, because the latter has priority. DLCPackResourceLoaderOnce you load a DLC pack you get a DLCPackResourceLoader object that you have to use to load the imported resources. This class activates and deactivates the DLC pack resources this object internally refers to. DLCThis is a singleton with functionality similar to ProjectSettings::load_resource_pack. It is the entry point of dealing with DLC packs. PackResource (Not included)There is an extra class that I didn't include but is from a custom module in my Godot build. It is similar to DLCPackResourceLoader in that it loads resources from a DLC pack but it is a resource itself that is in the DLC. It's meant to describe and load a DLC element (a composition of scenes and resources). Discovering DLC contentEach project should handle this in the way that fits them the best. In my case, I expect DLC to have res://resources.list which is a simple text file with a PackResource resource full path in each line. Addressing some proposals from this issue
Loading is implemented and unload is easily doable too
Besides not loading a resource pack twice (this implementation doesn't) what is the need for this?
DLCPackResourceLoader does this
You can only load resources from packs you explicitly desire to load from
I too wish we could have limited ClassDBs for user generated DLC packs.
Since DLC can have repeated file paths that don't conflict with the base game resources, this can be provided by a known file in the DLC root. But if you must, as in you want to avoid loading a resource pack, you can prepend or append meta-data to a pack file. That is, you can load resource packs with a file offset and use the additional file bytes for meta-data. |
|
@chottokite By the way, I think we should use the term "ResourcePack" instead of "DLC" as run-time-loaded packs may not necessarily be downloaded from an external source. It's longer, but I think the added consistency is worth it 🙂 |
@chottokite There isn't a specific need so much as I think it would be a good idea to have. Even if you expose some sort of
Maybe I'm misunderstanding, but this sounds like it conflicts with the original goal of this proposal. Specifically, I want to allow all enabled resource packs to be treated as a single filesystem (as we already do when loading resource packs) when using
I'm not sure I understand. What does the ClassDB have to do with this? I thought this only affected ResourceLoaders.
I don't really see the need for that, at least in regard to the first point. The first point regards things already inherent to a PCK file, like its file path, its file list, and so on. However, I could see such an idea working for the second point. The second point would be better described as "metadata", so using some extra bytes in the PCK file would work for that purpose. But then I wonder -- if we're going to allow loading a resource pack using an offset, why not just implement a "Metadata" tab to the Export dialog, and have that attached to the PCK in some way? But at that point, I start to think that my "metadata" idea would be better off as its own proposal, separate from this modding-focused proposal. That all said, the rest of your implementation looks nice, and honestly sort of promising. Thank you for sharing! :) |
|
We discussed this proposal in a meeting. We agreed that adding methods to unload a resource pack (and check if a resource pack is loaded) is welcome. However, scoped per-file loading and sandbox support is best left for later and should be discussed in a separate proposal (see also #4495). |
|
I'm interested in implementing this; it's a pretty important feature for a project I'm working on which has a core focus on being able to load user-generated content. I will, however, only focus on implementing the functionality to unload packs (and checking if a certain pack is loaded), mainly because I'm not familiar with modding concepts and paradigms, so it's best for someone else more experienced to handle that aspect. |
Sounds good to me 🙂 |
YuriSizov
moved this from In Discussion
to Ready for Implementation
in Godot Proposal Metaverse
Describe the project you are working on
I want Godot to have better modding support.
Describe the problem or limitation you are having in your project
While it is possible for additional resource packs to be loaded in via
ProjectSettings.load_resource_pack(), the current system does not allow unloading of resource packs, meaning that there is no way to unload mods beyond restarting the game (which can be a pain for large-scale games with lots of resources).Additionally, while the current system offers a
replace_filesparameter so we may choose if a mod can "overwrite" the game's files, this actually causes a much bigger problem. Namely, if the game and a mod contain a file at the same file path (let's say,res://enemies/list.txt), only one or the other can be accessed (and this problem only gets worse the more mods we add).Describe the feature / enhancement and how it helps to overcome the problem or limitation
I propose that we create a system where loaded resource packs are treated separately from each other. By doing so, we will be able to solve certain issues with moddability, such as:
and so on.
Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
I propose that we expose functions for the following:
Load and unload resource packs.
Check if a specific path is already loaded as a resource pack.
Retrieve resources from loaded resource packs.
@GDScript.load()norResourceLoader.load()wrap, is up for debate. However, I'm including this here for the sake of completeness.That said, if this is a new function, it should not deprecate other resource-loading functions.
Retrieve resources from a single loaded resource pack, even if the pack is not enabled.
Set a loaded pack as "disabled" (without unloading it), preventing resources from being loaded from the resource pack, unless the pack is specifically targeted.
Set a loaded pack as "untrusted", preventing scripts from being loaded from that pack (even if the pack is specifically targeted).
Obtain various pieces of information on a single loaded pack and its files (such as a loaded pack's path, file list, and the existence of certain files).
Obtain a file from the root of an unloaded pack, without needing to load the pack first, that would provide various bits of information to the game... such as pack name, pack author, pack version, dependencies, and so on.
If this enhancement will not be used often, can it be worked around with a few lines of script?
No, not really. The limitations I described in the "Describe the problem or limitation you are having in your project" section are not really possible to overcome through GDScript.
Is there a reason why this should be core and not an add-on in the asset library?
It's technically possible for this to be handled as an add-on... but I don't think this would work too well as just an add-on. Plus, even if 90% of Godot games might not implement modding, this could make modding much more flexible for the other 10%.
The text was updated successfully, but these errors were encountered: