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.

Sign up for GitHub

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

Jump to bottom

Exiting the editor while organelle popup is open is possible and crashes the game #4470

Closed
hhyyrylainen opened this issue Sep 14, 2023 · 4 comments · Fixed by #5775
Closed

Exiting the editor while organelle popup is open is possible and crashes the game #4470

hhyyrylainen opened this issue Sep 14, 2023 · 4 comments · Fixed by #5775
Labels
bug C# easy programming stage:microbe
Milestone
Release 0.8.1

Comments

@hhyyrylainen
Copy link
Member

Using the keyboard to select and click (spacebar) the confirm button while an organelle popup menu allows you to exit the editor and then crash the game when you click something on the popup.

2023-09-14_21 39 05 1666

The crashing exception:

ERROR: System.ObjectDisposedException: Cannot access a disposed object.
ERROR: Object name: 'CellEditorComponent'.
ERROR:   at Godot.Object.GetPtr (Godot.Object instance) [0x0001c] in <1598dd9b7b214c39a3e18009cd4aff45>:0 
ERROR:   at Godot.Node.AddChild (Godot.Node node, System.Boolean legibleUniqueName) [0x00000] in <1598dd9b7b214c39a3e18009cd4aff45>:0 
ERROR:   at NodeHelpers.ReParent (Godot.Node node, Godot.Node newParent) [0x00028] in <1608dc207f454f80a7587eceebbf12ac>:0 
ERROR:   at ModalManager.UpdateModals () [0x00027] in <1608dc207f454f80a7587eceebbf12ac>:0 
ERROR:   at ModalManager._Process (System.Single delta) [0x00008] in <1608dc207f454f80a7587eceebbf12ac>:0

So this is a pretty clear case where the editor should force close the popup before it allows exiting. Multicellular editor and late multicellular editor probably need the same fix so maybe a base class should be made to have a method that ensures all open popups are closed before exiting.
@buckly90
Copy link
Member

No luck reproducing the crash with this change. Very good!

@hhyyrylainen
Copy link
Member Author

(I think you commented on the wrong thing, you probably meant to comment on #4469 I'll merge that now wit the fix confirmation)

@CoreyHendrey CoreyHendrey mentioned this issue Sep 21, 2023
Closed
4 tasks
@hhyyrylainen hhyyrylainen added this to the Release 0.6.4 milestone Sep 21, 2023
@hhyyrylainen hhyyrylainen moved this to started but stuck (help wanted) in Thrive Planning Aug 7, 2024
@hhyyrylainen
Copy link
Member Author

With my change to disallowing the popup no longer to act on removed things, this might be better now, though still might run into the problem of accessing a disposed object to check if the targets are still valid for the popup.

@0HyperCube 0HyperCube mentioned this issue Dec 28, 2024
Merged
4 tasks
@hhyyrylainen
Copy link
Member Author

A fix has been made in general that basically fixes this (even if it isn't totally perfect) so this issue is closed by that general change. I think the likelihood of getting a more specific fix like the editor ensuring popups are closed before it begins the fade out is pretty low so I don't want to leave this issue open and hanging.

@github-project-automation github-project-automation bot moved this from started but stuck (help wanted) to Done in Thrive Planning Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug C# easy programming stage:microbe
Projects
Thrive Planning
Status: Done
Milestone
Release 0.8.1
2 participants
@hhyyrylainen @buckly90