How can I make my basic inventory / healing system more efficient? C++

Development Programming & Scripting
, ,
1

RIght now I feel like I’ve been spending more time on here…
I am currently creating a basic inventory system.

This is my Master class Item.h

};

UCLASS()
class AItem : public AActor
{
	GENERATED_BODY()

	/** METODS **/

public:

	AItem();

	UWidgetComponent* GetWidgetComponent();
	void SetVisibilityComponentON();
	void SetVisibilityComponentOFF();
	virtual void Interact() {}
	virtual void BeginPlay() override;

	UBoxComponent* GetCollision();

protected:

	void ResetCollisionResponse();

private:

	/** PROPERTIES **/

public:

	UPROPERTY(EditDefaultsOnly, BlueprintReadWrite)
	UWidgetComponent* InteractPrompt = nullptr;

	FItemData ItemInfo;

	UPROPERTY(EditDefaultsOnly)
	int32 MAX_STORAGE;

protected:

	UPROPERTY(VisibleAnywhere)
	UBoxComponent* BoxCollision = nullptr;;

private:

};

The struct gets stored in a TArray ItemDataList;

Which has a child class Medikit.
Upon interacting with the medikit, there is this function:

void AMedikit::Interact()
{
    Super::Interact();


    UFPSGameInstance* GameInstance = Cast<UFPSGameInstance>(GetGameInstance());
    if (GameInstance != nullptr)
    {
        FItemData* ExistingItemData = nullptr;

        for (FItemData& Data : GameInstance->ItemDataList)
        {
            if (Data.ItemName == ItemInfo.ItemName)
            {
                ExistingItemData = &Data;
                break;
            }
        }
        if (ExistingItemData != nullptr)
        {
            if (ExistingItemData->StoredQuantity < MAX_STORAGE)
            {
                ExistingItemData->StoredQuantity++;
                UE_LOG(LogTemp, Warning, TEXT("After Interaction - StoredQuantity: %d"), ExistingItemData->StoredQuantity);
                Destroy();
            }
            else
            {
                UE_LOG(LogTemp, Warning, TEXT("Storage full of Meds: %d"), ExistingItemData->StoredQuantity);
            }
        }
        else
        {
            GameInstance->ItemDataList.Add(ItemInfo);

            ItemInfo.StoredQuantity++;

            Destroy();
            UE_LOG(LogTemp, Warning, TEXT("New item added to GameInstance."));
        }

    
    }
}

And then, I have my character class that has this function to heal:

void AMyCharacter::Heal()
{
    UFPSGameInstance* GameInstance = Cast<UFPSGameInstance>(GetGameInstance());
    if (GameInstance)
    {
        FItemData* ExistingItemData = nullptr;
     
        FString HealItem = "Medikit";
        //PROBLEM: what if someone wants to change the name of the Medikit??

        for (FItemData& Data : GameInstance->ItemDataList)
        {
            if (Data.ItemName == HealItem)
            {
                ExistingItemData = &Data;
                break;
            }
        }

        if (ExistingItemData != nullptr)
        {
            if (ExistingItemData->StoredQuantity > 0)
            {
                ExistingItemData->StoredQuantity--;
                Health += 10;
                UE_LOG(LogTemp, Warning, TEXT("Used a medikit. New health: %d"), Health);
                UE_LOG(LogTemp, Warning, TEXT("Used a medikit. New Quantity: %d"), ExistingItemData->StoredQuantity);
            }
            else
            {
                const int32 IndexToRemove = GameInstance->ItemDataList.Find(*ExistingItemData);

                if (IndexToRemove != INDEX_NONE)
                {
                    GameInstance->ItemDataList.RemoveAt(IndexToRemove);
                }
                UE_LOG(LogTemp, Warning, TEXT("Storage empty of Meds: %d"), ExistingItemData->StoredQuantity);
            }
        }
    }
}

It all works perfectly in UE5.
But I was wondering if there is a more efficient way to implement this.

2

Yes there are ways to improve this code.

Firstly, I haven’t seen the FItemData but from your code it looks like it was meant to be used for as some sort of inventory data (storing the name of the item and the count of items that player holds). If that’s right there is basically no need to have a property of that type for the actor representation of the item. What you can do here, is that instead of TArray you use a TMap<TSubclassOf, FItemData>
And use UClass of your actors as the map key. Then if you want to check if such item exists in ItemDataList ( or technically ItemDataMap), you use Contains() method of the map. This itself reduces your array search average complexity of O(N/2) to O(1).
Also with this method if you change your item name it won’t affect the functionality of your code.

Also I would personally encapsulate the functionality of inventory in an actor component and attach it directly to the player. Holding this kind of stuff in game instance object isn’t really clean. It works for a small project and on a single player game but in the long run it will make your game instance code messy and if you decide to go for multiplayer you will have some massive refactoring to do.

Lastly, your game instance is set in the project settings. it is also supposed to last from when your game starts until when you quit the game. There is no need to use Cast on it. Just use static_cast and you should be fine. In this particular instance the difference is going to be virtually non-existent but if you end up doing a few hundred Casts every frame it will start to impact your performance.

1 Like
3

@KiiroiSenko gave a great answer, I would add:

Instead of one Game Instance to have different functionalities, use the new Subsystems to maintain functions per “system” ( like Inventory Subsystem, XYZ Subsystem ):
https://docs.unrealengine.com/4.26/en-US/ProgrammingAndScripting/Subsystems/
But as @KiiroiSenko it last while the game is running.

If you need to cast or get an object many times, do it in BeginPlay:

MyGameInstance = Cast<UFPSGameInstance>(GetGameInstance());

If possible, change .Add() to .Emplace():
https://docs.unrealengine.com/5.3/en-US/map-containers-in-unreal-engine/#emplace

1 Like
4

Sorry, I miss-clicked while coping the code.
FItemData is a struct witha Itemquantity and a ItemName declared in the class AItem.

But yeah, now that you’re explaining it to me like this, I see what I was doing wrong. I’m gonna give it a try asap!

1 Like