Messy elevator code

Help and Feedback Code Review
#1

I have this two floor elevator that has buttons to call it from the outside of each floor and buttons inside the elevator itself that move it to each floor. It all works, and the way I have it organized (if you can even call it that) to have a contingency for each scenario probably isn’t the cleanest or best optimized method. I don’t really know how to go about making this any better than how it is currently.

local ReplicatedStorage = game:GetService("ReplicatedStorage")
local TweenService = game:GetService("TweenService")
local ServerScriptService = game:GetService("ServerScriptService")

local SoundModuleServer = require(ServerScriptService.Modules.SoundModuleServer)

local zonePlus = require(ReplicatedStorage.Utility.Zone)

local soundsFolder = workspace.Audio.Sounds

local elevatorModel = script.Parent
local playerDetector = elevatorModel.playerDetector --invisible part that takes up almost the entire space inside the elevator

local playerZone = zonePlus.new(playerDetector)

local elevatorRoom = elevatorModel.ElevatorRoom --the room the players go inside
local elevatorRoomLeftDoor = elevatorRoom.Door.LeftDoor.PrimaryPart --the left door of the second set of doors inside the elevatorRoom
local elevatorRoomRightDoor = elevatorRoom.Door.RightDoor.PrimaryPart --the right door of the second set of doors inside the elevatorRoom

local elevatorRoomUpButton = elevatorRoom.Buttons.UpButton.Button --the button to go up that is inside the elevator
local elevatorRoomDownButton = elevatorRoom.Buttons.DownButton.Button --the button to go down that is inside the elevator

local floor1Doors = elevatorModel.Floor1Door --the exterior doors of the bottom first floor
local floor1LeftDoor = floor1Doors.Door.LeftDoor.PrimaryPart
local floor1RightDoor = floor1Doors.Door.RightDoor.PrimaryPart

local floor2Doors = elevatorModel.Floor2Door --the exterior doors of the top second floor
local floor2LeftDoor = floor2Doors.Door.LeftDoor.PrimaryPart
local floor2RightDoor = floor2Doors.Door.RightDoor.PrimaryPart

local floor1CallButton = elevatorModel.Floor1Door.Button.Button --exterior button to call the elevator to the bottom first floor
local floor2CallButton = elevatorModel.Floor2Door.Button.Button --exterior button to call the elevator to the top second floor

local floor1FinalPosition = elevatorModel.floor1 --position for where the elevator rooms floor should be when it reaches floor 1
local floor2FinalPosition = elevatorModel.floor2 --position for where the elevator rooms floor should be when it reaches floor 2

local elevatorCurrentFloor = 1 --the current floor the elevator room is on

local doorTime = 2.5 --time for doors to open and close
local elevatorMoveTime = 10 --time for the elevator to move to each floor

local elevatorDoorTweenInfo = TweenInfo.new(doorTime)
local elevatorRoomTweenInfo = TweenInfo.new(elevatorMoveTime)

local floor1DoorState = false --false when exterior doors on floor 1 are closed, set to true when open
local floor2DoorState = false --false when exterior doors on floor 2 are closed, set to true when open
local debounce = false --debounce to stop taking button inputs when elevator or doors are moving

local playersInElevatorTable = {}

playerZone.playerEntered:Connect(function(player)
	if not table.find(playersInElevatorTable, player) then
		table.insert(playersInElevatorTable, player)
		player.Character.Humanoid.JumpHeight = 0
	end
end)
playerZone.playerExited:Connect(function(player)
	if table.find(playersInElevatorTable, player) then
		table.remove(playersInElevatorTable, playersInElevatorTable[player])

		if not player.Character then return end
		player.Character.Humanoid.JumpHeight = 7.2
	end
end)

function playersInElevator(action: string, direction: string?)
	for _, player: Player in playersInElevatorTable do
		if not player then continue end

		local character = player.Character
		local humanoid = character.Humanoid

		if action == "freeze" then
			humanoid.WalkSpeed = 0
			task.wait(0.05)

			character.PrimaryPart.Anchored = true

			local weld = Instance.new("WeldConstraint")
			weld.Part0 = character.PrimaryPart
			weld.Part1 = elevatorRoom.PrimaryPart
			weld.Parent = elevatorRoom.PrimaryPart

			if direction == "up" then
				tweenModel(character, elevatorRoomTweenInfo, character.PrimaryPart.CFrame * CFrame.new(0, 100, 0))
			elseif direction == "down" then
				tweenModel(character, elevatorRoomTweenInfo, character.PrimaryPart.CFrame * CFrame.new(0, -100, 0))
			end
		elseif action == "unfreeze" then
			character.PrimaryPart.Anchored = false
			humanoid.WalkSpeed = 16

			for _, weld in elevatorRoom.PrimaryPart:GetChildren() do
				if not weld:IsA("WeldConstraint") then return end
				weld:Destroy()
			end
		end
	end
