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

New feature "reload externally changed scenes" not working for Built-in Scripts #47238

Open
mrimvo opened this issue Mar 21, 2021 · 12 comments
Open

Comments

@mrimvo
Copy link

mrimvo commented Mar 21, 2021

Godot version:
3.3 RC6

OS/device including version:
Ubuntu 18.04

Issue description:
When externally editing a .tscn file and changing a script, this script will not be updated when "reloading" the scene in Godot.

I came across this when changing a scene externally with git and trying to reload it in Godot.

Steps to reproduce:

  1. Open a .tscn file with an external Texteditor, change an internal (built-in) script and save.
  2. Switch back to godot. The dialog appears "files are newer on disk". Click Reload.
  3. open the built-in script. It didn't change. EXPECTED: script should show the changes made in the external editor.

Minimal reproduction project:
Just a One-Node Scene with a built-in script attached to the node:

[gd_scene load_steps=2 format=2]

[sub_resource type="GDScript" id=1]
script/source = "extends Node

# change this line with an external text editor"

[node name="Node" type="Node"]
script = SubResource( 1 )

Side Node
This might be a general problem not only related to Built-in Scripts, because I observed a similar issue with a color_ramp sub resource attached to a Particles2D node. I reverted a change with git, but after reloading, the particles did not change as expected. Not sure if this is related.

@Calinou
Copy link
Member

Calinou commented Mar 21, 2021

cc @KoBeWi

@KoBeWi
Copy link
Member

KoBeWi commented Mar 21, 2021

This happens because when you reload the scene, the built-in script reference might not get released (usually happens when another scene has this scene instanced or sometimes when the script is open in the script editor).

This is fixed on master (by #45903) and probably can't be fixed on 3.x as it's an inherent issue of built-in resources.

@akien-mga akien-mga added this to the 4.0 milestone Mar 23, 2021
@akien-mga
Copy link
Member

Closing as fixed in 4.0 then.

@mrimvo
Copy link
Author

mrimvo commented Mar 23, 2021

Not so quick!

As we discovered this feature is broken in 3.3 RC6. And we are actually promoting this feature as a major new feature for Version 3.3, all the while we know it's broken. So closing the issue in my opinion is not the appropriate action to take.

Auswahl_575
(source: godotengine.org)

I propose one of these ways to go from here:

  1. when reloading, and sub-resources couldn't be updated, at least tell the user about it. At least show a red error log message about that. Because right now, the user expects that feature to be working, no matter what.
  2. remove the feature alltogether. A partially reloaded scene is even worse than having the scene not reloaded at all
  3. maybe we could even close all scenes (thereby releasing all sub-resources) and reload them

@Zireael07
Copy link
Contributor

KoBeWi told you - the issue you are seeing is not fixable in 3.x.
Not having scenes reloaded at all was a big PITA for people using external editors and/or importing assets from Blender (this also affected inherited scenes)

@Calinou
Copy link
Member

Calinou commented Mar 23, 2021

See also #31758. While built-in scripts will remain available in 3.x, it is not certain whether they'll be kept starting from 4.0.

@mrimvo
Copy link
Author

mrimvo commented Mar 23, 2021

Thank you for your replies. I understand your points of view.

Not having scenes reloaded at all was a big PITA for people using external editors and/or importing assets from Blender (this also affected inherited scenes)

I'd love to have scenes reloaded too, as I'm working with git a lot. It's quite PITA not to have this feature, true ;)

I think everyone using this feature would like to get notified if something went wrong. I propose to at least print some error messages in the editor log stating which scenes (and which resources) couldn't get refreshed. This is not about fixing the problem, but mitigating it and making the thing usable.

While built-in scripts will remain available in 3.x, it is not certain whether they'll be kept starting from 4.0.

From what I understand, this is not a problem limited to built-in scripts, but to built-in resources in general.

@KoBeWi
Copy link
Member

KoBeWi commented Mar 23, 2021

I think everyone using this feature would like to get notified if something went wrong. I propose to at least print some error messages in the editor log stating which scenes (and which resources) couldn't get refreshed. This is not about fixing the problem, but mitigating it and making the thing usable.

We could add a general warning, but listing specific resources might be difficult to do.

From what I understand, this is not a problem limited to built-in scripts, but to built-in resources in general.

Built-in scripts seem to break more often than other resources.

@mrimvo
Copy link
Author

mrimvo commented Mar 23, 2021

We could add a general warning, but listing specific resources might be difficult to do.

A general warning would be fine for me.

Listing specific resources is not necessary in my opinion, but listing the affected scenes would be really helpful (if at all possible). Error logs would do. It's just about informing the user so he doesn't start working on a partially synced scene.

@mrimvo
Copy link
Author

mrimvo commented Mar 26, 2021

To the existing list of ideas how to mitigate this bug on Version 3.3:

  • We could add a general warning
  • listing the affected scenes [which failed reload successfully]

I would like to add another idea. Because "reload" doesn't work for most (!) of my scenes (more specifically, there's no way to check if it worked), I keep choosing "cancel" in the reload-screen and closing the affected scenes to prevent accidental re-save. What about having a "close scenes" button in the reload-window?

Auswahl_587

Also, since we keep talking in a closed issue, would you like me to open a new issue for these ideas?

@KoBeWi
Copy link
Member

KoBeWi commented Mar 26, 2021

What about having a "close scenes" button in the reload-window?

Not sure what that button would achieve. "closing to prevent re-save" is the same as reloading.

Also, since we keep talking in a closed issue, would you like me to open a new issue for these ideas?

I will just reopen it.

@KoBeWi KoBeWi reopened this Mar 26, 2021
@KoBeWi KoBeWi modified the milestones: 4.0, 3.3 Mar 26, 2021
@mrimvo
Copy link
Author

mrimvo commented Mar 26, 2021

What about having a "close scenes" button in the reload-window?

Not sure what that button would achieve. "closing to prevent re-save" is the same as reloading.

Ideally yes, it would be the same, but in reality, "reload" doesn't work reliably. Maybe it even works in many cases, but because there's no feedback about it worked or failed, one can not rely on the "reload" button to work (it doesn't for many of my scenes). For this reason I prefer to just close all changed scenes manually. The "reload" screen only helps me so far as to know which scenes changed. So to mitigate the unfixable reload button, I propose a "close scenes" button. I'm aware that's no ideal solution neither, but in my case more helpful than the actual reload button.

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

No branches or pull requests

5 participants