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

GDScript: Hot-reload changed scripts only #86676

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

rune-scape
Copy link
Contributor

hot reloading currently reloads all scripts
when working with dosens of scripts, it means a small change can take take a while, even tho you just wanted 1 script reloaded
with this PR only changed scripts are reloaded
was not sure how to change godot_icall_Internal_EditorDebuggerNodeReloadScripts csharp interop, so i left the behavior unchanged

core/object/script_language.h Outdated Show resolved Hide resolved
@rune-scape rune-scape force-pushed the sparse-script-reload branch from f1fa6d7 to cde478b Compare January 3, 2024 01:57
@rune-scape rune-scape requested a review from vnen January 3, 2024 04:56
Copy link
Member

@vnen vnen left a comment

Choose a reason for hiding this comment

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

LGTM

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jan 15, 2024
@@ -478,6 +478,7 @@ class CSharpLanguage : public ScriptLanguage {
/* TODO? */ void get_public_annotations(List<MethodInfo> *p_annotations) const override {}

void reload_all_scripts() override;
void reload_scripts(const Array &p_scripts, bool p_soft_reload) override;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
void reload_scripts(const Array &p_scripts, bool p_soft_reload) override;
virtual void reload_scripts(const Array &p_scripts, bool p_soft_reload) override;

This isn't done consistently here but should be done here when adding new entries IMO, it's done on some methods already above

Copy link
Contributor

Choose a reason for hiding this comment

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

This would only matter if someone was to extend CSharpLanguage, which is highly unlikely. So I'd say this is a non-blocking issue.

Copy link
Member

@AThousandShips AThousandShips Jan 17, 2024

Choose a reason for hiding this comment

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

No I mean that we always use virtual and override as a matter of style regularity, unless it's marked final it doesn't matter for that sense

@YuriSizov YuriSizov merged commit c027aec into godotengine:master Jan 17, 2024
15 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

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.

5 participants