(Tycoon) Droppers code review

Help and Feedback Code Review
, ,
#1
  • I want to make, so that I wouldn’t have to type this over and over again and if I made a second floor then I would have to make the code again, this repetition is huge suffering.
  • Maybe script.Parent or modeling I’ve considered, but that doesn’t work
-- local b1 = 1
local b2 = 2
local b3 = 3
local b4 = 4
local b5 = 5
local b6 = 6
local b7 = 7
local b8 = 8

local TimeCounter = 2

local Droppers = {workspace.Dropper1,workspace.Dropper2,workspace.Dropper3,workspace.Dropper4,workspace.Dropper5,workspace.Dropper6,workspace.Dropper7,
	workspace.Dropper8
}

local Dropper1 = Droppers[1]
local Dropper2 = Droppers[2]
local Dropper3 = Droppers[3]
local Dropper4 = Droppers[4]
local Dropper5 = Droppers[5]
local Dropper6 = Droppers[6]
local Dropper7 = Droppers[7]
local Dropper8 = Droppers[8]

local BuyDropper1 = workspace.BuyDropper1
local BuyDropper2 = workspace.BuyDropper2
local BuyDropper3 = workspace.BuyDropper3

local BuyDroppers = {BuyDropper1,BuyDropper2,BuyDropper3}

local Drop = game.Workspace.Drop
local V = game.Workspace.V
local Money = V.SurfaceGui.Frame.TextLabel1
local Door = game.Workspace.Door

Door.Touched:Connect(function(otherPart)
	local Player = game.Players:FindFirstChild(otherPart.Parent.Name)
	if Player then
		BuyDroppers[1].BillboardGui.Enabled = true
	end
end)

local function SpawnCubic1()
	while true do
		wait(TimeCounter)
		local Cubic = game.ServerStorage.Cubic
		local ClonedCubic = Cubic:Clone()
		ClonedCubic.Name = "Cubic"
		ClonedCubic.Parent = game.Workspace
		ClonedCubic.Size = Vector3.new(1,1,1)
		ClonedCubic.Position = Droppers[b1].DropPart.Position
		ClonedCubic.Position = ClonedCubic.Position - Vector3.new(0,1,0)
	end
end

local function SpawnCubic2()
	while true do
		wait(TimeCounter)
		local Cubic = game.ServerStorage.Cubic
		local ClonedCubic = Cubic:Clone()
		ClonedCubic.Name = "Cubic"
		ClonedCubic.Parent = game.Workspace
		ClonedCubic.Size = Vector3.new(1,1,1)
		ClonedCubic.Position = Droppers[b2].DropPart.Position
		ClonedCubic.Position = ClonedCubic.Position - Vector3.new(0,1,0)
	end
end

local function SpawnCubic3()
	while true do
		wait(TimeCounter)
		local Cubic = game.ServerStorage.Cubic
		local ClonedCubic = Cubic:Clone()
		ClonedCubic.Name = "Cubic"
		ClonedCubic.Parent = game.Workspace
		ClonedCubic.Size = Vector3.new(1,1,1)
		ClonedCubic.Position = Droppers[b3].DropPart.Position
		ClonedCubic.Position = ClonedCubic.Position - Vector3.new(0,1,0)
	end
end

BuyDroppers[1].Touched:Connect(function()
	BuyDroppers[2].BillboardGui.TextLabel.Text = "Buy Dropper[100$]"
	for i,v in pairs(Dropper1:GetChildren()) do
		v.Anchored = true; v.CanCollide = true; v.Transparency = 0;
	end
end)


BuyDroppers[1].TouchEnded:Connect(function()
	BuyDroppers[1]:Destroy()
	BuyDroppers[2].CanCollide = true
	BuyDroppers[2].Transparency = true
	SpawnCubic1()
end)

BuyDroppers[b2].Touched:Connect(function()
	BuyDropper3.BillboardGui.Enabled = true
	for i,v in pairs(Droppers[b2]:GetChildren()) do
		v.Anchored = true; v.CanCollide = true; v.Transparency = 0;
	end
end)

BuyDroppers[b2].TouchEnded:Connect(function()
	BuyDroppers[b2]:Destroy()
	BuyDroppers[b3].CanCollide = true
	BuyDroppers[b3].Transparency = true
	SpawnCubic2()
end)

BuyDroppers[b3].Touched:Connect(function()
	BuyDropper3.BillboardGui.Enabled = true
	for i,v in pairs(Droppers[b3]:GetChildren()) do
		v.Anchored = true; v.CanCollide = true; v.Transparency = 0;
	end
end)

BuyDroppers[b3].TouchEnded:Connect(function()
	BuyDroppers[b3]:Destroy()
	--BuyDroppers[b4].CanCollide = true
	--BuyDroppers[b4].Transparency = true
	SpawnCubic3()
end)




local Module = require(script.Parent.MoneyScript)

Drop.Touched:Connect(function(otherPart)
	if otherPart:IsA("Part") and otherPart.Name == "Cubic" then
		print("yes")
		otherPart:Destroy()
		Module.Cash = Module.Cash + 5
		print(Module.Cash)
	end
end)
#2

I suggest using a ModuleScript to make this happen. Pretty much instead of having to rewrite the code over and over again, you can call a function which is going to be no more than a line of code to access.

#3

First of all, your code could probably be rewritten and cut down by 90%, which would make future things easy.

Why are you adding droppers to the array only to set them to the separate local variables making the array pretty much pointless.

local Dropper1 = Droppers[1]
local Dropper2 = Droppers[2]
<..>

Are you aware that you can use arguments in fuctions? Meaning, you don’t need three SpawnCubic functions, since you can have one and pass dropper to it.

local function SpawnCubic(argDropper)
	while true do
		wait(TimeCounter)
		local Cubic = game.ServerStorage.Cubic
		local ClonedCubic = Cubic:Clone()
		ClonedCubic.Name = "Cubic"
		ClonedCubic.Parent = game.Workspace
		ClonedCubic.Size = Vector3.new(1,1,1)
		ClonedCubic.Position = argDropper.DropPart.Position
		ClonedCubic.Position = ClonedCubic.Position - Vector3.new(0,1,0)
	end
end

Edit: I just realised that you have while true do in your functions… Why? Unless I’m mistaken, they will freeze your game up.

Basically, any variable or function that has an index in its name and is multiplied should go, that’ll make your code reusable. And, yes, use ModuleScript.

You can also add attributes to your in-game objects, so you’d know whats their index/type/price/etc, without code (just validate it on server side).

#4

how would I do a multiply variable

#5

Sorry, I wasn’t very clear there (didn’t know how to say it in english), but you definitely don’t need b2, b3, b4 or Dropper1, Dropper2, Dropper3 variables in your code.

You can also add numeric attribute or NumberValue as a child to your in-game objects which would hold dropper index it activates. Or you can parent-child them.That way you will not need multiple Touched/TouchEnded connections either.

BuyDroppers.TouchEnded:Connect(function()
    local Index = BuyDroppers:GetAttribute("index")
--- or BuyDroppers.IndexNumber.Value
    local Dropper = Droppers[Index] -- check if nil before continuing

	BuyDroppers:Destroy()
	BuyDroppers.CanCollide = true
	BuyDroppers.Transparency = true

	SpawnCubic(Dropper)
end)