Skip to content
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

Fix resource loader not resolving shallow loaded scripts through dependencies (reverted) #96499

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

beev1s
Copy link

@beev1s beev1s commented Sep 2, 2024

When a script first gets shallow loaded through a dependency and and then requested by the resource loader, that script is already registered in ResourceCache and will therefore not be properly reloaded, which is necessary in order to resolve any Script inheritance. This can lead to errors when assigning objects with that script to a reference of its parent.

GDScript now overwrites set_path_cache which does not register it in the RessourceCache. set_path_cache replaces a call to set_path in get_shallow_script. get_full_script now calls set_path to register the script.

@beev1s beev1s requested review from a team as code owners September 2, 2024 22:10
@dalexeev dalexeev added this to the 4.4 milestone Sep 3, 2024
core/io/resource.h Outdated Show resolved Hide resolved
@beev1s beev1s force-pushed the shallow-script-cache-error branch from c97fdea to fd5fc9f Compare September 14, 2024 15:22
Copy link
Member

@dalexeev dalexeev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me. I tested it, this PR fixes the linked issue.

@beev1s Note that GitHub will not count this as your contribution. See GitHub Docs for details.

@dalexeev dalexeev requested review from a team and removed request for a team October 10, 2024 15:39
@RandomShaper RandomShaper added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Oct 22, 2024
@AThousandShips AThousandShips changed the title Fixed resource loader not resolving shallow loaded scripts through dependencies Fix resource loader not resolving shallow loaded scripts through dependencies Oct 22, 2024
@Repiteo Repiteo merged commit 2584f75 into godotengine:master Oct 25, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 25, 2024

Thanks, and congratulations on your first contribution! 🎉

akien-mga added a commit to akien-mga/godot that referenced this pull request Feb 4, 2025
This reverts commit fd5fc9f.

This caused significant regressions which are worse than the bug that godotengine#96499
aimed to address.

- Reverts godotengine#96499.
- Reopens godotengine#95909.
- Supersedes godotengine#102063.
- Fixes godotengine#99006.
- Fixes godotengine#101615.
@akien-mga akien-mga changed the title Fix resource loader not resolving shallow loaded scripts through dependencies Fix resource loader not resolving shallow loaded scripts through dependencies (reverted) Feb 4, 2025
@akien-mga
Copy link
Member

Reverting with #102429 as this introduced complex regressions which are worse than the bug this was addressing.

@akien-mga akien-mga removed the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Feb 4, 2025
sondredl pushed a commit to sondredl/godot that referenced this pull request Feb 6, 2025
This reverts commit fd5fc9f.

This caused significant regressions which are worse than the bug that godotengine#96499
aimed to address.

- Reverts godotengine#96499.
- Reopens godotengine#95909.
- Supersedes godotengine#102063.
- Fixes godotengine#99006.
- Fixes godotengine#101615.
SirQuartz pushed a commit to SirQuartz/godot that referenced this pull request Feb 7, 2025
This reverts commit fd5fc9f.

This caused significant regressions which are worse than the bug that godotengine#96499
aimed to address.

- Reverts godotengine#96499.
- Reopens godotengine#95909.
- Supersedes godotengine#102063.
- Fixes godotengine#99006.
- Fixes godotengine#101615.
dugramen pushed a commit to dugramen/godot that referenced this pull request Feb 8, 2025
This reverts commit fd5fc9f.

This caused significant regressions which are worse than the bug that godotengine#96499
aimed to address.

- Reverts godotengine#96499.
- Reopens godotengine#95909.
- Supersedes godotengine#102063.
- Fixes godotengine#99006.
- Fixes godotengine#101615.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants