Right now, I have all the items in game stored in RepStorage. Then I have an inventory and when the player equips what they’ve got in their inventory
if CurrentSlot.Holder.Item.Value then -- the value is an ObjectValue and the object is the item that's in RepStorage
local ItemInBackpack = CurrentSlot.Holder.Item.Value
if ItemInBackpack then
local CloneHolder = Instance.new('ObjectValue')
CloneHolder.Name = 'CloneHolder'
local ClonedItem = ItemInBackpack:Clone()
CloneHolder.Value = ClonedItem
ClonedItem.Parent = Character
CloneHolder.Parent = ItemInBackpack
end
end
I take a clone from repstorage, place that into the player and that clone acts as the item in game. When the player unequips the item, it gets removed from the player.
My only worry with this is while it works, I’m worried about security. I know certain people can access RepStorage, so I’m worried they’d be able to just equip any item in game without actually needing to have it in their inventory, so I was wondering what I could do to prevent that.
I know I could do a server side check like
if CurrentSlot.Holder.Item.Value then
local ItemInBackpack = CurrentSlot.Holder.Item.Value
if ItemInBackpack then
if CheckInventory:InvokeServer(ItemInBackpack ) then -- check if player has said item in their inventory from the server
local CloneHolder = Instance.new('ObjectValue')
CloneHolder.Name = 'CloneHolder'
local ClonedItem = ItemInBackpack:Clone()
CloneHolder.Value = ClonedItem
ClonedItem.Parent = Character
CloneHolder.Parent = ItemInBackpack
end
end
end
But I don’t that would really stop much?? I’m not really sure how much an exploiter can access, but couldn’t they still just manually go into repstorage and grab the item??
The clone would be a local object but you’ll have to place sanity checks on your remotes so that a local object can’t be used to fire remotes concerned with activation of tools. Optionally you could hook a ChildAdded connection to the backpack and the character, send this object to the server each time and get the server to check if it is a local object (in which case it receives nil) and take suitable action (delete local object or moderate)
Alternatively, you could keep all tools in ServerStorage and have the client request for a tool when it’s needed with InvokeServer, and then get the server to clone and return the tool after doing sanity checks.
Not related but, why are you cloning a tool locally?
I swear you couldn’t return items cloned from the server back to the client. You have to some fidgety stuff like parent it somewhere then get the client to check for it being added and thus. I want the easiest solution.
Which one of these would be safest tho? I just don’t want players being able to access items they shouldn’t be accessing
I don’t know whether that’s true but either way, you don’t have the client parent tools. Get the server do the logic currently on your client and handle tools, cloning and parenting and only then return the new objects.
If you really don’t want players to access your tools freely, you simply keep tools under someplace they can’t access – here, ServerStorage is most suitable.
I mean it doesn’t really matter where you store them because in the end exploiters can copy tools from other users and if they’re coded in a way to act fine even if only cloned locally (client-side), they’ll still work and break the game.
All objects in ReplicatedStorage will always be vulnerable to client access. The best you can do to prevent misuse is to implement client checks similar to the one posted before. Keep in mind that these checks too can be overcome by exploiters, albeit with more knowledge.
if CurrentSlot.Holder.Item.Value then
local ItemInBackpack = CurrentSlot.Holder.Item.Value
if ItemInBackpack then
if CheckInventory:InvokeServer(ItemInBackpack ) then -- check if player has said item in their inventory from the server
local CloneHolder = Instance.new('ObjectValue')
CloneHolder.Name = 'CloneHolder'
local ClonedItem = ItemInBackpack:Clone()
CloneHolder.Value = ClonedItem
ClonedItem.Parent = Character
CloneHolder.Parent = ItemInBackpack
end
end
end
You’re forgetting that the client can independently – without fooling around with your script – just clone tools from ReplicatedStorage. Your CheckInventory:InvokeServer(ItemInBackpack) check assumes that the client sets CurrentSlot.Holder.Item.Value to the cloned tool, which isn’t necessary. Parenting the cloned tool directly to the character will allow the client to use the tool at any point, without going through your checks.
-- Server
-- Fired on the InvokeServer from client
local function ReturnItem(player, item)
local User = PlayerData[player.UserId]
if not User then return end
for _, v in pairs(User.Inventory) do
for _, playerItem in pairs(v) do
print(playerItem.Name, item.Name) -- prints Wooden Axe Wooden Axe
if playerItem.Name == item.Name then
print('Yes') -- Does print
local CloneItem = item:Clone()
return CloneItem
end
end
end
end
-- Client
local Item = GetItem:InvokeServer(CurrentSlot.Holder.Item.Value)
print(Item) -- prints nil
if Item then
print(1)
Item.Parent = Character
end
So the server can’t return an object cloned from the server back to the client
Again, you don’t have to give the raw clone to the client. The client will end up parenting the tool somewhere, instead of having the client do this, you can have the server parent the tool to it’s suitable parent and hence also allow the tool to be replicated to the client, allowing you to pass the object through a remote.
local function RetrieveItem(player, equip, item)
if item == nil then return end
local User = PlayerData[player.UserId]
if not User then return end
local Character = player.Character or player.CharacterAdded:Wait()
if equip then
for _, v in pairs(User.Inventory) do
for _, playerItem in pairs(v) do
if playerItem.Name == item.Name then
local CloneItem = item:Clone()
CloneItem.Parent = Character
end
end
end
else
local Item = Character:FindFirstChild(item.Name)
if Item then
Item:Destroy()
end
end
end
and seems to work well enough not entirely sure how much of it an exploiter could bypass, but I’ll just have to go with it
When it comes to equipping tools, (imo) it’s best to have this done server side. Store the tools in ServerStorage, a service untouchable by any connected client regardless of exploit. If a tool needs to be equipped, the server can simply clone the tool into the Players Backpack. The server should know the contents of the players inventory, always. A recommended system to put in place is not have the client worry about anything except for invoking the server to buy, equip, remove etc the item. All cloning or removal should be done on the server as it can be easily be sanity checked.
EDIT => Seems like what I’ve said has already been said. I should read more carefully next time
How can I get the object to client then? because my inventory script works like so
local function Setup()
for i, v in pairs(PlayersInventory.Main) do
if v.Type then
local Slot = MainSlots:FindFirstChild(i)
if Slot then
local InventoryDefaultData = InventoryData[v.Type][v.Name]
Slot.Holder.Item.Value = Items[v.Type][v.Name]
Slot.Holder.Icon.Image = InventoryDefaultData.Image
if v.Quantity then
Slot.Holder.Quantity.Text = v.Quantity
end
if v.Durability then
UpdateDurabilityBar(true, Slot.Holder, v.Durability, InventoryDefaultData)
else
UpdateDurabilityBar(false, Slot.Holder)
end
end
end
end
end
and PlayersInventory is set via
local PlayersInventory = InventoryCheck:InvokeServer()
So I’m going through their inventory based on what the servers returned, so they can’t change that. But each slot has an ObjectValue inside it (Slot.Holder.Item) is the ObjectValue and so then I go through the repstored items and set the object to be that. But if they were stored server side I wouldnt be able to do that
Nevermind, I think I found a way around it, by using StringValues instead of ObjectValues.
I can just set the string to be the name of the object, and then use the server to check the string with their inventory and make sure they match, then get the item.
So if the items are stored in ServerStorage, no exploiter can access them, correct?
not entirely sure how much of it an exploiter could bypass, but I’ll just have to go with it