-
-
Notifications
You must be signed in to change notification settings - Fork 21.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Assign temporary path to preloaded resources #80281
Conversation
core/io/resource.h
Outdated
@@ -103,6 +103,7 @@ class Resource : public RefCounted { | |||
|
|||
virtual void set_path(const String &p_path, bool p_take_over = false); | |||
String get_path() const; | |||
void set_temporary_path(const String &p_path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have not looked in depth but since its hacky, maybe better to use a metadata, then clear it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clear when? Assigning temporary metadata is prone to getting it saved accidentally.
(on a side note, it would be nice if we had a way to add metadata that isn't saved, like with a special prefix)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would name it for what it does.
void set_temporary_path(const String &p_path); | |
void set_path_cache(const String &p_path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems relatively safe. Let's merge and test it in the next beta.
Thanks! |
Well, that code isn't known to cause problems, so it wasn't necessary. This solution is basically a hack, so it should be used only where relevant. |
Given your current fix, I'm not really sure why those instances above wouldn't cause problems. Right now, you're assigning the path_cache to both the internal resources and the main resource in resource_format_binary, but in resource_format_text, you're only assigning the path_cache to the internal resources. Unless you intended to only assign the path_cache to the internal resources in resource_format_binary? |
I'm not, the code I added for both is the same. |
Fixes #72854 in a safe and very hacky way.
The issue is waiting for a "proper" fix (see the discussion in #72257), but it kept annoying me in the meantime so I'm coming with another hack 🤷♂️
I added a method to Resource that sets the path completely bypassing the cache, so
Resource.get_path()
will return the "correct" path, while not causing unwanted side-effects. The method is called for built-in resources loaded with CACHE_MODE_IGNORE.It was not enough, because GDScript keeps its own
path
, set with the regularset_path()
method. I could makeset_temporary_path()
virtual, but instead I added a flag to track whether the path is valid and try returningget_path()
if it's not.Here's an extended MRP for the debugger bug that this fixes:
PreloadBug.zip
Preloaded built-in scripts still can't be clicked for some reason, but at least you can see their path.
EDIT:
Fixes #80812