Elevator Code Cleanup

Help and Feedback Code Review
#1

I recently quickly made this code for an elevator, it is very glitchy but it works. I want to see if anyone has any suggestions for what to change. Thanks!

local TweenService = game:GetService("TweenService")
local DoorTween = TweenInfo.new(1,Enum.EasingStyle.Quart,Enum.EasingDirection.Out,0,false,0)
local MainTween = TweenInfo.new(4,Enum.EasingStyle.Bounce,Enum.EasingDirection.Out,0,false,0)
local Moving = false

local CarModel = script.Parent.Car
local Shaft = script.Parent.Shaft
local CurrentStory = script.Parent.CurrentStory

local Story1DoorRight = Shaft.Floor1.RightDoor
local Story1DoorLeft = Shaft.Floor1.LeftDoor



local Story2DoorRight = Shaft.Floor2.RightDoor
local Story2DoorLeft = Shaft.Floor2.LeftDoor

local Story3DoorRight = Shaft.Floor3.RightDoor
local Story3DoorLeft = Shaft.Floor3.LeftDoor

local CarDoorLeft = CarModel.LeftDoor
local CarDoorRight = CarModel.RightDoor

local Story1ButtonCD = Shaft.Floor1.ButtonOpen.ClickDetector
local Story2ButtonCD = Shaft.Floor2.ButtonOpen.ClickDetector
local Story3ButtonCD = Shaft.Floor3.ButtonOpen.ClickDetector
local CarButtonCF1 = CarModel.Button1.ClickDetector
local CarButtonCF2 = CarModel.Button2.ClickDetector
local CarButtonCF3 = CarModel.Button3.ClickDetector

local story1p = CarModel.PrimaryPart.Position
local story2p = story1p + Vector3.new(0,CarModel.PrimaryPart.Size.Y,0)
local story3p = story2p + Vector3.new(0,CarModel.PrimaryPart.Size.Y,0)
	
	
function ChangeDoors(s,both,is)
		if both ~= nil then
		if both == true then
		local dl = Shaft["Floor"..s].LeftDoor
		local dr = Shaft["Floor"..s].RightDoor
		if is == "Open" then
		if dl.Opened.Value == false and dr.Opened.Value == false then
		local DoorTween1 = TweenService:Create(dl,DoorTween,{Position = Vector3.new(dl.Position.X,dl.Position.Y,dl.Position.Z+3.25)})
		DoorTween1:Play()
		local DoorTween2 =  TweenService:Create(dr,DoorTween,{Position = Vector3.new(dr.Position.X,dr.Position.Y,dr.Position.Z-3.25)})
		DoorTween2:Play()
		dl.Opened.Value = true
		dr.Opened.Value = true
		end
				else
				if dr.Opened.Value == true and dl.Opened.Value == true then
			local DoorTween1 = TweenService:Create(dl,DoorTween,{Position = Vector3.new(dl.Position.X,dl.Position.Y,dl.Position.Z-3.25)})
		DoorTween1:Play()
		local DoorTween2 =  TweenService:Create(dr,DoorTween,{Position = Vector3.new(dr.Position.X,dr.Position.Y,dr.Position.Z+3.25)})
		DoorTween2:Play()
		dl.Opened.Value = false
		dr.Opened.Value = false
		end
		end
		end
		end
		if is == "Open" then
		if CarDoorRight.Opened.Value == false and CarDoorLeft.Opened.Value == false then
		local DoorTween3 =  TweenService:Create(CarDoorLeft,DoorTween,{Position = Vector3.new(CarDoorLeft.Position.X,CarDoorLeft.Position.Y,CarDoorLeft.Position.Z+CarDoorLeft.Size.X)})
		DoorTween3:Play()
		local DoorTween4 =  TweenService:Create(CarDoorRight,DoorTween,{Position = Vector3.new(CarDoorRight.Position.X,CarDoorRight.Position.Y,CarDoorRight.Position.Z-CarDoorRight.Size.X)})
		DoorTween4:Play()
		CarDoorRight.Opened.Value = true
		CarDoorLeft.Opened.Value = true
		end
	else
		if CarDoorRight.Opened.Value == true and CarDoorLeft.Opened.Value == true then
			local DoorTween3 =  TweenService:Create(CarDoorLeft,DoorTween,{Position = Vector3.new(CarDoorLeft.Position.X,CarDoorLeft.Position.Y,CarDoorLeft.Position.Z-CarDoorLeft.Size.X)})
		DoorTween3:Play()
		local DoorTween4 =  TweenService:Create(CarDoorRight,DoorTween,{Position = Vector3.new(CarDoorRight.Position.X,CarDoorRight.Position.Y,CarDoorRight.Position.Z+CarDoorRight.Size.X)})
		DoorTween4:Play()
		end
		CarDoorRight.Opened.Value = false
		CarDoorLeft.Opened.Value = false
		end
