Could my Contract script use improving?

I made a cool contract thing, that I think works. It does look faulty to me though unless I did something that could be simplified. I’m open to improvements/feedback!

local MarketplaceService = game:GetService("MarketplaceService")
local StarterGUI = game:GetService("StarterGui")
local Lighting = game:GetService("Lighting")

local Player = game:GetService("Players").LocalPlayer
local Character = Player.Character or Player.CharacterAdded:Wait()
local HRP = Character:WaitForChild("HumanoidRootPart")

if not MarketplaceService:UserOwnsGamePassAsync(Player.UserId, 138031602) then return end

local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Remotes = ReplicatedStorage:WaitForChild("Remotes")

local VolumeEvent = Remotes:WaitForChild("BG_Volume")

local Modules = ReplicatedStorage:WaitForChild("Modules")
local FollowerModule = require(Modules:WaitForChild("Follower"))

local ContractRS = ReplicatedStorage:WaitForChild("ContractRS")
local Stand = ContractRS:WaitForChild("GhostKing")
local Ghoul = Stand.Ghoul
local ProximityPrompt = Ghoul.HumanoidRootPart.Talk

local TS = game:GetService("TweenService")

local tweenInfo = TweenInfo.new(1, Enum.EasingStyle.Quint, Enum.EasingDirection.InOut)

local Defused = false

--if MarketplaceService:UserOwnsGamePassAsync(Player.UserId, 138031602) then
--	Stand.Parent = workspace
--	--Ghoul.Humanoid:LoadAnimation(Ghoul.Data.Idle):Play()
--ProximityPrompt.Enabled = true
--end

Stand.Parent = workspace
Ghoul.Humanoid:LoadAnimation(Ghoul.Data.Idle):Play()
ProximityPrompt.Enabled = true

ProximityPrompt.Triggered:Connect(function()
	ProximityPrompt.Enabled = false
	local GUI = game:GetService("Lighting"):WaitForChild("Tracker"):Clone()
	local Stages = workspace:WaitForChild("Stage"):WaitForChild("Stages")
	local MaxStageNumber = Stages[math.max(#Stages:GetChildren() - 1)]
	local Target = Stages:FindFirstChild(tostring(MaxStageNumber))
	
	GUI.Parent = Target
	GUI.Adornee = Target
	
	VolumeEvent:Fire(0)
	
	HRP.CFrame = workspace.Stage.Stages:FindFirstChild("0").CFrame
	
	--Player.PlayerGui.StageTransfer.Enabled = false
		
	Target.Touched:Connect(function(hit)
		if not Defused then
			if hit.Parent:FindFirstChild("Bomb") then
				Defused = true
				GUI:Destroy()
				TS:Create(Lighting.DeathWish.EndMusic, tweenInfo, {Volume = 0}):Play()
				TS:Create(Lighting.DeathWish.StartMusic, tweenInfo, {Volume = 0}):Play()
				
				task.wait(1)
				
				script.SUCCESS:Play()
				Lighting.DeathWish.StartMusic:Stop()
				Lighting.DeathWish.EndMusic:Stop()
				FollowerModule.Destroy(Player, "Ghost")
				Character.Humanoid:UnequipTools()
				ReplicatedStorage.Remotes.RespawnRE:FireServer()
				ProximityPrompt.Enabled = true
				VolumeEvent:Fire(1)
				Remotes.RewardRE:FireServer(1000, 1)
			end
		end
	end)

	Character.Humanoid.Changed:Connect(function()
		if Character.Humanoid.Health == 0 then
			GUI:Destroy()
			script["GAME OVER"]:Play()
			Lighting.DeathWish.StartMusic:Stop()
			Lighting.DeathWish.EndMusic:Stop()
			FollowerModule.Destroy(Player, "Ghost")
			Character.Humanoid:UnequipTools()
			ReplicatedStorage.Remotes.RespawnRE:FireServer()
			ProximityPrompt.Enabled = true
			VolumeEvent:Fire(1)
		end
	end)
	
	ContractRS:WaitForChild("Remotes"):WaitForChild("EndContract").Event:Connect(function()
		GUI:Destroy()
		script["GAME OVER"]:Play()
		Lighting.DeathWish.EndMusic:Stop()
		Lighting.DeathWish.StartMusic:Stop()
		FollowerModule.Destroy(Player, "Ghost")
		Character.Humanoid:UnequipTools()
		ReplicatedStorage.Remotes.RespawnRE:FireServer()
		ProximityPrompt.Enabled = true
		VolumeEvent:Fire(1)
	end)
	
	ReplicatedStorage.Remotes.StartTimer:Fire(300)
	ContractRS.Remotes.AddBomb:FireServer("START")
	StarterGUI:SetCoreGuiEnabled(Enum.CoreGuiType.Backpack, false)
	StarterGUI:SetCoreGuiEnabled(Enum.CoreGuiType.PlayerList, false)
	task.wait(2.5)
	script.EquippedSound:Play()
end)

That first guard clause seems a bit off, you should try giving it more of a special place earlier in the script so you (and possible teammates who work with your code) can faster understand why that exists. Try to push that guard clause outside of the script and initialize the script only when the if doesn’t return.

I’d make every HRP teleportation move you up by Vector3.new(0, 5, 0) but that depends on how you build the map.

It seems to me like you’re trusting the client too much with the work, try to remake all your remotes be requests that the server will verify instead of requesting a certain reward. This can easily be exploited (don’t rely on Byfron to fix all the exploits yet, it’s too early for that).

I personally like putting my event connections on the bottom and connecting them to functions because that lets me name the code block, try it. For example
ProximityPrompt.Triggered:Connect(startStage)
Target.Touched:Connect(defuseBomb)
Character.Humanoid.Changed:Connect(updateHumanoid)
ContractRS:WaitForChild("Remotes"):WaitForChild("EndContract").Event:Connect(endContract)

1 Like