Hi, I have a question that I can’t answer so I need help answering it
Question:
Is it better if I script everything into a module then run the code, in the module, when I need it?
for example:
I script a buy function into a module then when a player clicks a button it runs the buy function from the module
Module:
function module:BuyItem() print("Player Bought Item") end
LocalScript:
script.Parent.Activated:Connect(function() module:BuyItem() end)
It would depend on what your measure of “better” is. If the buy function is only ever used in that one place, it wouldn’t really affect much performance wise, but would probably still be cleaner (arguably).
But I personally like this format. Making functions in a module and keeping them as single purpose things can make your code clearer and that’s worth it, imo.
Other thing I’d suggest is to have different modules when you have different concepts. One module could handle everything item related, for example; then you’d put the rest (anything that doesn’t handle items) into other modules. So you clean it that way as well
Small nitpick if you go with your example idea, don’t use :.
Since your function doesn’t rely on self or any passed state, you should be using function module.BuyItem(), that way you can then format your 2nd piece of code simply as:
script.Parent.Activated:Connect(module.BuyItem)
can you please explain to me what self is?
When you type:
function module:method()
end
What you’re actually saying is:
function module.method(self)
end
Similarly, a:b() translates to a.b(a). This is a feature for Lua object oriented code, which you should omit when you’re not making use of it. self is the built in name for the automatic parameter : inserts into the function.
Here’s a post that goes more into detail on this, but I just suggest avoiding it altogether until you find yourself in a situation that calls for it, since it isn’t directly related to this OP. All about Object Oriented Programming
ok, thanks for the definition of self but I got a problem with something. So I am trying to make it that when you buy something it refreshs the shop. In the shop script there is a i,v in pairs loop to delete all the previous frames from the shop but instead of deleting it, it multiples. I don’t know the problem to it. Can you please help me?
function module:LoadGUIStore(player) local backpack = player.Backpack local character = player.Character local frames = script.Parent.Parent.Frames local shopFrame = frames.Shop local templateFolder = shopFrame.Templates local toolSelection = shopFrame.ToolSelection local ToolTemp = templateFolder["ToolTemplate"] local extraFolder = ToolTemp["Extra"] local ownedText = extraFolder["OwnedText"] for i, v in pairs(toolSelection:GetChildren())do if v:IsA("ImageButton") or v:IsA("TextButton")then v:Destroy() end for toolName, tool in pairs(itemInfo.Tools)do if Tools:FindFirstChild(toolName) then local Temp = ToolTemp:Clone() local ToolNameTextLabel = Temp["ToolName"] local ToolCostTextLabel = Temp["ToolCost"] Temp.Name = toolName ToolNameTextLabel.Text = toolName ToolCostTextLabel.Text = tool["Cost"][1].." "..tool["Cost"][2] Temp.Parent = toolSelection Temp.LayoutOrder = tool["Cost"][1] if backpack:FindFirstChild(toolName) and character:FindFirstChild(toolName)then ownedText.Visible = true else ownedText.Visible = false end Temp.Visible = true end end end local deb = {} for i, button in pairs(toolSelection:GetChildren())do if button:IsA("ImageButton") or button:IsA("TextButton")then button.Activated:Connect(function() if deb[player.Name] == false or deb[player.Name] == nil then deb[player.Name] = true events["BuyTool"]:FireServer(button.Name) wait(1) deb[player.Name] = false end end) end end end
this is the working code:
function module:LoadGUIStore()
local backpack = self.Backpack
local character = self.Character or self.CharacterAdded:Wait()
local frames = script.Parent.Parent.Frames
local shopFrame = frames.Shop
local templateFolder = shopFrame.Templates
local toolSelection = shopFrame.ToolSelection
local ToolTemp = templateFolder["ToolTemplate"]
local extraFolder = ToolTemp["Extra"]
local ownedText = extraFolder["OwnedText"]
for i, v in pairs(toolSelection:GetChildren())do
if v:IsA("ImageButton") or v:IsA("TextButton")then
v:Destroy()
end
for toolName, tool in pairs(itemInfo.Tools)do
if Tools:FindFirstChild(toolName) then
local Temp = ToolTemp:Clone()
local ToolNameTextLabel = Temp["ToolName"]
local ToolCostTextLabel = Temp["ToolCost"]
Temp.Name = toolName
ToolNameTextLabel.Text = toolName
ToolCostTextLabel.Text = tool["Cost"][1].." "..tool["Cost"][2]
Temp.Parent = toolSelection
Temp.LayoutOrder = tool["Cost"][1]
if backpack:FindFirstChild(toolName) and character:FindFirstChild(toolName)then
ownedText.Visible = true
else
ownedText.Visible = false
end
Temp.Visible = true
end
end
end
local deb = {}
for i, button in pairs(toolSelection:GetChildren())do
if button:IsA("ImageButton") or button:IsA("TextButton")then
button.Activated:Connect(function()
if deb[self.Name] == false or deb[self.Name] == nil then
deb[self.Name] = true
events["BuyTool"]:FireServer(button.Name)
wait(1)
deb[self.Name] = false
end
end)
end
end
end
what i changed? i changed every player to self and at function module:LoadGUIStore(player) i delete player
Isn’t deleting player making it pointless as if it got no player to send a tool to?
This is an incorrect solution and just a workaround to keep the :. This is not object oriented, it’s lazy. The correct way would be dropping the : in favor of a ., as that frees the extra parameter.
This is not OOP, you have no object, you have no self state, it’s no different than using a normal parameter.
Thank for the Information but this have nothing to do with OOP, It’s just a regular BuyItem Function not an object