end

--used for the interior second set of doors that are inside the elevator room so it follows up and down with the room (each interior doors PrimaryPart is anchored). Is also used to move players characters that are in the elevator
function tweenModel(modelToTween: Model, info, cframe: CFrame)
	local cframeValue = Instance.new("CFrameValue")
	cframeValue.Value = modelToTween:GetPrimaryPartCFrame()

	local tween = TweenService:Create(cframeValue, info, {Value = cframe})
	tween:Play()

	cframeValue.Changed:Connect(function()
		if not modelToTween.PrimaryPart then return end
		modelToTween:SetPrimaryPartCFrame(cframeValue.Value)
	end)

	tween.Completed:Connect(function()
		cframeValue:Destroy()
	end)
end

--used for opening and closing the doors and settings their states accordingly
function doorTween(adjustment: string, leftDoor: Instance, rightDoor: Instance, floor)
	if adjustment == "open" then
		if floor == 1 then
			floor1DoorState = true
		elseif floor == 2 then
			floor2DoorState = true
		end

		TweenService:Create(leftDoor, elevatorDoorTweenInfo, {CFrame = leftDoor.CFrame * CFrame.new(0, 0, 3.7)}):Play()
		TweenService:Create(rightDoor, elevatorDoorTweenInfo, {CFrame = rightDoor.CFrame * CFrame.new(0, 0, -3.7)}):Play()
	elseif adjustment == "close" then
		if floor == 1 then
			floor1DoorState = false
		elseif floor == 2 then
			floor2DoorState = false
		else

			TweenService:Create(leftDoor, elevatorDoorTweenInfo, {CFrame = leftDoor.CFrame * CFrame.new(0, 0, -3.7)}):Play()
			TweenService:Create(rightDoor, elevatorDoorTweenInfo, {CFrame = rightDoor.CFrame * CFrame.new(0, 0, 3.7)}):Play()
		end
	end
end

