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 saving wrong edited scene state when switching scene tabs #83251

Merged

Conversation

Rindbee
Copy link
Contributor

@Rindbee Rindbee commented Oct 13, 2023

When creating a new scene, the scene tab will actually switch to the newly created one. Also worth noting that switching scene tabs saves state (by save_edited_scene_state).

When trying New Scene again, the previously empty scene tab will be removed.

So the correct logic is: switch the tab save state first, and then remove the previous empty scene tab. To prevent current_edited_scene from being incorrect when saving state.

Fix #83213.

@Rindbee Rindbee force-pushed the fix-current_edited_scene-wrong-move branch from bb0122f to f652dfe Compare October 13, 2023 09:08
@akien-mga akien-mga added this to the 4.2 milestone Oct 13, 2023
@akien-mga akien-mga requested a review from a team October 13, 2023 09:14
@KoBeWi
Copy link
Member

KoBeWi commented Oct 16, 2023

It doesn't seem to fix the linked issue, the scene tree is still getting empty.

@Rindbee
Copy link
Contributor Author

Rindbee commented Oct 16, 2023

It doesn't seem to fix the linked issue, the scene tree is still getting empty.

Is it the same way to reproduce?

0.mp4

@KoBeWi
Copy link
Member

KoBeWi commented Oct 16, 2023

I'm getting different results in the first case 🤔

godot.windows.editor.dev.x86_64_rNIjV7CZGH.mp4

EDIT:
Ok, it's still broken when you have a single open scene. It works if there is more.

@Rindbee
Copy link
Contributor Author

Rindbee commented Oct 16, 2023

Ok, it's still broken when you have a single open scene. It works if there is more.

I still can't reproduce it, maybe due to some non-default editor settings? (I deleted the config, cache and data directories)

1.mp4

@KoBeWi
Copy link
Member

KoBeWi commented Oct 16, 2023

I tried in a fresh project in self-contained mode and still get the bug.
EDIT:
No I tested wrongly. I don't get it in a new project. Not sure what causes it my old test project.
EDIT2:
It's not even project-specific ;_; I get it in a one scene, but only initially. After closing and reopening it, the issue doesn't appear again.

@KoBeWi
Copy link
Member

KoBeWi commented Oct 16, 2023

Ok I think I got the steps more or less:

  • open a project (can be a new project)
  • make sure there is only one opened scene and there is no selected node
    • if any of the above is not true, close extra scenes, unselect everything, save and restart
  • the bug should now occur when you press Ctrl+N twice

@Rindbee Rindbee force-pushed the fix-current_edited_scene-wrong-move branch from f652dfe to 80d60cd Compare October 17, 2023 01:01
@Rindbee
Copy link
Contributor Author

Rindbee commented Oct 17, 2023

It seems that when _load_open_scenes_from_config(), the newly added scenes are not tracked in editor_history.

@YuriSizov
Copy link
Contributor

YuriSizov commented Oct 17, 2023

Seems like a regression from #80517 then? So my main concern is whether this introduces some excessive calls again. editor_history.add_object doesn't look particularly excessive, and I guess it makes sense that we need to "initialize" the history.

My question would be whether we need to do it to every scene, or just the last one that becomes active.

@Rindbee
Copy link
Contributor Author

Rindbee commented Oct 17, 2023

My question would be whether we need to do it to every scene, or just the last one that becomes active.

We need to do it to every scene, #83251 (comment) can be reproduced in all open scenes (as long as no nodes are selected).

@YuriSizov
Copy link
Contributor

I see. So something else expects every open scene to be in the history, even if the user never navigated to them. I think it's worth adding a comment to the else clause to explain that.

@Rindbee Rindbee force-pushed the fix-current_edited_scene-wrong-move branch 2 times, most recently from c7c07f8 to 685d107 Compare October 17, 2023 14:51
When creating a new scene, the scene tab will actually switch to the newly created one.
Also worth noting that switching scene tabs saves state (by `save_edited_scene_state`).

When trying New Scene again, the previously empty scene tab will be removed.

So the correct logic is: switch the tab save state first, and then remove the previous
empty scene tab. To prevent `current_edited_scene` from being incorrect when saving state.
@Rindbee Rindbee force-pushed the fix-current_edited_scene-wrong-move branch from 685d107 to 225a5e2 Compare October 17, 2023 14:53
@akien-mga akien-mga merged commit e06d092 into godotengine:master Oct 17, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@Rindbee Rindbee deleted the fix-current_edited_scene-wrong-move branch October 17, 2023 22:35
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.

'Scene' tab of existing scenes can become stuck empty when creating new scenes
4 participants