Unlink BudgetMenu instances before one is invalidated

Author: Hop311

extension/doc_classes/MenuSingleton.xml

@@ -255,6 +255,11 @@
 			<description>
 			</description>
 		</method>
+		<method name="unlink_budget_menu_from_cpp">
+			<return type="void" />
+			<description>
+			</description>
+		</method>
 		<method name="update_search_results">
 			<return type="void" />
 			<param index="0" name="text" type="String" />

extension/src/openvic-extension/singletons/MenuSingleton.cpp

@@ -446,6 +446,7 @@ void MenuSingleton::_bind_methods() {
 
 	/* BUDGET MENU */
 	OV_BIND_METHOD(MenuSingleton::link_budget_menu_to_cpp, { "godot_budget_menu" });
+	OV_BIND_METHOD(MenuSingleton::unlink_budget_menu_from_cpp);
 
 	/* Find/Search Panel */
 	OV_BIND_METHOD(MenuSingleton::generate_search_cache);
@@ -1539,9 +1540,15 @@ String MenuSingleton::get_longform_date() const {
 void MenuSingleton::link_budget_menu_to_cpp(GUINode const* const godot_budget_menu) {
 	ERR_FAIL_NULL(godot_budget_menu);
 	GameSingleton& game_singleton = *GameSingleton::get_singleton();
-	BudgetMenu* old_instance = budget_menu.get();
-	if (old_instance != nullptr) {
-		game_singleton.gamestate_updated.disconnect(&BudgetMenu::update, old_instance);
+
+	if (budget_menu) {
+		UtilityFunctions::push_error(
+			"Trying to link new C++ and GDScript BudgetMenu instances without unlinking the old instances first! "
+			"The unlinking must happen just before the GDScript BudgetMenu is freed, "
+			"otherwise the C++ BudgetMenu will continue running despite all its UI node pointers now being invalid."
+		);
+
+		unlink_budget_menu_from_cpp();
 	}
 
 	auto const& strata_keys = game_singleton.get_definition_manager().get_pop_manager().get_stratas();
@@ -1556,6 +1563,15 @@ void MenuSingleton::link_budget_menu_to_cpp(GUINode const* const godot_budget_me
 	game_singleton.gamestate_updated.connect(&BudgetMenu::update, budget_menu.get());
 }
 
+void MenuSingleton::unlink_budget_menu_from_cpp() {
+	if (budget_menu) {
+		GameSingleton::get_singleton()->gamestate_updated.disconnect(&BudgetMenu::update, budget_menu.get());
+		budget_menu.reset();
+	} else {
+		UtilityFunctions::push_warning("unlink_budget_menu_from_cpp called but no C++ BudgetMenu instance was linked!");
+	}
+}
+
 /* Find/Search Panel */
 
 Error MenuSingleton::generate_search_cache() {

extension/src/openvic-extension/singletons/MenuSingleton.hpp

@@ -287,6 +287,7 @@ namespace OpenVic {
 
 		/* BUDGET MENU */
 		void link_budget_menu_to_cpp(GUINode const* const godot_budget_menu);
+		void unlink_budget_menu_from_cpp();
 
 		/* Find/Search Panel */
 		// TODO - update on country government type change and state creation/destruction

game/src/UI/Session/BudgetMenu.gd

@@ -78,6 +78,11 @@ func _notification(what : int) -> void:
 	match what:
 		NOTIFICATION_TRANSLATION_CHANGED:
 			_update_info()
+		NOTIFICATION_PREDELETE:
+			# If the C++ BudgetMenu isn't freed before the destruction of this GUINode, then its update method, triggered by
+			# gamestate updates, could be called after all the child UI nodes they refer to have been freed,
+			# meaning the C++ BudgetMenu would be dereferencing invalid pointers.
+			MenuSingleton.unlink_budget_menu_from_cpp()
 
 func _on_update_active_nation_management_screen(active_screen : NationManagement.Screen) -> void:
 	_active = active_screen == SCREEN