What can I remove/change/add (meteor script)

Help and Feedback Code Review
#1

It all works fine, if a video of it is wanted, I can make one. Right now it is supposed to be activated from a button

How can I make this better? What stuff could I add/remove/change? (I know it is a bit messy)
Btw, if you didn’t read the title it’s a meteor

Script 
local currentRing
local currentBlast

Detector.Triggered:Connect(function()
	local y = math.random(5,15)
	local rz = math.random(-200, -5)
	local r = math.random(-400, -5)
	
	local meteor = game.ReplicatedStorage.Meteor:Clone() 
	meteor.Size = Vector3.new(y,y,y)
	meteor.AlignPosition.Position = Vector3.new(r, -15, rz)
	meteor.Parent = workspace
	
	local ran = math.random(-100, 100)
	local ran2 = math.random(-100, 100)
	meteor.Position = Vector3.new(ran, 182, ran2)
	
	local part = meteor
	local Landed = false

	local function onTouched(otherPart)

		if otherPart.Name == "Baseplate" then
			
			meteor.AlignPosition:Destroy()
			
			part.Anchored = true

			local currentX = part.Position.X
			local currentZ = part.Position.Z		
			part.Position = Vector3.new(currentX, 0, currentZ)

			game.ReplicatedStorage.RemoteEvent:FireAllClients()

			local ring = game.ReplicatedStorage.Ring:Clone()
			currentRing = ring
			ring.Position = Vector3.new(currentX, ring.Position.Y, currentZ)
			ring.Parent = workspace

			local blast = game.ReplicatedStorage.blast:clone()
			currentBlast = blast
			blast.Parent = workspace
			blast.Position = Vector3.new(currentX, 0, currentZ)

			spawn(function()
				--blast script
				local chances = math.random(200, 250)

				local goal = {}
				goal.Size = Vector3.new(chances, chances, chances)

				local tweenInfo = TweenInfo.new(1., Enum.EasingStyle.Sine)

				local tween = TweenService:Create(currentBlast, tweenInfo, goal)

				tween:Play()
				tween.Completed:Connect(function()
					repeat
						currentBlast.Transparency += 0.1
						currentBlast.Size -= Vector3.new(1,1,1)
						wait(1)
					until currentBlast.Transparency == 1
					currentBlast:Destroy()
				end)
				
			end)
			
			spawn(function()
				--ring script

				local chances = math.random(800, 1000)

				local goal = {}
				goal.Size = Vector3.new(chances, chances, chances)
				goal.Transparency = 1

				local tweenInfo = TweenInfo.new(2, Enum.EasingStyle.Sine)

				local tween = TweenService:Create(currentRing, tweenInfo, goal)

				tween:Play()
				tween.Completed:Connect(function()
					currentRing:Destroy()
				end)
				
			end)
			
			Landed = true

			for _, player in pairs(game.Players:GetPlayers()) do
				local distance = (part.Position - player.Character.HumanoidRootPart.Position).magnitude
				if distance <= 15 then
					player.Character.Humanoid.Health = 0
				end
			end
			
			
		end
	end
	
	spawn(function()
		
		while Landed == false do
			local x = math.random(0,50)
			local y = math.random(0,50)
			local z = math.random(0,50)

			wait()

			meteor.Orientation += Vector3.new(x, y, z)
		end
		
	end)

	part.Touched:Connect(onTouched)
	
end)
2 Likes
#2

There are some repetitions/redundancies and tiny optimizations you can make to improve performance:

Avoid redundant indexing. Store commonly accessed instances as variables (known as aliasing) and reuse the said variables instead. Not only will it make coding easier (less words typed out), it will also make a small performance difference.

Parenting an instance should always be the last thing to do following its initialization.

task.spawn() is usually preferred over the original spawn() function. Read more here: Task Library - Now Available!

You do not need to create a new TweenInfo for each function call. Instead, create only one at the top of the script and reuse the variable.

You especially do not need to create a new vector for each translation. Once again, you can store & reuse; but in your case, you can use one of the vector constants stored within the Vector3 global itself: currentBlast.Size -= Vector3.one.

Using ipairs is faster than pairs when iterating through arrays.

This looks like it could error; the player’s character isn’t guaranteed to be present, i.e. the player is dead.

Directly setting the health of players is discouraged; please read: Humanoid:TakeDamage

#3

Yes, it crashes my game or causes a lot of lag without it
Thank you for all the other tips, some I skipped due to being lazy (too lazy to make variables)

#4

Use game:GetService() for your services and all the other things @Prototrode suggested.

#5

My bad, I didn’t see that it was inside a loop :sweat_smile: