Enable users to react to save/load operations globally.

[willnationsdev]

Closes godotengine/godot-proposals#3232.

Initially implemented as adding signals to ResourceSaver and ResourceLoader. Upon receiving feedback from reduz below, it has been rewritten as a new virtual callback associated with the ResourceFormatSaver and ResourceFormatLoader.

[reduz]

The logic of signals is that they are not emitted when the user directly calls a function. As such, adding signals to ResourceLoader/Saver is out of scope. You control when this happens in the game, so you can call whenever you want.

If, on the other hand, you want to this on the editor, EditorFileSystem knows when a resource was saved and the filesystem tree updated, so you could add a signal there (if not present already).

[reduz]

I just checked, and there is void EditorNode::_resource_saved(Ref<Resource> p_resource, const String &p_path); which calls EditorFileSystem::update_file. Imo you could make it call a new function EditorFileSystem::file_saved_notify, which calls update_file and then emits the signal, so the signal is emitted when the file information of EditorFileSystem has been updated.

[Diddykonga]

Normally, I would agree with @reduz but having an Signal as part of the internal nodes functions works a lot easier when sharing code or working with librarys/addons as you know anyone/anywhere who is concerned with me loading/saving my resources, will be able to be notified about it.

[willnationsdev]

@reduz Exactly as @Diddykonga says. My use case is that I want an addon to be able to detect when a resource is saved or loaded by the user regardless of whether it is at runtime or in the editor. That way, it can do responsive actions regardless of HOW the user is choosing to save the resource. Yes, I can control when a specific resource is saved for my own project, but if I want to make a third-party system that adds custom save/load logic to other people's projects, I would have to expose some sort of singleton to implement my custom resource saving/loading logic. That would then force my users to use that singleton for all their resource operations. I want to avoid that by piggybacking off the core APIs they already use (the ResourceSaver and ResourceLoader). It can't be anything that has a dependency on the EditorNode or similar tooling.

If placing signals directly on the core-bound saver/loader singletons is out of scope, how should I adjust the implementation to support that use case? What options are there besides the singletons that everyone already uses in their scripts?

If we simply can't use signals, then what about leveraging the new "first-class functions" of the scripting API to allow people to attach handlers for save/load directly onto the ResourceSaver and ResourceLoader?

[derammo]

Resource loading can happen on many competing threads and is complicated as a result. Making an extension API that is safe to use may be tricky. Signals for example would be async or come from arbitrary threads which is bad API.

If you are just trying to support a different way of getting the resource data, maybe define protocol handlers instead? Like you register code for http:// or mystuff:// and then the Resource system can use your “driver” when actually accessing the data? It would be called from various threads of course but it would only call you from one thread for each URL since it handled the races already.

[willnationsdev]

@derammo

Resource loading can happen on many competing threads and is complicated as a result. Making an extension API that is safe to use may be tricky. Signals for example would be async or come from arbitrary threads which is bad API.

Thank you. This explanation makes it a bit more clear why signals aren't a good solution for this.

If you are just trying to support a different way of getting the resource data, maybe define protocol handlers instead? Like you register code for http:// or mystuff:// and then the Resource system can use your “driver” when actually accessing the data? It would be called from various threads of course but it would only call you from one thread for each URL since it handled the races already.

What I ultimately wanted was a way for users to define some Index type and some IndexProvider where IndexProvider describes how one or more saved resource(s) map to zero or more Index instances. Then these act as synced and queryable indexes for searching through and loading resources (w/ a GUI for building queries and displaying the resource data as a grid/spreadsheet).

Ideally, I wanted such a system to work with any type of resource the user might save or load since it's meant to be open-ended to begin with. For it to work as intended, it needs to be able to run the user's scripted IndexProvider logic on each saved resource regardless of how that resource was saved (I wouldn't have thought that the specific protocol of the saving operation should matter). And if I defined my own protocol handler, then I would have the same problem of locking users into saving stuff using my own custom addon-specific API rather than flexibly allowing users to keep using the core systems they would traditionally use.

If the issue is just about needing to resolve race conditions for the saved/loaded resource before triggering the extra functionality, isn't there a way we can do that?

[derammo]

I look at it this way:

Front End

The ResourceLoader and ResourceSaver registered singletons let various threads make a request for a resource by URI. Then some stuff happens that is opaque to the user (i.e. all the threading tricks and resource caching etc.) and then each caller gets back an answer. Some of those will be the same resource object, but that's not known to the callers. The actual loading might have been done on the back end by another thread but the front-end client doesn't know that.

Back End

Resources are actually stored in different providers that use different URI protocols. 'res://' is one of those. These serve resources when called in a straight synchronous way. If 3 front-end threads request the same resource, it would still only be loaded once here. Or not at all if it is in the ResourceCache.

Your Changes

I was saying you could solve the problem at the back end by defining a new protocol so 'myres://' or whatever could be registered with the ResourceLoader to mean to make the actual loading calls through your code. Then it would be instrumenting actual loads.

Or you could instrument the front-end. An extension API could allow you to register a different ResourceLoader and ResourceServer singleton, which you provide. Then those would instrument requests for resouces, not the actual loads and stores. You'd do your stuff and then call the default implementation ResourceLoader/ResourceSaver to do the work. You'd have to be thread safe re-entrant etc just like the default implementation.

I am overexplaining for you to clarify which one you are trying to instrument.

(also: this is all conceptual, not how the classes are split now)

[willnationsdev]

@derammo

I was saying you could solve the problem at the back end by defining a new protocol so 'myres://'

This solution wouldn't work well (I don't think), because then it would only be active if the user loads the resource using that protocol, i.e. it wouldn't automatically piggyback off any existing resource loading operations. It also wouldn't be something that can be incorporated into a variety of other protocols like res://, user://, or uid://.

Or you could instrument the front-end. An extension API could allow you to register a different ResourceLoader and ResourceServer singleton, which you provide.

Having an alternative loader/saver would work, but wouldn't be practical if any other addons/plugins/extensions wanted to do similar kinds of logic; those addons/plugins/extensions would need wrappers that take into account my own wrapper or vice versa, and I'd rather not have addons/plugins/extensions need to account for any external factors like that.

---

What we really need is some sort of middleware pipeline that extensions and scripts can contribute to which each protocol provider must then accommodate in its implementation to make the middleware relevant. That way, all the third-party developers have to do is register a ResourceHandler type and get the callbacks they need to be triggered w/o it actually being subject to the nuanced implementation of the (de)serialization process.

[willnationsdev]

I have modified the implementation of this pull request. Initially, I was going to add ResourceHandler types with _on_save and _on_load methods, but in the course of implementing their integration into the ResourceSaver and ResourceLoader respectively, I realized that they were functionally identical to the ResourceFormat* classes except for the fact that the loop for those class's methods stops early on the first matching format. So rather than add more complexity to the overall API and the singletons' configuration/state, I instead just added a single _on_save and _on_load method to the respective ResourceFormat* base types and modified the save and load operations so that they would additionally call these functions after successfully saving or loading a resource. This should satisfy the requirements without resorting to signals and without causing undue complications in the engine's low-level API.