Core: Integrate Ref instantiate where possible

[Repiteo]

Followup to #88183, which expanded Ref<T>::instantiate() to accept constructor arguments. This PR is focused on converting existing Ref constructors to the shorthand syntax where applicable. Does not convert every instance, as some constructors happen on the same line a Ref is declared; converting these would require a static method, and is more suitable as a separate PR.

[AThousandShips]

Looks good and improves readability quite substantially

[fire]

I'm curious, we seem to flip between memnew and instantiate. Which one do we pick for reviews?

[Repiteo]

Ideally, we'd want to use instantiate over memnew where possible. As mentioned in the OP, the only places where that isn't feasible is when it's the same line Ref is declared. That could be solved with a static method along the lines of:

// old
Ref<SomeType> some_value = Ref<SomeType>(memnew(SomeType()));

// new
Ref<SomeType> some_value = Ref<SomeType>::construct();

Initially I wanted to tackle that in a separate PR, but if desired I could integrate it here instead

[Repiteo]

…Alternatively, if keeping within the initial scope of the PR is desired, there's this:

// old
Ref<SomeType> some_value = Ref<SomeType>(memnew(SomeType()));

// new
Ref<SomeType> some_value;
some_value.instantiate();

I'd say this is still more readable than using memnew, even if it's arguably more verbose. There's already places in the repo utilizing this exact structure, so it wouldn't be unprecedented.

However, this wouldn't remove the need for a static constructor. To demonstrate, here's a scenario where instantiate would hurt readability:

// old
some_function(Ref<SomeType>(memnew(SomeType)));

// new (static)
some_function(Ref<SomeType>::construct());

// new (local)
Ref<SomeType> some_value;
some_value.instantiate();
some_function(some_value);

With all this in mind, I'll probably leave this concept alone for now. Maaaybe I'll sweep for cases that aren't reliant on one-liners, but I'd prefer to keep this PR as simple as possible

[adamscott]

Everything seems alright!

[adamscott]

…Alternatively, if keeping within the initial scope of the PR is desired, there's this:

// old
Ref<SomeType> some_value = Ref<SomeType>(memnew(SomeType()));

// new
Ref<SomeType> some_value;
some_value.instantiate();

I'd say this is still more readable than using memnew, even if it's arguably more verbose. There's already places in the repo utilizing this exact structure, so it wouldn't be unprecedented.

I'm all for the new behavior. I'm already coding in that way and it does what it says.

However, this wouldn't remove the need for a static constructor. To demonstrate, here's a scenario where instantiate would hurt readability:

// old
some_function(Ref<SomeType>(memnew(SomeType)));

// new (static)
some_function(Ref<SomeType>::construct());

// new (local)
Ref<SomeType> some_value;
some_value.instantiate();
some_function(some_value);

I actually don't agree. I don't think passing a new instance of Ref<> is actually clearer than being explicit.

void my_func() {
    ...
    {
        // The ideal code would scope the ref to deference it ASAP.
        Ref<SomeType> some_value;
        some_value.instantiate();
        some_function(some_value);
    }
    ...
}

Or, we could always make it so that instantiate() returns itself.

void my_func() {
    ...
    {
        Ref<SomeType> some_value;
        some_function(some_value.instantiate());
    }
    ...
}

[akien-mga]

Looks good.