Made a door system. How can it improve?

Help and Feedback Code Review
#1

Greetings!

I made this door system where the door opens, and it fades and then teleports you into the place.

Here is a video demonstration:

However, I think my code is pretty iffy.
I have to duplicate everything for both the outside door and Inside door.

local CollectionService = game:GetService("CollectionService")
local TweenService = game:GetService("TweenService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Players = game:GetService("Players")

local DoorOpening = TweenInfo.new(1.4, Enum.EasingStyle.Sine, Enum.EasingDirection.InOut, 0, false)
local Doors = CollectionService:GetTagged("Doors")

for _, Door in Doors do
	Door.Touched.Touched:Connect(function(Part)
		local Humanoid = Part.Parent:FindFirstChildOfClass("Humanoid")
		if Humanoid then
			local Player = Players:GetPlayerFromCharacter(Part.Parent)
			if Player then
				if not Player:GetAttribute("UsingDoor") then
					Player:SetAttribute("UsingDoor", true)
					local StarterDoorOpenedValue = Instance.new("CFrameValue")
					local TeleportDoorOpenedValue = Instance.new("CFrameValue")
					local TeleportDoor = Door.TeleportDoor.Value
					StarterDoorOpenedValue.Value = Door.Door2:GetPivot()
					TeleportDoorOpenedValue.Value = TeleportDoor.Door1:GetPivot()
					local Tween = TweenService:Create(StarterDoorOpenedValue, DoorOpening, {Value = Door.Door2.WorldPivot * CFrame.Angles(0, math.rad(-90), 0)})
					local Tween2 = TweenService:Create(TeleportDoorOpenedValue, DoorOpening, {Value = TeleportDoor.Door1.WorldPivot * CFrame.Angles(0, math.rad(90), 0)})
					local DoorChanged = StarterDoorOpenedValue.Changed:Connect(function()
						Door.Door2:PivotTo(StarterDoorOpenedValue.Value)
					end)
					local TeleportDoorChanged = TeleportDoorOpenedValue.Changed:Connect(function()
						TeleportDoor.Door1:PivotTo(TeleportDoorOpenedValue.Value)
					end)
					Tween:Play()
					Tween2:Play()
					Door.DoorTop.DoorOpen:Play()
					TeleportDoor.DoorTop.DoorOpen:Play()
					task.spawn(function()
						Tween.Completed:Wait()
						Door.DoorTop.ExtraSound:Play()
						TeleportDoor.DoorTop.ExtraSound:Play()
					end)
					task.delay(1, function()
						ReplicatedStorage.Fade:FireClient(Player, true)
						task.wait(0.5)
						Player.Character:PivotTo(Door.TeleportDoor.Value.TeleportPart.CFrame)
						ReplicatedStorage.Fade:FireClient(Player, false)
						local ClosingTween = TweenService:Create(StarterDoorOpenedValue, DoorOpening, {Value = Door.Door2.WorldPivot * CFrame.Angles(0, math.rad(90), 0)})
						local ClosingTween2 = TweenService:Create(TeleportDoorOpenedValue, DoorOpening, {Value = TeleportDoor.Door1.WorldPivot * CFrame.Angles(0, math.rad(-90), 0)})
						ClosingTween:Play()
						ClosingTween2:Play()
						ClosingTween.Completed:Wait()
						Door.DoorTop.DoorClose:Play()
						TeleportDoor.DoorTop.DoorClose:Play()
						Player:SetAttribute("UsingDoor", false)
					end)
				end
			end
		end
	end)
end

If you can help make this better, that would be great.

Sincerely, Will

#2

You should make it so that if a player is within a certain range of those doors, it’ll open. Then if the player touches the black parts of the door, they teleport.

1 Like
#3

Use CollectionService; you can bind the main function to all the doors, this makes your code look a lot nicer.

Granted, you’ll have to tag all the doors.

#4

Here is a suggested revision of the script:

local CollectionService = game:GetService("CollectionService")
local TweenService = game:GetService("TweenService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Players = game:GetService("Players")

-- store the DoorOpening TweenInfo in a variable to use later
local doorOpeningTweenInfo = TweenInfo.new(1.4, Enum.EasingStyle.Sine, Enum.EasingDirection.InOut, 0, false)

-- store all Doors in a variable
local doors = CollectionService:GetTagged("Doors")

-- loop through each Door
for _, door in pairs(doors) do
	door.Touched.Touched:Connect(function(part)
		local humanoid = part.Parent:FindFirstChildOfClass("Humanoid")
		if humanoid then
			local player = Players:GetPlayerFromCharacter(part.Parent)
			if player then
				if not player:GetAttribute("UsingDoor") then
					player:SetAttribute("UsingDoor", true)
					
					local starterDoorOpenedValue = Instance.new("CFrameValue")
					local teleportDoorOpenedValue = Instance.new("CFrameValue")
					local teleportDoor = door.TeleportDoor.Value
					starterDoorOpenedValue.Value = door.Door2:GetPivot()
					teleportDoorOpenedValue.Value = teleportDoor.Door1:GetPivot()
					
					-- create the DoorOpening tweens using the doorOpeningTweenInfo variable
					local tween = TweenService:Create(starterDoorOpenedValue, doorOpeningTweenInfo, {Value = door.Door2.WorldPivot * CFrame.Angles(0, math.rad(-90), 0)})
					local tween2 = TweenService:Create(teleportDoorOpenedValue, doorOpeningTweenInfo, {Value = teleportDoor.Door1.WorldPivot * CFrame.Angles(0, math.rad(90), 0)})
					
					local doorChanged = starterDoorOpenedValue.Changed:Connect(function()
						door.Door2:PivotTo(starterDoorOpenedValue.Value)
					end)
					local teleportDoorChanged = teleportDoorOpenedValue.Changed:Connect(function()
						teleportDoor.Door1:PivotTo(teleportDoorOpenedValue.Value)
					end)
					
					tween:Play()
					tween2:Play()
					door.DoorTop.DoorOpen:Play()
					teleportDoor.DoorTop.DoorOpen:Play()
					
					-- use a coroutine instead of a delay to play the ExtraSound
					spawn
#5

Is this basically just my script but in camelCase and a suggestion, and extra gaps?