Add small chapter for registering ResourceFormatLoaders/Savers.

[MrJoermungandr]

Hey as I said on discord the small "guide" for the ResourceFormatLoader and Savers.
I've taken a slightly different approach on the writing style and instead tried to provide a pretty all boilerplate included example which you can easily copy and paste to just get it working in your projects.
If this is not desired and a more tutorial like approch is preffered i can also rewrite it in that style.
I also wasnt entirly sure because this is by no means the most minial example because in theory you can just directly register the Saver/Loader in the gdextension trait impl but idk feels wrong since you "leak" on exit.
Lemme know any formulation or clarity problems :)

You can also freely edit it to your desire if you want to.

Thanks in advance :>

[Bromeon]

Thanks a lot for this nice addition, and sorry for the late review!

I cleaned up some parts, some feedback for next time:

• I'd appreciate if you could run a spell/grammar checker on it. Should be very little work with today's AI tools.
• Would also be nice if the CI passed 😉 there are some limitations on the line length etc.

---

I've taken a slightly different approach on the writing style and instead tried to provide a pretty all boilerplate included example which you can easily copy and paste to just get it working in your projects.
If this is not desired and a more tutorial like approch is preffered i can also rewrite it in that style.

I think it's mostly OK, but I'd still split at least the different classes and give a short introductory text to each. So you'd have 4 code blocks:

• ExtensionLibrary
• MyAssetSingleton
• MyAssetSaver
• MyAssetLoader

They could still be copy-pasted individually, but then it's not a massive wall of code hitting the reader.

---

Some comments about the code and contents:

1. Unregistering singleton

pub fn unregister_input_output() {
    Engine::singleton().unregister_singleton("AssetSingleton");
}

This is still a memory leak. You unregister the singleton but don't free the instance. See Singleton chapter...

You also mention

and on exit, ClassDB will clean up for you

Are you sure that's true? Godot will hint at some leaks as a best-effort, but I'm not sure if it's guaranteed.

And it's borderline "bad practice" so I would not even suggest that, as it encourages not cleaning up properly.

2. Resource detection

// The stringified name of your resource should be returned.
    fn get_resource_type(&self, path: GString) -> GString {
        // This is a slight hack to check if the file has the right extension.
        // You can change this to your heart's content.
        if path.to_string().ends_with(".myextension") {
            "MyResourceType".into()
        } else {
            // In case of not handling the given resource, it must return an empty string.
            GString::new()
        }
    }

Can you elaborate why this is a hack and how one would do it properly?

Thanks for mentioning the empty string behavior, that's very helpful 👍

3. Saving and loading logic

// TODO: Put your saving logic in here, with the FileAccess API.

I would rather mention GFile in this context, maybe even include the link in code.

It would also be nice if the tutorial/page about saving loading would quickly touch on the semantics of the save and load functions. When I see these signatures...

    fn save(
        &mut self,
        resource: Option<Gd<Resource>>,
        path: GString,
        flags: u32,
    ) -> godot::global::Error

    fn load(
        &self,
        path: GString,
        original_path: GString,
        use_sub_threads: bool,
        cache_mode: i32,
    ) -> Variant

...I have some questions as a reader:

• In save(), what meaning and type does the flags parameter have?
• In load(), what what do all the parameters mean?
• What should load() return -- is there a reason why the type is not Gd<Resource>?
  • Genuine question, I'm not sure if that's just bad API on Godot's side or some other reason for the variant.
• It's not quite clear to me why save() takes &mut self and load() only &self
  • Again, there might not be a deeper reason, but it's also good to know if that's something we should correct in our own API...

---

After filling in those gaps, it would be a very comprehensible page! Let me know if you need help completing these parts. Thanks a lot!

[MrJoermungandr]

Thanks for the review and the suggenstions/feedback! No problem that it took so long i understand its just not a big priority ;)

Ye fair points was originally intended as a draft as i was certain that some things needed improvement since your very thourough from what i've seen ^^ (good thing imo).

Splitting the blocks up is so obvious and i completly agree havent thought of that possibility. will do

1. Unregistering singleton

Your right i completly forgot that Singletons are manually managed. will fix

I was contemplating myself whether to include the last statement regarding the necessity when i wrote it. Now i think we should delete it since if your looking for a guide it should be a bit opinionated and following good/the best practices. If you really want to you can f around yourself and find out eventually that its not necessary. But as written it doesnt add any value.

