Your theory is exactly right - your for loop only runs for the items that are already in items at the start of the game.
By the way, though it's perfectly valid to have a for loop like that, the more common form is
for i, v in ipairs(items:GetChildren()) (for lists; as in, it has only integer keys starting at 1 and going up)
for k, v in pairs(items:GetChildren()) (if it's a dictionary; as in, it has non-integer keys or they have holes; learn more here )
You have a couple options:
- The code that is cloning the object into the workspace should be server-side. You could wrap the code in your top script lines 6-11 in a function and put that function in a module script. The cloning script would then call this function with the newly cloned item.
- You could listen to the event
items:ChildAdded(child), see the documentation. You'll also need to wrap the lines 6-11 in a function for this as well, so that you can refer to the code twice with ease.
Here's the second option:
local ResourceManager = require(game.ReplicatedStorage.ResourceManager)
local event = game.ReplicatedStorage:WaitForChild('ItemClick')
local items = workspace.Items
function ConnectClickDetector(v)
v.ClickDetector.MouseClick:connect(function(player)
print('Click Detected')
local objectType = v.ObjectType.Value
local data = ResourceManager.Resources[objectType]
event:FireClient(player,data)
end)
end
for i,v in next, items:GetChildren() do
ConnectClickDetector(v)
end
items.ChildAdded:connect(ConnectClickDetector)
Note that in this case there is no need for an anonymous function when connecting the ChildAdded event since the event sends the new child as its argument, and that's exactly what we want ConnectClickDetector to receive. As you can see, an anonymous function would do nothing more than pass this value on:
items.ChildAdded:connect(function(child)
ConnectClickDetector(child)
end)
and so is unneeded.
As I mentioned earlier, the cloning should occur server-side. (If you wanted to be exploit proof, you would also have a copy of each player's inventory server side and make sure that they can drop whatever it is they're claiming they want to drop. You would need to keep the server-side and client-side inventories in sync; ex, if the client says "I want to pick up this tool", then it should use a RemoteFunction to ask the server to try and pick it up. If the server says "Yes", then both the server and client should record that the player has that tool. The server might need to say "That object doesn't exist" -- ex, perhaps another player picked it up just an instant before this player did. In that event, neither the server nor the client should record an increase in inventory.)
So, in your Drop code, you should call a RemoteEvent that tells the server that this player wants to drop an object of the specified type. The server should make sure that they have said inventory. If they don't, then don't drop anything. (In either case it's safe for the client to delete the object from inventory, since whether they had it or not before, they don't anymore.) If they do, then drop the item into the workspace.
Note: local obj = game.Workspace.GAME_PROPERTIES.ItemSelected.Value will be different for each client, if you have FilteringEnabled on. If you attempt to use this value from the server, it will always be nil (unless the server assigns a value to it). Ideally this ItemSelected object should be in the PlayerGui, or - even better - just a variable in a script somewhere (it can be a ModuleScript if multiple scripts need to know about it).
ex:
local maxInventorySlots = 10
local InventoryManagement = {}
local local inventory = {}
local function NewInventorySlot()
local slot = {}
slot.Type = "None"
slot.Number = 0
return slot
end
for i = 1, maxInventorySlots do
inventory[i] = NewInventorySlot()
end
InventoryManagement.Inventory = inventory
return InventoryManagement
This ModuleScript would be part of the gui code and located in StarterGui/PlayerGui, PlayerScripts, or ReplicatedStorage (however you want to organize it). If you can use this same code for both server and client, keep it in ReplicatedStorage.
Since the server wouldn't use the "InventoryManagement.SelectedItem", this is a sign that "SelectedItem" variable doesn't belong in this script. It should probably belong to the code that deals with the player clicking on inventory items.
1 comment
Excellent response and explanation. Thank you once again for your valuable feedback!