What is the best approach to set an on-clicked event as an anonymous function?
Hello,
I've been developing a script that destroys instances when they are clicked. The problem is, I would have to specify the part and perform the on-clicked event, rather than call if from ReplicatedStorage, which is one approach that I am interested in doing so.
The following is my code for the generator:
Generate = function(Self)
--hint GUI to count regenerating bricks
local hintRegen= Instance.new("Hint",game.Workspace.MessagesGroup)
--create a group called "DirtBlockCloneGroup", with if statement
local dirtBlockCloneModel= Instance.new("Model",game.Workspace)
dirtBlockCloneModel.Name= "DirtBlockCloneGroup"
for timer= 1,0,-1 do --timer that counts from 3/ any # to 0 by seconds
wait(1)
print(timer)
if timer== 0 then
print("Done.")--ends timer, then does action below
end--end if
end--end for
local x0= game.Workspace.DirtBlock.Position.X
local y0= game.Workspace.DirtBlock.Position.Y
local z0= game.Workspace.DirtBlock.Position.Z
--sets whatever position the original dirt block is in, stores in variables
local original= Vector3.new(x0,y0,z0)
local zIncr= 3 --increments new clone blocks by changing z coordinate
--line below checks and gets testing plate size
local zSizeBase= game.Workspace.BasePlate.Size.Z
local zSizeCheck= 3--baseplate comparison, so clone won't pass plate boundaries
local xSizeBase= game.Workspace.BasePlate.Size.Z
local xSizeCheck= 3
local xIncr= 3
--brick counter begins here
local numClonedBricks= 0
for i= 1,1 do--sets how many layers of bricks
for i= 1,(xSizeBase/3) do-- because it increments by 3 studs
while zSizeCheck<zSizeBase do
local clone= game.Workspace.DirtBlock:Clone()-- creates clone
clone.Name= "DirtBlockClone" -- changes name of dirt block
clone.Parent= game.Workspace.DirtBlockCloneGroup --adds the instance into the Workspace
clone.Position= Vector3.new(x0,y0,z0-zIncr)
zIncr= zIncr+3-- ex. 3 away from original, 6 away, 9 away etc.
zSizeCheck= zSizeCheck+3
numClonedBricks= numClonedBricks+1
hintRegen.Text="Bricks generated: "..tostring(numClonedBricks)
--print and store value globally/ accessible from anywhere
--print(numClonedBricks)--substitute for hint GUI
local found= game.Workspace:FindFirstChild("ClonedBricksValue")
if found then
game.Workspace.ClonedBricksValue:Destroy()
end--ends if statement
local intValue= Instance.new("IntValue",game.Workspace)
intValue.Name= "ClonedBricksValue"
game.Workspace.ClonedBricksValue.Value= numClonedBricks
--click detector: 'clicked' variable should be global so child script of Script can call it
clicked= Instance.new("ClickDetector")--leave global
clicked.Parent= clone
clicked.MouseClick:connect(
function()
--print("Destroyed: "..tostring(numClonedBricks))--it works so far..
game.Workspace.DirtBlockCloneGroup.DirtBlockClone:Destroy()
game.Workspace.ClonedBricksValue.Value= game.Workspace.ClonedBricksValue.Value-1
msg= Instance.new("Message",game.Workspace)--keep global
msg.Text= "Brick is destroyed somewhere."
wait(1)
game.Workspace.Message:Destroy()
--should label each block an ID
end
)
As you can see, it is not the best idea to put the function as a whole directly in the loop, and I may associate it with the bugs/ glitches I have been receiving. In single player testing mode, the script works; the messages pop up and a brick is deleted (somewhere; that's another problem that I will not elaborate for this question). However online, none of the commands within the function work; I've made sure that the variables are global. Moreover, this is one of a couple ways that I had tried:
return
{
Speed= 5,
OnClickedModule= function(Self)
click= Instance.new("ClickDetector",game.Workspace)
click= Parent.MouseClicked:Connect()
function()
--action goes here
end
end
}
Therefore, with something like this I can call the script from my main script, then specify my variables there.
Edit:
I was thinking about posting this update as a separate post, but instead posted below my original. The following is my WIP code so far:
Generate = function(Self)
workspace.numClonedBricks.Value= 0-- makes sure numClonedBricks is set to zero
--sets position of original dirt block broken into dir, stores in variables
local xOriginal= workspace.DirtBlock.Position.X
local yOriginal= workspace.DirtBlock.Position.Y
local zOriginal= workspace.DirtBlock.Position.Z
-- gets baseplate size
local zSizeBase= workspace.BasePlate.Size.Z
local xSizeBase= workspace.BasePlate.Size.X
local incr= 3 --amount to increment by between blocks, x and z dir
local incrY= 0 --amount to increment in the y-dir
--after variable inclusion, the original dirt block can be edited to be replaced by the clones
for yCount= 1,1 do--how many times incrY increments/ how many layers wanted
for x= 0, (xSizeBase-incr), incr do--minus 3 prevents clones from going over base by 1 block
for z= 0, (zSizeBase-incr), incr do--minus 3 prevents clones from going over base by 1 block
local clone= workspace.DirtBlock:Clone()
clone.Name= "DirtBlockClone"
clone.Parent= workspace.DirtBlockCloneGroup
clone.Position= Vector3.new(xOriginal+x,yOriginal+incrY,zOriginal-z)
workspace.numClonedBricks.Value= workspace.numClonedBricks.Value+1
print(workspace.numClonedBricks.Value)
end
wait(0)
end
incrY= incrY+3
end
end,--end function generate
There is still a whole lot of work needed to be done on the code, but so far it works.
0
0
0
1 answer
Global variables are bad. You essentially never want global variables.
There's not really a better way to go about connecting to the MouseClick event, because the event can't help you identify the appropriate brick -- it can only tell you the player. Generally, it's better to avoid defining functions in loops, but when you truly need a closure -- a function that uses local variables defined outside of it -- then that's appropriate (and unavoidable).
Because you need a closure, but because functions can be big, frequently you define an anonymous function which does nothing but call another named function:
function Clicked(part)
-- do whatever
end
--- elsewhere
for _, part in pairs(parts) do
parts.ClickDetect.MouseClick:connect(function() Clicked(part) end)
end
Variables
Again, as mentioned, you want to use local variables everywhere. You don't often want global variables, and even if you want them, it's probably not a good idea (global variables are evil -- hopefully eventually everyone figures this out)
In a few different places, you make a variable:
local intValue = Instance.new("IntValue",game.Workspace)
and then immediately refer to it by finding it in the workspace:
game.Workspace.ClonedBricksValue.Value= numClonedBricks
(You also do this with msg)
This is silly -- it's dangerous, because you might not be referring to the right one, since names don't have to unique (it looks like this will only affect msg)
game.Workspace
game.Workspace is lengthy. You can say workspace instead. Much neater!
original
Instead of remaking the position, you should just use the position:
local original = workspace.DirtBlock.Position
for i = ...
It appears you're moving zIncr manually.
Why not simplify and use for loops proper?
local y = workspace.DirtBlock.Position.y
for x = 0, xSizeBase, xIncr do
for z = 0, zSizeBase, zIncr do
.....
clone.Position = Vector3.new(x, y, z)
end
end
Formatting
Your code isn't tabbed or spaced well. It makes a huge difference.
ClonedBricksValue
There's no reason to make this a number value. It's messy and more confusing than necessary -- just the one variable is fine. (If you have a value, you should update it in a consistent way, e.g., writing a update() function that sets it, rather than manually incrementing/decrementing it yourself)
Comments
There's no point in including obvious comments. "end of if" is obvious from decent tabbing, especially when it's short. "adds the instance to the workspace" is similarly obvious. Etc.
Total
I imagine the cleaned up version looking something like this:
local numClonedBricks = 0 function Click(part) part:Destroy() numClonedBricks = numClonedBricks - 1 local msg = Instance.new("Message", workspace) msg.Text = "Brick is destroyed somewhere." wait(1) msg:Destroy() --should label each block an ID end Generate = function(Self) --hint GUI to count regenerating bricks local hintRegen = Instance.new("Hint", workspace.MessagesGroup) --create a group called "DirtBlockCloneGroup", with if statement local dirtBlockCloneModel = Instance.new("Model", workspace) dirtBlockCloneModel.Name= "DirtBlockCloneGroup" --sets whatever position the original dirt block is in, stores in variables local original = workspace.DirtBlock.Position local y0 = original.y local zSizeBase = game.Workspace.BasePlate.Size.Z local xSizeBase = game.Workspace.BasePlate.Size.Z --brick counter begins here local incr = 0 -- amount to increment by between blocks for x = 0, xSizeBase, incr do for z = 0, zSizeBase, incr do local clone = game.Workspace.DirtBlock:Clone() clone.Name = "DirtBlockClone" clone.Parent = dirtBlockCloneModel clone.Position = Vector3.new(x, y0, z) numClonedBricks = numClonedBricks + 1 hintRegen.Text ="Bricks generated: " .. tostring(numClonedBricks) local clicked = Instance.new("ClickDetector") clicked.Parent = clone clicked.MouseClick:connect(function() Click(clone) end) end end
0
0
0
0
0
0