Coin.Touched:Connect(function(Part)
local Char = Part:FindFirstAncestorOfClass("Model")
if Char then
local Player = Players:GetPlayerFromCharacter(Char)
if Player then
local Data = shared[Player.UserId]
if Data then
Data[2] += Value[Type]
Coin:Destroy()
end
end
end
end)
Or:
Coin.Touched:Connect(function(Part)
local Char = Part:FindFirstAncestorOfClass("Model")
if not Char then return end
local Player = Players:GetPlayerFromCharacter(Char)
if not Player then return end
local Data = shared[Player]
if not Data then return end
Data[2] += Value[Type]
Coin:Destroy()
end)
But the problem is, if Char is nil, then there is, at best, an unnecessary call to GetPlayerFromCharacter and at worst, an error, if that function does not accept nil values.
Lowkey I use #2 except I add extra spaces between each variable check. So my scripts look like:
coin.Touched:Connect(function(part)
local char = part:FindFirstAncestorOfClass("Model")
if not char then return end
local player = Players:GetPlayerFromCharacter(char)
if not player then return end
local data = shared[player]
if not data then return end
data[2] += value[t]
coin:Destroy()
end)
Which might take up more space, but it’s so easy to skim through and understand.
local Char = Part:FindFirstAncestorOfClass("Model")
local Player = Char and Players:GetPlayerFromCharacter(Char)
local Data = Player and shared[Player]
if not Data then return end
--code
the 2nd one looks better, but you can add some empty lines to make it easier to read
Coin.Touched:Connect(function(Part)
local Char = Part:FindFirstAncestorOfClass("Model")
if not Char then return end
local Player = Players:GetPlayerFromCharacter(Char)
if not Player then return end
local Data = shared[Player]
if not Data then return end
Data[2] += Value[Type]
Coin:Destroy()
end)
vs
Coin.Touched:Connect(function(Part)
local Char = Part:FindFirstAncestorOfClass("Model")
if not Char then return end
local Player = Players:GetPlayerFromCharacter(Char)
if not Player then return end
local Data = shared[Player]
if not Data then return end
Data[2] += Value[Type]
Coin:Destroy()
end)
but what Player has suggested is more readable than both of them
Inversion is pretty usefull, as it reduce nesting, but what you really should think about is code redability, simpliest methood to do that is to separate your script by empty lines + add scoping to return statements
Coin.Touched:Connect(function(Part)
local Char = Part:FindFirstAncestorOfClass("Model")
if not Char then
return
end
local Player = Players:GetPlayerFromCharacter(Char)
if not Player then
return
end
local Data = shared[Player]
if not Data then
return
end
Data[2] += Value[Type]
Coin:Destroy()
end)
EDIT: Also you shouldn’t use global tables, try using module scripts as a way of storing data, because they are better with performance and organization
Another thing is that you probably have hard-coded variable, Data[2] can’t have other value than that, maybe try naming variable and using it as costant instead of magic number?