--what moves the elevator room up and down, all parts are unanchored except the floor and is all welded to the floor part
function moveElevator(floor: Instance, floorCalling: number, insideRoom: boolean)
	if floorCalling == 2 and elevatorCurrentFloor == 1 then
		if floor1DoorState then
			doorTween("close", elevatorRoomLeftDoor, elevatorRoomRightDoor)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)
			task.wait(0.5)

			doorTween("close", floor1LeftDoor, floor1RightDoor, 1)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor1Doors.Door.DoorFrame.SoundPart)
			task.wait(doorTime)
		end
		if floor2DoorState then
			doorTween("close", elevatorRoomLeftDoor, elevatorRoomRightDoor)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)
			task.wait(0.5)

			doorTween("close", floor2LeftDoor, floor2RightDoor, 2)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor2Doors.Door.DoorFrame.SoundPart)
			task.wait(doorTime)
		end

		task.wait(0.5)
		playersInElevator("freeze", "up")

		TweenService:Create(elevatorRoom.PrimaryPart, elevatorRoomTweenInfo, {CFrame = elevatorRoom.PrimaryPart.CFrame * CFrame.new(0, 100, 0)}):Play()
		tweenModel(elevatorRoom.Door, elevatorRoomTweenInfo, elevatorRoom.Door.PrimaryPart.CFrame * CFrame.new(0, 100, 0))

		SoundModuleServer.playSound(soundsFolder.ElevatorMoving, elevatorRoom.PrimaryPart)
		task.wait(elevatorMoveTime)

		task.wait(0.5)
		playersInElevator("unfreeze")

		doorTween("open", elevatorRoomLeftDoor, elevatorRoomRightDoor)
		SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)
		task.wait(0.5)

		doorTween("open", floor2LeftDoor, floor2RightDoor, 2)
		SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor2Doors.Door.DoorFrame.SoundPart)

		task.delay(doorTime, function()
			debounce = false
		end)
	elseif floorCalling == 2 and elevatorCurrentFloor == 2 then
		if insideRoom then
			if floor1DoorState then
				doorTween("close", elevatorRoomLeftDoor, elevatorRoomRightDoor)
				SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)
				task.wait(0.5)

				doorTween("close", floor1LeftDoor, floor1RightDoor, 1)
				SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor1Doors.Door.DoorFrame.SoundPart)
				task.wait(doorTime)
			end
			if floor2DoorState then
				doorTween("close", elevatorRoomLeftDoor, elevatorRoomRightDoor)
				SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)
				task.wait(0.5)

				doorTween("close", floor2LeftDoor, floor2RightDoor, 2)
				SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor2Doors.Door.DoorFrame.SoundPart)
				task.wait(doorTime)
			end

			task.wait(0.5)
			playersInElevator("freeze", "up")

			TweenService:Create(elevatorRoom.PrimaryPart, elevatorRoomTweenInfo, {CFrame = elevatorRoom.PrimaryPart.CFrame * CFrame.new(0, 100, 0)}):Play()
			tweenModel(elevatorRoom.Door, elevatorRoomTweenInfo, elevatorRoom.Door.PrimaryPart.CFrame * CFrame.new(0, 100, 0))

			SoundModuleServer.playSound(soundsFolder.ElevatorMoving, elevatorRoom.PrimaryPart)
			task.wait(elevatorMoveTime)

			task.wait(0.5)
			playersInElevator("unfreeze")

			doorTween("open", elevatorRoomLeftDoor, elevatorRoomRightDoor)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)
			task.wait(0.5)

			doorTween("open", floor2LeftDoor, floor2RightDoor, 2)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor2Doors.Door.DoorFrame.SoundPart)

			task.delay(doorTime, function()
				debounce = false
			end)
		else
			if floor2DoorState then return end
			doorTween("open", elevatorRoomLeftDoor, elevatorRoomRightDoor)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)
			task.wait(0.5)

			doorTween("open", floor2LeftDoor, floor2RightDoor, 2)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor2Doors.Door.DoorFrame.SoundPart)

			task.delay(doorTime, function()
				debounce = false
			end)
		end
	elseif floorCalling == 1 and elevatorCurrentFloor == 2 then
		if floor1DoorState then
			doorTween("close", elevatorRoomLeftDoor, elevatorRoomRightDoor)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)
			task.wait(0.5)

			doorTween("close", floor1LeftDoor, floor1RightDoor, 1)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor1Doors.Door.DoorFrame.SoundPart)
			task.wait(doorTime)
		end
		if floor2DoorState then
			doorTween("close", elevatorRoomLeftDoor, elevatorRoomRightDoor)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)
			task.wait(0.5)

			doorTween("close", floor2LeftDoor, floor2RightDoor, 1)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor2Doors.Door.DoorFrame.SoundPart)
			task.wait(doorTime)
		end

		task.wait(0.5)
		playersInElevator("freeze", "down")

		TweenService:Create(elevatorRoom.PrimaryPart, elevatorRoomTweenInfo, {CFrame = elevatorRoom.PrimaryPart.CFrame * CFrame.new(0, -100, 0)}):Play()
		tweenModel(elevatorRoom.Door, elevatorRoomTweenInfo, elevatorRoom.Door.PrimaryPart.CFrame * CFrame.new(0, -100, 0))

		SoundModuleServer.playSound(soundsFolder.ElevatorMoving, elevatorRoom.PrimaryPart)
		task.wait(elevatorMoveTime)

		task.wait(0.5)
		playersInElevator("unfreeze")

		doorTween("open", floor1LeftDoor, floor1RightDoor, 1)
		SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor1Doors.Door.DoorFrame.SoundPart)

		task.wait(0.5)
		doorTween("open", elevatorRoomLeftDoor, elevatorRoomRightDoor)
		SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)

		task.delay(doorTime, function()
			debounce = false
		end)
	elseif floorCalling == 1 and elevatorCurrentFloor == 1 then
		if insideRoom then
			if floor1DoorState then
				doorTween("close", elevatorRoomLeftDoor, elevatorRoomRightDoor)
				SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)
				task.wait(0.5)

				doorTween("close", floor1LeftDoor, floor1RightDoor, 1)
				SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor1Doors.Door.DoorFrame.SoundPart)
				task.wait(doorTime)
			end
			if floor2DoorState then
				doorTween("close", elevatorRoomLeftDoor, elevatorRoomRightDoor)
				SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)
				task.wait(0.5)

				doorTween("close", floor2LeftDoor, floor2RightDoor, 2)
				SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor2Doors.Door.DoorFrame.SoundPart)
				task.wait(doorTime)
			end

			task.wait(0.5)
			playersInElevator("freeze", "up")

			TweenService:Create(elevatorRoom.PrimaryPart, elevatorRoomTweenInfo, {CFrame = elevatorRoom.PrimaryPart.CFrame * CFrame.new(0, 100, 0)}):Play()
			tweenModel(elevatorRoom.Door, elevatorRoomTweenInfo, elevatorRoom.Door.PrimaryPart.CFrame * CFrame.new(0, 100, 0))

			SoundModuleServer.playSound(soundsFolder.ElevatorMoving, elevatorRoom.PrimaryPart)
			task.wait(elevatorMoveTime)

			task.wait(0.5)
			playersInElevator("unfreeze")

			doorTween("open", floor2LeftDoor, floor2RightDoor, 2)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor2Doors.Door.DoorFrame.SoundPart)

			task.wait(0.5)
			doorTween("open", elevatorRoomLeftDoor, elevatorRoomRightDoor)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)

			task.delay(doorTime, function()
				debounce = false
			end)
		else
			if floor1DoorState then return end
			doorTween("open", floor1LeftDoor, floor1RightDoor, 1)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor1Doors.Door.DoorFrame.SoundPart)
			task.wait(0.5)

			doorTween("open", elevatorRoomLeftDoor, elevatorRoomRightDoor)
			SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)

			task.delay(doorTime, function()
				debounce = false
			end)
		end
	end	

	elevatorCurrentFloor = floorCalling
