I need some external input on my networked inventory system, because I got the point, that I’m not sure if my current solution is safe or good. It works, and I do some server side validation for input but still…
For now let’s just focus on inventory and looting.
My Inventory consist of these functions and properties:
UPROPERTY()
TArray<FInventorySlot> PossesedItems;
UPROPERTY(ReplicatedUsing=OnRep_InventoryChanged, BlueprintReadWrite, Category="Inventory")
TArray<FInventorySlot> Inventory;
UPROPERTY(Replicated)
bool IsInventoryChanged;
UFUNCTION()
void OnRep_InventoryChanged();
UFUNCTION(Client, Reliable)
void ClientSetInventoryChanged();
void AddPickItemToInventory(FInventorySlot Item);
void AddItemToInventory(FInventorySlot Item);
UFUNCTION(Server, Reliable, WithValidation)
void ServerAddItemToInventory(FInventorySlot Item);
void AddItemToInventoryOnSlot(FInventorySlot Item, int32 SlotID);
UFUNCTION(Server, Reliable, WithValidation)
void ServerAddItemToInventoryOnSlot(FInventorySlot Item, int32 SlotID);
bool RemoveItemFromInventory(FName ItemID, int32 SlotID);
UFUNCTION(Server, Reliable, WithValidation)
void ServerRemoveItemFromInventory(FName ItemID, int32 SlotID);
To explain what it does.
PossesedItems array, contains Keys (IDs), of all items that are player possession. If item is manipulated inside inventory, there are always checks against this array, to make sure, that player really owns manipulated item.
OnRep is actually not used.
ClientSetInventoryChanged(), set bool IsInventoryChanged to true, when any operation on inventory is performed, to allow UI refresh list of items, from Inventory. It’s bit convoluted but long story short, some Slate widgets can’t directly operate on data from UObject, without causing Circual reference, or plain memory leak.
And besides that I still must notify widget, that it should refresh it self, not rebuild list of items every frame or something.
All functions are reliable, because I think that:
- I just can’t allow possibility of operation on inventory to be unsuccessful.
- It’s not something performed terribly often, so it shouldn’t really matter for performance.
What really concerns me, is that client store information about items. Their Keys (FNames) and Indexes (used to query items from my custom data asset).
To manipulate inventory on client I do this:
if (this->EquipmentSlot == EEquipmentSlot::Item_Inventory)
{
if (Inventory.IsValid() && Operation.IsValid() && Equipment.IsValid())
{
int32 tempSlot = Operation->LastItemSlot->InventoryItem->SlotID;
if (InventoryItem.IsValid())
tempSlot = InventoryItem->SlotID;
if (Operation->LastItemSlot->InventoryItem->EEquipmentSlot == EEquipmentSlot::Item_RightHandOne)
{
Equipment->RemoveWeapon(Operation->PickedItem->ItemID, Operation->PickedItem->SlotID, 1);
Equipment->UnEquipWeapon(Operation->PickedItem->ItemID, 1);
}
else if (Operation->LastItemSlot->InventoryItem->EEquipmentSlot == EEquipmentSlot::Item_LeftHandOne)
{
Equipment->RemoveWeapon(Operation->PickedItem->ItemID, Operation->PickedItem->SlotID, 0);
Equipment->UnEquipWeapon(Operation->PickedItem->ItemID, 0);
}
else if (Operation->LastItemSlot->InventoryItem->EEquipmentSlot == EEquipmentSlot::Item_LootWindow)
{
if (LootedObject.IsValid())
{
}
}
InventoryItem = Operation->PickedItem;
//ItemInThisSlot = Operation->ItemInThisSlot;
FInventorySlot item;
item.SlotID = Operation->PickedItem->SlotID;
item.OldSlotID = Operation->PickedItem->SlotID;
item.ItemID = Operation->PickedItem->ItemID;
/*
I probably shouldn't give client id's of items.
But somehow figure them out on server side.
*/
item.ItemIndex = Operation->PickedItem->ItemIndex;
item.ItemSlot = Operation->PickedItem->ItemSlot;
//if (Operation->PickedItem->EEquipmentSlot == 12)
//{
// item.EEquipmentSlot = EEquipmentSlot::Item_Inventory;
//}
//else
//{
item.EEquipmentSlot = Operation->LastItemSlot->EquipmentSlot;
//}
//item.EEquipmentSlot = EEquipmentSlot::Item_Inventory;
Inventory->AddItemToInventoryOnSlot(item, tempSlot);
if (Operation->LastItemSlot->SlotType != EItemSlot::Item_Inventory)
{
if (Equipment.IsValid())
{
Equipment->UnEquipItem(*InventoryItem);
}
Operation->LastItemSlot->InventoryItem.Reset();
}
Operation->LastItemSlot->InventoryItem.Reset();
return FReply::Handled();
}
}
As you can see it involves fair bit of copying around some information and passing it back to server. And even though I validate this information on server, and I’m not 100% sure it is safe.
Is there better way to approach it, or what I’m doing is more or less valid approach ?