Close organelle menu when leaving cell editor

[0HyperCube]

Brief Description of What This PR Does

Automatically close a modal when the parent is deleted. If the modal is not immediately deleted, then there will be an exception if the modal is closed and the modal manager tries to reparent it back to the now deleted parent.

Related Issues

Closes #4470

Progress Checklist

Note: before starting this checklist the PR should be marked as non-draft.

• PR author has checked that this PR works as intended and doesn't
  break existing features:
  https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
  (this is important as to not waste the time of Thrive team
  members reviewing this PR)
• Initial code review passed (this and further items should not be checked by the PR author)
• Functionality is confirmed working by another person (see above checklist link)
• Final code review is passed and code conforms to the
  styleguide.

Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.

[revolutionary-bot]

The lead programmer for Thrive is currently on vacation until 2025-01-07. Until then other programmers will try to make pull request reviews, but please be patient if your PR is not getting reviewed.

PRs may be merged after multiple programmers have approved the changes (especially making sure to ensure style guide conformance and gameplay testing are good). If there are no active experienced programmers who can perform merges, PRs may need to wait until the lead programmer is back to be merged.

[dligr]

When exiting the editor with the organelle menu open, I get the following exception:

Exception stack trace

E 0:00:54:0255 GodotObject.base.cs:78 @ nint Godot.GodotObject.GetPtr(Godot.GodotObject): System.ObjectDisposedException: Cannot access a disposed object. Object name: 'CellEditorComponent'. System.ObjectDisposedException /root/godot/modules/mono/glue/GodotSharp/GodotSharp/Core/GodotObject.base.cs:78 @ nint Godot.GodotObject.GetPtr(Godot.GodotObject) GodotObject.base.cs:78 @ nint Godot.GodotObject.GetPtr(Godot.GodotObject) Node.cs:776 @ void Godot.Node.AddChild(Godot.Node, bool, Godot.Node+InternalMode) NodeHelpers.cs:121 @ void NodeHelpers.ReParent(Godot.Node, Godot.Node) ModalManager.cs:169 @ void ModalManager.UpdateModals() ModalManager.cs:68 @ void ModalManager._Process(double) Node.cs:2395 @ bool Godot.Node.InvokeGodotClassMethod(Godot.NativeInterop.godot_string_name&, Godot.NativeInterop.NativeVariantPtrArgs, Godot.NativeInterop.godot_variant&) NodeWithInput_ScriptMethods.generated.cs:48 @ bool NodeWithInput.InvokeGodotClassMethod(Godot.NativeInterop.godot_string_name&, Godot.NativeInterop.NativeVariantPtrArgs, Godot.NativeInterop.godot_variant&) ModalManager_ScriptMethods.generated.cs:128 @ bool ModalManager.InvokeGodotClassMethod(Godot.NativeInterop.godot_string_name&, Godot.NativeInterop.NativeVariantPtrArgs, Godot.NativeInterop.godot_variant&) CSharpInstanceBridge.cs:24 @ Godot.NativeInterop.godot_bool Godot.Bridge.CSharpInstanceBridge.Call(nint, Godot.NativeInterop.godot_string_name*, Godot.NativeInterop.godot_variant**, int, Godot.NativeInterop.godot_variant_call_error*, Godot.NativeInterop.godot_variant*)

It seems like at the moment of ModalManager re-parenting the organelle menu, the latter's parent (cell editor) is already disposed

[0HyperCube]

Thanks for the feedback @dligr. I have updated the code so that all modals are deleted as soon as their parent is deleted. This will stop the exception from occurring again.

[dligr]

The organelle context menu now correctly closes on editor exit, but an exception is now thrown when exiting with an organelle upgrade popup open.

Stack trace

E 0:01:45:0885 GodotObject.base.cs:78 @ nint Godot.GodotObject.GetPtr(Godot.GodotObject): System.ObjectDisposedException: Cannot access a disposed object. Object name: 'CustomConfirmationDialog'. System.ObjectDisposedException /root/godot/modules/mono/glue/GodotSharp/GodotSharp/Core/GodotObject.base.cs:78 @ nint Godot.GodotObject.GetPtr(Godot.GodotObject) GodotObject.base.cs:78 @ nint Godot.GodotObject.GetPtr(Godot.GodotObject) Node.cs:752 @ Godot.StringName Godot.Node.GetName() Node.cs:374 @ Godot.StringName Godot.Node.get_Name() ModalManager.cs:168 @ void ModalManager.UpdateModals() ModalManager.cs:68 @ void ModalManager._Process(double) Node.cs:2395 @ bool Godot.Node.InvokeGodotClassMethod(Godot.NativeInterop.godot_string_name&, Godot.NativeInterop.NativeVariantPtrArgs, Godot.NativeInterop.godot_variant&) NodeWithInput_ScriptMethods.generated.cs:48 @ bool NodeWithInput.InvokeGodotClassMethod(Godot.NativeInterop.godot_string_name&, Godot.NativeInterop.NativeVariantPtrArgs, Godot.NativeInterop.godot_variant&) ModalManager_ScriptMethods.generated.cs:138 @ bool ModalManager.InvokeGodotClassMethod(Godot.NativeInterop.godot_string_name&, Godot.NativeInterop.NativeVariantPtrArgs, Godot.NativeInterop.godot_variant&) CSharpInstanceBridge.cs:24 @ Godot.NativeInterop.godot_bool Godot.Bridge.CSharpInstanceBridge.Call(nint, Godot.NativeInterop.godot_string_name*, Godot.NativeInterop.godot_variant**, int, Godot.NativeInterop.godot_variant_call_error*, Godot.NativeInterop.godot_variant*)

[0HyperCube]

Thanks for spotting @dligr. This occurred because the popup was in the demoted popup queue before it was destroyed. I have now fixed this by removing all destroyed popups from this queue.

[dligr]

After looking through the code, it seems like modals are added to demotedModals when they are closed. So, would it make sense not to close the popups in the first place (in the OnParentLost method), so that they don't have to be removed from the stack?

[0HyperCube]

@dligr I choose to call the close method since some popups have custom behaviour that is run when they are closed (e.g. the pause menu will unpause the game). From my testing, it seems that no menu will actually be effected though since the pause menu can't loose its parent.

[dligr]

I think that it really is better to retain the popups' custom OnClose() functionality.
But maybe it's possible to get around needing to remove the popup from the demotedModals by removing the modal from the modalStack before closing it? OnModalLost() doesn't handle modals that aren't present in that stack, so the modal wouldn't be added to demotedModals in the first place.

[hhyyrylainen]

I think this should guard against duplicate signals as MakeModal I think can be called multiple times (so the signal registration also happens multiple times).

[hhyyrylainen]

I think it would be worth it to track what parents are connected in this way? So that they can be unconnected on normal close (and not require leaving the tree). Or at least made so that no duplicate connections are made by this method.