end

floor1CallButton.ClickDetector.MouseClick:Connect(function()
	if debounce then return end
	if floor1DoorState then return end
	debounce = true

	SoundModuleServer.playSound(soundsFolder["Button Press"], floor1CallButton)
	moveElevator(floor1FinalPosition, 1)
end)
floor2CallButton.ClickDetector.MouseClick:Connect(function()
	if debounce then return end
	if floor2DoorState then return end
	debounce = true

	SoundModuleServer.playSound(soundsFolder["Button Press"], floor2CallButton)
	moveElevator(floor2FinalPosition, 2)
end)

elevatorRoomUpButton.ClickDetector.MouseClick:Connect(function()
	if debounce then return end

	if elevatorCurrentFloor == 2 then 
		if floor2DoorState then return end

		SoundModuleServer.playSound(soundsFolder["Button Press"], elevatorRoomUpButton)

		doorTween("open", elevatorRoomLeftDoor, elevatorRoomRightDoor)
		SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)
		task.wait(0.5)
		
		doorTween("open", floor2LeftDoor, floor2RightDoor, 2)
		SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor2Doors.Door.DoorFrame.SoundPart)

		task.delay(doorTime, function()
			debounce = false
		end)

		return 
	end

	debounce = true
	SoundModuleServer.playSound(soundsFolder["Button Press"], elevatorRoomUpButton)
	moveElevator(floor1FinalPosition, 2, true)
end)
elevatorRoomDownButton.ClickDetector.MouseClick:Connect(function()
	if debounce then return end

	if elevatorCurrentFloor == 1 then 
		if floor1DoorState then return end

		SoundModuleServer.playSound(soundsFolder["Button Press"], elevatorRoomDownButton)

		doorTween("open", elevatorRoomLeftDoor, elevatorRoomRightDoor)
		SoundModuleServer.playSound(soundsFolder.ElevatorDoor, elevatorRoom.Door.Frame.PrimaryPart)
		task.wait(0.5)
		
		doorTween("open", floor1LeftDoor, floor1RightDoor, 1)
		SoundModuleServer.playSound(soundsFolder.ElevatorDoor, floor1Doors.Door.DoorFrame.SoundPart)

		task.delay(doorTime, function()
			debounce = false
		end)

		return 
	end

	debounce = true
	SoundModuleServer.playSound(soundsFolder["Button Press"], elevatorRoomDownButton)
	moveElevator(floor2FinalPosition, 1, true)
end)
3 Likes
#2

Here is a video of it

4 Likes
#3

Hey, I mean if it’snot broke don’t fix it, literally the first rule of programming.

3 Likes
#4

If you’re going to have more than one elevator in your game, I would suggest placing these functions in a ModuleScript in ServerStorage, then let your code call them when events are fired. That way multiple elevators can call these functions without having to duplicate them into every single elevator.

local doorTime = 2.5 --time for doors to open and close
local elevatorMoveTime = 10 --time for the elevator to move to each floor

^ These could be a setting in your ModuleScript.

Settings = {
   doorTime = 2.5 --time for doors to open and close
   elevatorMoveTime = 10 --time for the elevator to move to each floor
}

I would also take code spacing into account, so you have some leeway to read your code in sections.

Hope this helps.

1 Like
#5

Instead of relying on elevatorMoveTime you should calculate the time it would take to get to each floor by using a speed constant and a fairly easy (distance / speed) = time calculation. Reason for this is because it will add consistency to all your elevators no matter how far apart each floor is and not needing to change the so called elevatorMoveTime when you make a new elevator elsewhere in your map.

Another thing you could do is raycast each of the characters or when entering the elevator zone, you should multiply on the delta cframes from the previous heartbeat step to the current one, so there is no need to weld the characters to the elevator. In my game I do this both on the server/client. Reason for the client is to eliminate that jitteriness (Unavoidable but works).

Another thing you could do is because the elevators are in the same spot for the entire game you can actually create the tweens once and save them in a table to be reused. This can be easily used on the doors if you weld them to the chassis of the elevator using the Weld instance (not WeldConstraint), where you’d manipulate the C1 property. And, same for each floor.

Other than it not using collectionservice (components), I’d say as @EpsteinFromEXO said: “if it ain’t broke, don’t fix it”.