Fix sub-resource IDs resetting when preloaded

[KoBeWi]

Probably fixes #62258, idk what the issue is about at this point

I had a case in my project where I had a scene A preloading scene B. If I opened A first and then opened B, all B's sub-resource IDs would reset and the built-in script would show as [unsaved][*]. It might be the clue of all sub-resource issues reported in the issue.

The fix I did is not probably correct (the code seems to have been like this for a very long time, so changing it might have some unwanted side-effects), but it fixes the issue.

EDIT:
I came upon this comment
#71171 (comment)
which basically confirms that it isn't the right fix. But if the cache is ignored, loading the same resource again should replace the resource from ignored load (to make sure it has a path this time).

[akien-mga]

Needs rebase to pass CI, I fixed the GDScript test error.

[akien-mga]

Tentatively approving to keep track of it as high priority :) But still needs @reduz to check it.

[reduz]

The problem is that, in many cases, we use CACHE_IGNORE to convert resources on background (on the exporter, as an example) and this change will break the ones currently loaded and edited.

I think the right fix is a bit simpler here, just change this code:

if (do_assign && cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE) {
	res->set_path(path, cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE);
	res->set_scene_unique_id(id);
}

by

if (do_assign && cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE) {
	res->set_path(path, cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE);
}
res->set_scene_unique_id(id);

This way, when the resource is re-saved it will keep the unique ID, but you don't necessarily kill the existing resource when using the CACHE_IGNORE mode.

[reduz]

After further discussion, it appears that what we need here is to separate the resource cache and the subresource cache modes in the ResourceLoader::load function. I will work on a PR for this soon.

[KoBeWi]

After some investigation, the original issue is caused by this line:

godot/modules/gdscript/gdscript_cache.cpp

Line 367 in 0a9e6e4

scene->set_path(p_path);

Resource loaded with CACHE_MODE_IGNORE takes over a path and causes id reset. The solution mentioned in the above comment would fix it, but I think this is not really a correct behavior, idk.

[KoBeWi]

Closing. This is wrong solution and the actual issue was identified to be different.

[KoBeWi]

Reopening, as fixing the internal ids is high-priority matter and this works as a temporary fix.

[akien-mga]

Thanks!

[ashelleyPurdue]

You mentioned that this was probably not a "correct" fix because it breaks something else. What is this other thing that it breaks, out of curiosity?

[KoBeWi]

That was before I updated the PR. It used to set path for resources that ignore cache (i.e. they are supposed to be temporary). Now it only sets ID, which shouldn't break anything.