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 broken built-in script reloading #92177

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented May 20, 2024

Fixes #92157
I have no idea what this changes, I only figured that doing this will fix both the error and reload issue.

If default reload() is used, this condition

ERR_FAIL_COND_V(!p_keep_state && has_instances, ERR_ALREADY_IN_USE);

will prevent properly loading the script for reasons unknown.

@KoBeWi KoBeWi added this to the 4.3 milestone May 20, 2024
@KoBeWi KoBeWi requested a review from a team as a code owner May 20, 2024 21:59
@huwpascoe
Copy link
Contributor

I have no idea what this changes

This PR is covering up whatever the real bug is, there should be a comment like // TODO: I have no idea why this fix works if it's gonna be merged!

@akien-mga akien-mga changed the title Fix broken script reloading Fix broken built-in script reloading Jun 10, 2024
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Couldn't get reviews in a timely manner, but since this fixes a regression, I'll yolo merge and we'll see if anything breaks (and be prepared to revert this if it's problematic).

@akien-mga akien-mga merged commit 6c3f811 into godotengine:master Jun 18, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the literally_wtf branch June 18, 2024 16:19
@permelin
Copy link
Contributor

A comment for the future in case this turns out to break something.

I've been running this same fix locally in my own branch since April, but for a different reason. It fixes another problem:
If I changed a .gd file outside the editor, while the editor was running, but without an active LSP connection, I also got ERR_ALREADY_IN_USE and I had to restart the editor for it to reload the script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Built-in scripts of child nodes in editor require editor restart to reload
4 participants