end
	wait(8)
	
	
	function ChangeStories(story)
		Moving = true
		ChangeDoors(CurrentStory.Value,true,"Close")
		wait(1)
		if story == 2 then 
			CurrentStory.Value = 2
			if CarModel.Center.Position.Y < story2p.Y then
				repeat
					CarModel:SetPrimaryPartCFrame(CFrame.new(CarModel.PrimaryPart.Position.X,CarModel.PrimaryPart.Position.Y+0.2,CarModel.PrimaryPart.Position.Z))
					wait() 
				until CarModel.Center.Position.Y >= story2p.Y 
				
			else
				repeat
					CarModel:SetPrimaryPartCFrame(CFrame.new(CarModel.PrimaryPart.Position.X,CarModel.PrimaryPart.Position.Y-0.2,CarModel.PrimaryPart.Position.Z))
					wait() 
				until CarModel.Center.Position.Y <= story2p.Y 
			end
			
		end
			
		if story == 1 then
			CurrentStory.Value = 1 
			if CarModel.Center.Position.Y < story1p.Y then
				repeat
					CarModel:SetPrimaryPartCFrame(CFrame.new(CarModel.PrimaryPart.Position.X,CarModel.PrimaryPart.Position.Y+0.2,CarModel.PrimaryPart.Position.Z))
					
					wait() 
				until CarModel.Center.Position.Y >= story1p.Y 
				
			else
				repeat
					CarModel:SetPrimaryPartCFrame(CFrame.new(CarModel.PrimaryPart.Position.X,CarModel.PrimaryPart.Position.Y-0.2,CarModel.PrimaryPart.Position.Z))
					wait() 
				until CarModel.Center.Position.Y <= story1p.Y 
				CarModel:SetPrimaryPartCFrame(CFrame.new(CarModel.PrimaryPart.Position.X,CarModel.PrimaryPart.Position.Y+0.2,CarModel.PrimaryPart.Position.Z))
			end
			
		end
		
		if story == 3 then 
			CurrentStory.Value = 3
			if CarModel.Center.Position.Y < story3p.Y then
				repeat
					CarModel:SetPrimaryPartCFrame(CFrame.new(CarModel.PrimaryPart.Position.X,CarModel.PrimaryPart.Position.Y+0.2,CarModel.PrimaryPart.Position.Z))
					wait() 
				until CarModel.Center.Position.Y >= story3p.Y 
				
			else
				repeat
					CarModel:SetPrimaryPartCFrame(CFrame.new(CarModel.PrimaryPart.Position.X,CarModel.PrimaryPart.Position.Y-0.2,CarModel.PrimaryPart.Position.Z))
					wait() 
				until CarModel.Center.Position.Y <= story3p.Y 
			end
			
		end
					
		Moving = false
		
		
		ChangeDoors(CurrentStory.Value,true,"Open")
	end
	
	
	
	
	
	
	
	
	Story1ButtonCD.MouseClick:Connect(function()
		if CurrentStory.Value == 1 then
			ChangeDoors(1,true,"Open")
	else
		repeat wait() until Moving == false
		wait(5)
			ChangeStories(1)
		end
	end)
	
	Story2ButtonCD.MouseClick:Connect(function()
		if CurrentStory.Value == 2 then
			ChangeDoors(2,true,"Open")
	else
		repeat wait() until Moving == false
		wait(5)
			ChangeStories(2)
		end
	end)
	Story3ButtonCD.MouseClick:Connect(function()
		if CurrentStory.Value == 3 then
			ChangeDoors(3,true,"Open")
	else
		repeat wait() until Moving == false
		wait(5)
			ChangeStories(3)
		end
	end)

	CarButtonCF2.MouseClick:Connect(function()
		if Moving == false then
		if CurrentStory.Value ~= 2 then
		ChangeStories(2)
		end
		end
	end)
	
CarButtonCF1.MouseClick:Connect(function()
	if Moving == false then
		if CurrentStory.Value ~= 1 then
		ChangeStories(1)
		end
		end
	end)

CarButtonCF3.MouseClick:Connect(function()
	if Moving == false then
	if CurrentStory.Value ~= 3 then
		ChangeStories(3)
	end
	end
	end)
	
	while wait() do
		local Surface1 = Shaft.Floor1.OuterWallTop.SurfaceGui.Main
		local Surface2 = Shaft.Floor2.OuterWallTop.SurfaceGui.Main
		local Surface3 = Shaft.Floor3.OuterWallTop.SurfaceGui.Main
		local Surface4 = CarModel.WallTop.SurfaceGui.Main
		if Moving == false then
		Surface1.Text = CurrentStory.Value
		Surface2.Text = CurrentStory.Value
		Surface3.Text = CurrentStory.Value
		Surface4.Text = CurrentStory.Value
		end
		if Moving == true then
			Surface1.Text = "Going to "..CurrentStory.Value
			Surface2.Text = "Going to "..CurrentStory.Value
			Surface3.Text = "Going to "..CurrentStory.Value
			Surface4.Text = "Going to "..CurrentStory.Value
		end
		
	end

My Explorer
Annotation 2020-05-03 164314

Here is the game
https://www.roblox.com/games/4980745153/Elevator-Testing

1 Like
#2

If your code is functional please post it in #help-and-feedback:code-review instead. #help-and-feedback:scripting-support is for code with errors or is not functioning as intedned.

#5

The code is already working, but it needs improvement. Thus matching the criteria of #help-and-feedback:code-review. No need to move it to #help-and-feedback:scripting-support. Misread.

The script is extremely messy, and I will take my time to see if I can rewrite it and throw suggestions at it.

List of suggestions:

  1. Try adding a few comment lines to sort out functions and variables. Would make the workflow extremely easy.
  2. Best way to sort the opening and closing logic is to create a function for each.
  3. There’s a wild wait(8) on the global scope without a reason, perhaps removing it would fix the code smell.
  4. Sorting the script per story requires a table with the keys matching to the story with the function to each.
  5. For each story’s calling button, create one function and connect each button to one same function using an argument telling the function that it is on story X.
  6. while wait() do main loop is unnecessary as you only need to depend on the buttons’ events rather than the loop itself.
1 Like
#6

Very Helpful! I should really start fixing my script.