Expose `ISpudObject` members to both C++ and BP

[error454]

Change the interface prototypes in ISpudObject to allow overriding in both BP and C++. BP can override via the normal interface override process and interface methods will just show up in the sidebar as expected.

C++ can override *_Implementation methods.

Change existing calls to the interface functions in C++ to use the prescribed entry point that covers both C++ and BP use cases.

I ran the 4 integration tests and tested this in the SPUDExamples project to make sure everything worked out as expected. It behaves as expected.

[sinbad]

Pretty sure that Cast<ISpudObject> will return null on a pure Blueprint implementation (not C++ derived). To do this it requires UKismetSystemLibrary::DoesImplementInterface instead. I was trying to avoid this because it's a much heavier call, hence the C++ only restriction originally. I haven't profiled it though so don't know the actual impact, was just being cautious.

That said if I add GameplayTag support the detection is going to get a little heavier anyway.

[error454]

Pretty sure that Cast<ISpudObject> will return null on a pure Blueprint implementation (not C++ derived).

🤦You are absolutely correct. Funny how easy it is to miss the basics sometimes. AActor provides Implements which I tend to use in other projects, it just calls through to the Class implementations.

Another caveat here is that pure blueprint implementations of the interface do not get the defaults, which is probably your motivation for keeping this C++ only to begin with. However, my game has an inheritance structure that looks like this:

BaseActor (C++)
|- Derived A (BP)
   |- Derived B (BP) <--- Save this

Where I only want to save data on Derived B and I do not want to restore transforms. Thus the motivation to allow overriding in pure blueprint implementations.

I haven't profiled it though so don't know the actual impact, was just being cautious.

I did a bit of this today, pushing a performance branch to both the examples project and spud:

• https://github.com/error454/SPUD/commit/fa1501058e412d286c584ad9c419e5774ce74943
• https://github.com/error454/SPUDExamples/commit/4794c8d4740c498d744ed00badb95089f1c7d78f

I made a test level containing 10,000 actors that implement ISpudObject and setup the level to:

• Wait 3 seconds
• Save game
• Wait 3 seconds
• Load game

I then let this run for 10 Save/Load iterations, testing the original case of Cast<ISpudObject>(Actor) vs Actor->Implements<USpudObject>(). The delta seems negligible.

Cast Average Time for 10 runs (ms)	Implements Average Time for 10 runs (ms)
392.4	392.5

With a typical call graph looking like this.

[sinbad]

Awesome, thanks for checking that performance impact!

However, I'd actually forgotten about the defaults issue, and this makes this approach unworkable because now, anyone who just implements ISpudObject against a plain Blueprint class will never get their movable transform / velocity restored because the default implementation of ShouldRestoreTransform returns false. You can see this just by running the examples - if you move the cubes in the starting area, then save and load, they return to their original level positions.

There are 2 options:

1. Invert the tests so that we rely on the "false" default.
2. Keep the method overrides C++ only, and address your issue via Gameplay Tags

I don't really like 1. because I don't know if false is guaranteed as a default, and it won't work for anything that's not true/false. So I propose going with the gameplay tags option instead for people who want to customise this behaviour in BPs.

[sinbad]

