ResourceLoader: Fix handling of uncached loads

[RandomShaper]

• CACHE_MODE_IGNORE_DEEP is checked in addition to CACHE_MODE_IGNORE to determine if a load is uncached. This avoids crashes in uncached loads due to prematurely freed load tasks.
• Cached load tasks are isolated (not registered in the task map ever). This avoids regular loads from reusing in-flight cached loads, which is not correct.

[akien-mga]

Thanks!

[mrTag]

Since we merged in the latest master a few days ago, we had errors in our game every now and then (like "play the game for at least 5 minutes and they started occurring" kind of frequency). They were in the resource_parser_text and errors like "could not parse" or "error loading dependency" or even "no resource found at path". We use the threaded resource loader quite a lot, so this could be a rare problem for others...

We traced the problem back to this PR through bisecting. And while looking at the changes I found it odd that the new code returns the load_token even when it is invalid:

godot/core/io/resource_loader.cpp

Lines 476 to 481 in 82cedc8

	if (!load_token.is_valid()) {
	// The token is dying (reached 0 on another thread).
	// Ensure it's killed now so the path can be safely reused right away.
	thread_load_tasks[local_path].load_token->clear();
	}
	return load_token;

while the old code only returned in an else (so only when the token is valid) and otherwise populated the load_token later on in the function before returning:

if (!load_token.is_valid()) {
	// The token is dying (reached 0 on another thread).
	// Ensure it's killed now so the path can be safely reused right away.
	thread_load_tasks[local_path].load_token->clear();
} else {
	if (p_cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE) {
		return load_token;
	}
}

So I tried just adding the else back to the condition and the errors went away:

if (!load_token.is_valid()) {
	// The token is dying (reached 0 on another thread).
	// Ensure it's killed now so the path can be safely reused right away.
	thread_load_tasks[local_path].load_token->clear();
} else {
	return load_token;
}

I am totally unsure if that is correct in any way, though 😀 Can you look into this @RandomShaper ?

[RandomShaper]

Excellent catch. I'm preparing a PR that fixes a few other issues and will include the fix there. Thank you.