I was wondering why this is DEBUG_ENABLED, when the rest of this class isn't guarded similarly.

I found that Object::connect is defined like this:

// When in debug, some non-virtual functions can be overridden for multithreaded guards.
#ifdef DEBUG_ENABLED
#define MTVIRTUAL virtual
#else
#define MTVIRTUAL
#endif // DEBUG_ENABLED

        MTVIRTUAL Error connect(const StringName &p_signal, const Callable &p_callable, uint32_t p_flags = 0);

So we're kind of abusing the system here by using connect as a virtual method. It seems it's not meant to be, and was just made virtual for debug builds to add thread guards.

Maybe it's fine to (ab)use the system this way for something that's also fairly specific to debug/editor functionality.

Last time you mentioned that overriding connect was a big no-no. I don't remember for what reason, likely for adding an editor warning onto a Node. Meanwhile here is the rest of the engine doing so nonchalantly.

I don't recall the details, but yeah committing a crime against core architecture for the sake of warnings is a not great. I assume this was also done in a more frequently used node or resource type which could have an impact on performance of connecting signals.

Here, this is done on MissingNode which is only used in case of reloading extensions in the editor, so it's not a hot path. And more importantly, the purpose is to allow converting C# scripts from the mono module to C# classes compatible with the dotnet module, so the stakes are different :)

It's still a hack, but not all hacks are created equal.

This is probably OK, given what this node is used for.

But it does make the MTVIRTUAL in object.h kind of poorly named, and the comment where it's defined only partially accurate. I don't know if it make sense to do in this PR, but it may be worth renaming MTVIRTUAL to DEBUG_VIRTUAL (or similar) and updating that comment?

Yeah I agree, let's rename MTVIRTUAL to DEBUG_VIRTUAL in this PR and update the comment @raulsntos