Fix missing UID handling in Dependency Editor #73131
Conversation
| if (!fallback_path.is_empty()) { | ||
| path += "::" + fallback_path; | ||
| } |
There was a problem hiding this comment.
How will code using this differentiate between type and fallback_path when reading what comes after ::, in case both p_add_types on/off and non-empty fallback_path can happen?
There was a problem hiding this comment.
I guess there is no way to differentiate that 🙃
But given how fallback_path is used, it's not a big problem if it's invalid. Maybe I should add more comments about its caveats? It doesn't need to be super robust for now, as it does its job.
There was a problem hiding this comment.
Ok I changed it to just add another :: if p_add_types is false, so type and fallback are at fixed positions and can be retrieved reliably.
There was a problem hiding this comment.
Tentatively approving, would be good to merge for wider testing (that was the plan originally with postponing it to 4.1, but it went under the radar again...).
Could you rebase for good measure? (And double check if this is still the way you think this bug should be solved.)
KoBeWi
force-pushed
the
decency_editor
branch
2 times, most recently
from
85afb2a
to
237ee80
Compare
|
Rebased and made a minor tweak related to the review comment. |
|
Needs a rebase to fix CI, the doc checks were broken temporarily yesterday by another PR. |
|
Thanks! |
|
This PR seems to be too risky to include into the stable 4.0 branch at the moment, as it requires further fixes from follow-up PRs and more time dedicated to battle-testing it. We are not going to cherry-pick it, but if there is a strong need to have this fixed in a future 4.0.x release, please make a dedicated PR with all the follow-up fixes applied once it has been tested for at least a few months. Otherwise, we recommend updating to Godot 4.1 if this fix is important to your project. I'll remove the cherry-pick label. |
Fixes #63366
When the scene had UID and path as a dependency, it preferred listing the UID. This can however fail if the UID does not path to any path (not sure if possible normally; it's definitely a problem when copying files between projects).
I made the text loader add path as an optional dependency field and Dependency Editor will use it if UID fails.