Hrm, the only problem with using Gameplay Tags as I've just realised is that the container for those tags is completely user-defined, so I'd have to add a method like GetGameplayTagContainer to ISpudObject that BPs would have to implement anyway :/ Unless there's a more seamless way I'm not aware of (I'm new to gameplay tags)

[error454]

I don't really like 1. because I don't know if false is guaranteed as a default, and it won't work for anything that's not true/false. So I propose going with the gameplay tags option instead for people who want to customise this behaviour in BPs.

With the exception of structs, UFUNCTION parameters and UPROPERTY marked variables are zero-initialized. So you'd be guaranteed that the default is always false for bool. However, for an enum, you'd have to always structure it so that the desired default value is the first in the enumeration.

Hrm, the only problem with using Gameplay Tags as I've just realised is that the container for those tags is completely user-defined, so I'd have to add a method like GetGameplayTagContainer to ISpudObject that BPs would have to implement anyway :/ Unless there's a more seamless way I'm not aware of (I'm new to gameplay tags)

Oof, things are always more complicated than it seems. The standard for those using GameplayTags is to implement IGameplayTagAssetInterface which provides the necessary method to get the tag container: virtual void GetOwnedGameplayTags(FGameplayTagContainer& TagContainer) const=0;

I was going to say to piggyback on this interface but then realized that it is marked as CannotImplementInterfaceInBlueprint which really puts us back to square-1 in terms of a solution that's accessible to C++ and blueprints.

So it seems you'd have to provide your own getter. People that already use GameplayTags could simply re-use their existing tag container, providing it to your getter as well.

Just to spitball another idea, maybe as an alternative to GameplayTags or as a persuasion into them. What about a getter that returns a reference to a customFStruct e.g. FSpudOptions? You can set struct member defaults easily for all consumers. People simply make a new FSpudOptions in their base class, can override default values in derived classes and return it to the interface. 🤷 They could even skip defining a variable and create it on the fly directly in the interface implementation.

[sinbad]

With the exception of structs, UFUNCTION parameters and UPROPERTY marked variables are zero-initialized. So you'd be guaranteed that the default is always false for bool. However, for an enum, you'd have to always structure it so that the desired default value is the first in the enumeration.

Does this guarantee apply to the return values of interface functions though? I suspect maybe it does but I think I'd like to dig into the source to confirm before relying on it. If it does, I think it's the preferred approach to simply flip these tests so the default is correct and can be overridden in BP; it seems like the least-effort solution for users who will be implementing this interface anyway, and finding an override is the most obvious place to look for behaviour changes to this class. I'd considered project plugin options instead, where you can list class names and set options for them, but I think that's less discoverable. Returning an options struct is perhaps better if you change a lot of settings at once, but more intimidating if you just want to tweak one thing.

[error454]

Does this guarantee apply to the return values of interface functions though?

Don't know for sure. UInterface's are UObjects so I would guess that it does apply, but I've never stepped through that code. I know that when it comes to blueprints, when they're saved, any interfaces you've implemented are forcibly converted to UK2Node_Event, so that would be the place to learn how serialization and overriding of defaults might happen.

[sinbad]

Short answer, I've proved that yes the return values are always initialised so I've flipped the interface functions so that's the safe return value.

Here's how it works:

The main call to one of our interface methods:

static FName NAME_USpudObject_ShouldSkipRestoreVelocity = FName(TEXT("ShouldSkipRestoreVelocity"));
bool ISpudObject::Execute_ShouldSkipRestoreVelocity(const UObject* O)
{
    check(O != NULL);
    check(O->GetClass()->ImplementsInterface(USpudObject::StaticClass()));
    SpudObject_eventShouldSkipRestoreVelocity_Parms Parms;
    UFunction* const Func = O->FindFunction(NAME_USpudObject_ShouldSkipRestoreVelocity);
    if (Func)
    {
        // --- THIS IS THE PATH WE TAKE
        const_cast<UObject*>(O)->ProcessEvent(Func, &Parms);
    }
    else if (auto I = (const ISpudObject*)(O->GetNativeInterfaceAddress(USpudObject::StaticClass())))
    {
        Parms.ReturnValue = I->ShouldSkipRestoreVelocity_Implementation();
    }
    return Parms.ReturnValue;
}

Interestingly, just implementing the interface (no method overrides) makes us go down the first path, as if the BP really does have a method defined. That's a shame, because if it went down the other path, it would call the default C++ implementation, but it doesn't. I'm not sure why, seems it would be much more useful this way.

We then call AActor::ProcessEvent which does nothing in this case except call the superclass UObject::ProcessEvent. Much of what it executes is uninteresting except this part:

                if (!bUsePersistentFrame)
        {
            Frame = (uint8*)FMemory_Alloca(Function->PropertiesSize);
            // zero the local property memory
            FMemory::Memzero(Frame + Function->ParmsSize, Function->PropertiesSize - Function->ParmsSize);
        }

        // initialize the parameter properties
                FMemory::Memcpy(Frame, Parms, Function->ParmsSize);

This section does indeed execute, zeroing out the memory used by the return structure (it zeroes Frame, then copies it into Parms which is the reference version of the return structure passed in)

So we can definitely rely on the pure Blueprint default implementation exhibiting the typical zeroed-memory behaviour for return values. I've pushed a commit to the PR which uses this to make default BPs work now.

[sinbad]

Thanks for the PR, I learned a little more about C++/BP interface combinations as well :)

[error454]

Interestingly, just implementing the interface (no method overrides) makes us go down the first path, as if the BP really does have a method defined. That's a shame, because if it went down the other path, it would call the default C++ implementation, but it doesn't. I'm not sure why, seems it would be much more useful this way.

I was reading up on the code generator end last night, looking at the same thing: https://github.com/EpicGames/UnrealEngine/blob/99b6e203a15d04fc7bbbf554c421a985c1ccb8f1/Engine/Source/Programs/UnrealHeaderTool/Private/CodeGenerator.cpp#L2778

Interfaces in UE4 have always felt inconsistent compared to other blueprint implementations. I'd expect the same behavior, or at the least, the option to right-click the implementation node and "Call Parent" like you can do with other blueprint overrides.

This section does indeed execute, zeroing out the memory

🙌, this is great to confirm!