Fix internal CONNECT_INHERITED being saved in PackedScene & Make Local

[Mickeon]

Fixes #75801.

The title is pretty self-explanatory.
Tested with the attached project. The issue fixes itself after resaving the scene.

It would be nice to receive feedback from KoBeWi because judging by the cause, it seems like I have a tendency to overlooking things.

[Mickeon]

As a bit of a side note, there seems to be a regression from #81221 where the PopupMenu isn't updated properly, causing some options to be entirely disabled because of that early return.

godot/editor/connections_dialog.cpp

Lines 1193 to 1203 in 5f1e56f

	void ConnectionsDock::_slot_menu_about_to_popup() {
	TreeItem *item = tree->get_selected();
	if (!item || _get_item_type(*item) != TREE_ITEM_TYPE_CONNECTION) {
	return;
	}
	bool connection_is_inherited = item->has_meta("_inherited_connection");
	slot_menu->set_item_disabled(slot_menu->get_item_index(SLOT_MENU_EDIT), connection_is_inherited);
	slot_menu->set_item_disabled(slot_menu->get_item_index(SLOT_MENU_DISCONNECT), connection_is_inherited);
	}

Worth keeping that in mind when testing.

[KoBeWi]

As a bit of a side note, there seems to be a regression

I think it should be fixed first, because the popup is broken right now.
As for the PR, it only fixes saving the flag, but you can make new instance and make it local and run into the same issue (until you reload the scene).

[Mickeon]

Unsure how to handle this, then. "Sanitizing" the input when the scene is made local may make sense, but having it work seamlessly with UndoRedo looks to be overly verbose. Sanitizing it in set_scene_file_path() also seems to make sense to me, but it's a whole new step to an otherwise simple method. Perhaps the usage of a flag included among the rest has been a mistake all along, if it needs to be discarded half the time...

[Mickeon]

I opened a PR about the aforementioned issue. #81750. Let's see. One thing at the time.

[Mickeon]

This PR has been updated with an (admittedly) shoddy solution to prevent Local Scene from ever keeping CONNECT_INHERITED. The problem now is that I'm unsure how to go about using UndoRedo here, as dealing with the Node dock like this feels like unexplored territory.

[Mickeon]

I apologise. This PR has been closed on accident because GitHub's links are silly like that.

[KoBeWi]

I wonder if there is better way to refresh it.

[Mickeon]

I wondered too, but it didn't seem like it because all of the "refreshing" logic is inside the set_node method. It would require moving it out...

Maybe NodeDock::restore_last_valid_node would work just the same? In hindsight, I don't see why not.

[KoBeWi]

It's probably fine for now, but if we ever decide to add early return to set_node() it will stop working.
Though my biggest concern would be that this method is not used outside editor node, so either the dock is not refreshed by anything else or there is a better way.

[Mickeon]

I was about to go with the change, but I'll keep set_node() then. The lack of an early return is unexpected though, yeah.

The NodeDock has a pretty simple bunch of methods. None of them are like restore_last_valid_node() or set_node(), so they seem like the closest thing to a refresh?

[KoBeWi]

I think your new method should be recursive.

[Mickeon]

Pushed to use make the connection stripping recursive.

[KoBeWi]

There are still some problems (like undo not restoring the flag), but at least it fixes the issue, which is more important.

[akien-mga]

Thanks!