From the angle of are you sure if this is true? No. I actually arent 🥴 i'm certain that the operating system will clean up and it is worded misleading and badly on my side mb. Was writing it because i thought its easier to grasp but ... well that doesnt help anybody. In turn it could be worded with something along the lines of: The downside of this approach is that the destructor does not run which entails that if you depend on the destructor it is possible to have bad side effects if not the operating system will take care of reclaiming the memory.
As mentioned in the paragraph above let's just not include this if you agree since imo it doesnt offer any value whatsoever.

2. Resource detection

Right i failed again to do proper reasoning lol.
The reason i called it "hack" is because Godot internally uses their GString get_extension() method. Taken from the GDScript loader:

String el = p_path.get_extension().to_lower();
	if (el == "gd" || el == "gdc") {
		return "GDScript";
	}

I didnt use this feature because it was not available at the time of writing but you added the methods recently sooo it's not necessary anymore and we can just mirror godot with get_extension().to_lower() and remove the comment.
Funnily enough i have literally no idea how they dont use something along the lines of .ends_with() because when i print the input it shows the full path so i theory the GDScript loader shouldnt even work? Edit: My brain was fried we of cource call get_extension() so obvious how we get the extension lol
(btw GString is just too funny of a name for the type i should start telling my homies i have a collection of gstrings xD)

3. Saving and loading logic

I actually wasn't familiar with the GFile class probably glossed over it woops.
Seems nice and it definetly makes more sense to use the wrapper there.

The signatures are indeed a bit confusing but i thought because they are virtual methods a user would just look up the godot docs but maybe we just copy the documentation above since the Godot docs also just offer the bare minimum.

• In save(), what meaning and type does the flags parameter have?

1. Should probably get a special case because it actually takes SaverFlags.

• In load(), what what do all the parameters mean?

1. path: the path for the loading request where the plugin has to look at.

2. original_path: if the path was a result of a import step (think .png -> godot representation) you can get the original file with the original_path. In fact those two arguments are the exact same if not obtained through a import.

3. use_sub_threads: I tried getting to the bottom of this but looking at the source im lost tbh and im running out of time. Heres my search path in case anybody also wants to give it a try:

   1. GdScript calls ResourceLoader.load()
   2. Godot obtains a LoadToken through _load_start()
   3. If successfull it start actually loading through the _load_complete() function
   4. This will do some wild threading stuff since multible thread can call load() for the same resource
   5. If we succeed some checks that we good to go we advance into _run_load_task() where we actually actually start loading.
   6. There we do some thread syncing again if all is good we actually actually actually start loading with the _load() function of the correct ResourceFormatLoader
   7. So now we arrived and have no further info sadge but its actually defaulted to false
   8. It only gets injected when we call ResourceLoader::load_threaded_request() with true.
   9. when we enter true as a arguemnt for load_threaded_request it does weird this like casting the bool to an enum which can be DISTRIBUTED and if it is it sets the property use_sub_threads from ThreadLoadTask to true again which then somehow gets passed on or whatever.
   10. Since this is still useless i should probably look at how gdscript does it. Well turns out they completly ignore that property 😭. -1.5hrs digging into this i guess but was kinda funny nonetheless. Maybe its not ignored in other cases but i dont wanna research this any further atm
   11. I conclude this can be safely ignored and we should probably add a comment that we dont know what the shit is going on and it is recommended to not worry about it since stuff actually breaks?

4. This should take a CacheMode enum. Maybe also special case?

• What should load() return -- is there a reason why the type is not Gd?

Consider the following information your fault since you made me look:
If the Variant is a integer it actually means it is an error and the integer gets parsed as a global_scope::Error code. otherwise parse it as a resource xdd

If this gets a special case anyway because of the SaverFlags mentioned above and it's trivial to implement you could consider changing this to a Result.

• It's not quite clear to me why save() takes &mut self and load() only &self

I've sadly run out of time for this one ^^

I've written more for this today than for my bachelors thesis xD maybe im sneaking the investigation in there haha.
Since the deadline is getting closer and closer i dont know when im able to actually implement the changes i've wrote about probably next tuesday evening. If you feel like finishing it you can also do it :)

Thanks for your continuus effort and a great evening!

[MrJoermungandr]

As promised the little overhaul :)

I actually did some more reaserch as well as a little bug adventure since the Saver was actually not working as intended.
I have added a comment on the registration of the saver.
This behaviour is currently not working and relies on this pr in godot to actually be acurate. (But i really hope this gets merged quickly)

On the topic of special casing those functions/classes it would probably also be good if you special case to remove the Options for the resource parameter in recognize(), get_recognized_extensions() and save() because Godot should only calls these with actual resources.

Would be great if you could apply your last bit of feedback if any and squash the commits for me if its not too much to ask ^^

Have a great evening!