Storing reference of delegate in pointer

Development Programming & Scripting C++
, , , ,
1

I have multiple different classes which need to perform the same complex operation

So to keep my code dry, I’m using a manager object which requires a delegate for the complex operation

Here’s a simplified example:

DECLARE_DYNAMIC_MULTICAST_DELEGATE(FOnChange);

UCLASS()
class MYPROJECT_API UMyManager : public UObject {
    GENERATED_BODY()

   public:
    UFUNCTION()
    void Setup(UObject* NewOwner, FOnChange& NewOnChange) {  
        Owner = NewOwner;
        OnChange = &NewOnChange;  
    }

    UFUNCTION()
    void DoComplex(){ 
        if(Owner == nullptr || OnChange == nullptr){
            return;
        }

        ...
     }


   private:
    FOnChange* OnChange;
    UObject* Owner;
};

UCLASS()
class MYPROJECT_API AMyActor : public AActor {
    GENERATED_BODY()

   public:
    UPROPERTY(BlueprintAssignable)
    FOnChange OnChangeA;

    UPROPERTY()
    UMyManager* Manager;

    virtual void BeginPlay() {
        Super::BeginPlay();
        Manager->Setup(this, OnChangeA);
    }
};

UCLASS()
class MYPROJECT_API AMyCharacter : public ACharacter {
    GENERATED_BODY()

   public:
    UPROPERTY(BlueprintAssignable)
    FOnChange OnChangeB;

    UPROPERTY()
    UMyManager* Manager;

    virtual void BeginPlay() {
        Super::BeginPlay();
        Manager->Setup(this, OnChangeB);
    }
};

Will this cause any unexpected behaviour or problems? Or should I take a completely different approach which will allow me to use delegates in a similar way

Note:
Please do not suggest using Blueprint Function Library, The manager also handles other functionality and has references elsewhere. I’ve only simplified it in the example

2

Yes, this is going to be a problem. Delegates are generally created as temporaries, so storing a pointer to one would be a no-no. Generally you don’t pass in multi-cast delegates as function parameters, you’d pass in single cast delegates (possibly adding it to a multi-cast delegate). Or you have a publicly available multi-casts delegate and let objects add to them normally. Depending how you’re using this, you may also need to consider if a non-dynamic is a better fit for binding delegates if you’re only doing it from C++. There are limitations to dynamic delegates that can be a bother if you’re only doing C++ bindings anyway.

It’s hard to say what a good solution is for your problem with such a generic snippet. Off the top of my head a component could fit the bill, or an async action. But there are lots of solutions that have various pros and cons.

I would say that having something that is called manager but with an owner feels weird. Managers usually describe something coordinating work of other objects and aren’t owned by anything. If it’s more like a temporary thing like a Task that would make more sense. But for UObjects, the “owner” would be more appropriate to apply as the Outer when calling NewObject than doing an extra setup step to assign it.

3

You can create singleton class and storage all delegates in him. example:

UGameDelegateManager* UGameDelegateManager::get()
{
    if (UGameDelegateManager::singleton == nullptr)
    {
        if (BaseUtil::can_create_uobject<UGameDelegateManager>())
        {       
            singleton = NewObject<UGameDelegateManager>();
            singleton->construct();
            singleton->AddToRoot();
        }
    }
    return UGameDelegateManager::singleton;
}

template<typename Type>
inline bool BaseUtil::can_create_uobject()
{
	if (IsEngineExitRequested() == true)return false;

	if (Type::StaticClass()->IsValidLowLevel() == false)
	{
		return false;
	}

	auto transient_package = GetTransientPackage();
	if (transient_package == nullptr || transient_package->IsValidLowLevel() == false)
	{
		return false;
	}

	return true;
}

i using can_create_uobject check because some objects can try access to delegates after manager destroyed and manager recreation impossible. Also you can declare delegates without horribly macro If you don’t want export them to blueprints.

TMulticastDelegate<void(bool new_state)> set_gameplay_widget_move_state;
4

You’re right I think I’ll go with

and just shift the delegate(s) into the manager as a UPROPERTY()


Nope the manager can also be in a UObject (or anything derived of it) not just Actors


Apologies for calling it manager that’s just poor naming on my part and yesGetOuter() would be better

However the Setup()is not an extra step just to assign it, some other functionality also takes place in it (I forgot to add it in the example) since UObjects don’t have their own BeginPlay() or equivalent I have to manually invoke setup

5

Yeah, the lack of a similar hook to BeginPlay is annoying. I meant ‘extra’ only for the assignment of the “owner” when it seems like a subobject that shouldn’t require a manual assignment. Having some sort of initialization is definitely something I also find myself doing for certain types of subobjects and widgets.

1 Like