I am working on some kind of tool inventory system and have made this. It technically works, but I am not sure if this is a good idea.
(Not the full code)
Players.PlayerAdded:Connect(function(Player)
local CurrentTools = {
};
local function GetTools(InvokedPlayer)
if InvokedPlayer == Player then
return CurrentTools
end
end
ToolsFunctions.GetTools.OnInvoke = GetTools
end)
function Tools.GetTools(Player)
return ToolsFunctions.GetTools:Invoke(Player)
end
Is this a good idea, since it will fire for every player each time? Are there any easy alternatives, I am not very experienced with this sort of thing.
I think your code will break with more than 1 player, because BindableFunctions uses a callback function (set to the .OnInvoke property), and callback functions only accept 1 function. So the second player to join will overwrite the first callback function for his own
The way you should do this is with module scripts. With a module script, you can script your backpack module, and expose methods that outside scripts can call (to get the tools, and much more if needed)
You current solution is definitely performant enough and safe, but I dislike the use of Bindables for cross script communication. Module scripts can basically always do the same, in a more readable and organized manner
My main problem is that the data is stored on the player, so how would I access it with a module script? Should I change the system to store the data for every player in 1 table instead of a table for each player?
You can have 1 table that stores every player’s tables. Otherwise you can use an OOP approach, where you have a constructor function that returns a table containing the player’s data, and methods to do stuff with that data