Can I optimize or clean this code?

[Bankrovers]

Hi, I just created a script that spawns airdrops. I now have no idea how to make this script clearer and better optimized.

Script in ServerScriptService:

local debris: Debris = game:GetService("Debris")
local players: Players = game:GetService("Players")
local serverStorage: ServerStorage = game:GetService("ServerStorage")
local tweenService: TweenService = game:GetService("TweenService")

local airdropWeapons = serverStorage:FindFirstChild("AirdropWeapons"):GetChildren()

local tweenInfo: TweenInfo = TweenInfo.new(15, Enum.EasingStyle.Quad, Enum.EasingDirection.Out)

local function giveWeapon(player: Player)
	local backpack: Backpack = player:FindFirstChildOfClass("Backpack") :: Backpack
	
	local randomWeapon: Tool = airdropWeapons[math.random(1, #airdropWeapons)]:Clone()
	if backpack:FindFirstChild(tostring(randomWeapon)) then randomWeapon:Destroy() giveWeapon(player) return end
	
	randomWeapon.Parent = backpack
end

local function spawnAirdrop()
	local airdropRandomPosition: Vector3 = Vector3.new(math.random(-100, 100), 100, math.random(-100, 100))
	
	local rayOrigin: Vector3 = airdropRandomPosition
	local rayDestination: Vector3 = airdropRandomPosition - Vector3.new(0, 200, 0)
	
	local raycastParams: RaycastParams = RaycastParams.new()
	raycastParams.RespectCanCollide = true
	
	local rayDirection: Vector3 = (rayDestination - rayOrigin)
	
	local raycastResult: RaycastResult = workspace:Raycast(rayOrigin, rayDirection)
	if not raycastResult then spawnAirdrop() return end
	
	if raycastResult.Position.Y > 0 then spawnAirdrop() return end
	if tostring(raycastResult.Instance) == "Water" then spawnAirdrop() return end
	
	local airdrop: Model = script.Airdrop:Clone()
	airdrop:PivotTo(CFrame.new(airdropRandomPosition) * CFrame.Angles(0, 0, math.rad(90)))
	airdrop.Parent = workspace
	
	local primaryPart: BasePart = airdrop.PrimaryPart :: BasePart
	
	local function createTween()
		local tween:Tween = tweenService:Create(primaryPart, tweenInfo, {CFrame = CFrame.new(raycastResult.Position + Vector3.new(0, (primaryPart.Size.Y / 2) + 4.5, 0)) * CFrame.Angles(0, 0, math.rad(90))})
		tween:Play()

		tween.Completed:Wait()
		
		local parachute: Model = airdrop:FindFirstChild("Parachute") :: Model
		parachute:Destroy()
		
		local crate: Model = airdrop:FindFirstChild("Crate") :: Model
		local cratePrimaryPart: BasePart = crate.PrimaryPart :: BasePart

		local proximityPrompt: ProximityPrompt = Instance.new("ProximityPrompt")
		proximityPrompt.ActionText = "Open"
		proximityPrompt.HoldDuration = 5
		proximityPrompt.ObjectText = "Crate"
		proximityPrompt.RequiresLineOfSight = false
		proximityPrompt.Parent = cratePrimaryPart
		
		proximityPrompt.Triggered:Connect(function(playerWhoTriggered: Player)
			airdrop:Destroy()
			
			giveWeapon(playerWhoTriggered)
		end)
		
		debris:AddItem(airdrop, 60)
	end
	
	local createTweenCoroutine = coroutine.create(createTween)
	coroutine.resume(createTweenCoroutine)
end

while true do
	spawnAirdrop()
	
	local playersAmount: number = #players:GetPlayers()

	local minWaitTime: number = 30
	local maxWaitTime: number = 60	

	for index: number = 1, playersAmount do
		minWaitTime -= 3
		maxWaitTime -= 6
	end
	
	task.wait(math.random(minWaitTime, maxWaitTime))
end

[Bearturnedhuman (BearPlusPlus)]

The first thing I notice is your type annotations. While it’s a very powerful feature, it is possible to overuse it.

-- Index is never used, why do we need to know it's a number?
for index = 1, playersAmount do -- I removed :number to make it easier on the eyes
	minWaitTime -= 3
	maxWaitTime -= 6
end

Personally I’d take it a step further and replace index with i (abbreviations and single characters are bad practice but I always use i for loop counters, and it’s pretty standard)

for i = 1, playersAmount do
	minWaitTime -= 3
	maxWaitTime -= 6
end

A few other things you can do for optimization and clarity:

-- Organize your code with comments! Services, Fucntions, Connections, etc
-- Services
local Players = game:GetService("Players")

-- Config
-- I added Seconds to the end of timer variables so we know what unit we're using
local minPlayerDelaySeconds = 3 -- How many seconds should be removed per player (minAirdropDelay)
local maxPlayerDelaySeconds = 6 -- How many seconds should be remover per player (maxAirdropDelay)

local minAirdropDelaySeconds = 30
local maxAirdropDelaySeconds = 60

-- Functions
-- Instead of doing the math everytime an airdrop spawns, modify the wait times when players join or leave
local function OnPlayerAdded()
	minAirdropDelaySeconds -= minPlayerDelaySeconds
	maxAirdropDelaySeconds -= maxPlayerDelaySeconds
end

local function OnPlayerRemoving()
	minAirdropDelaySeconds += minPlayerDelaySeconds
	maxAirdropDelaySeconds += maxPlayerDelaySeconds
end

-- Connections
Players.PlayerAdded:Connect(OnPlayerAdded)
Players.PlayerRemoving:Connect(OnPlayerRemoving)

As for your functions (specifically spawnAirdrop) I would use ProximityPromptService and its .PromptTriggered event instead of the ProximityPrompt’s .Triggered event (that one is more preference and scalability, though)

And why do we need to know that airdrop.PrimaryPart is a BasePart if we already know primaryPart is a BasePart?

local primaryPart: BasePart = airdrop.PrimaryPart :: BasePart -- :(

local primaryPart: BasePart = airdrop.PrimaryPart  -- :D

You want your code to be readable but not overwhelming for future you (and other developers)

Lastly, I’d advise against this:

if backpack:FindFirstChild(tostring(randomWeapon)) then randomWeapon:Destroy() giveWeapon(player) return end

but instead multiple lines, like:

if backpack:FindFirstChild(tostring(randomWeapon)) then
	randomWeapon:Destroy()
	giveWeapon(player)
	return
end

It’s much more readable that way! I hope this helped!

[0_Xism (XISM)]

Tween the Part on the client (very smooth) and after its finished on the client move it in the server to where the tween ends (aka where u want it to be)

[0_Xism (XISM)]

Ik you asked to optimize it but it makes it look smoother in the client

[Bankrovers]

Just did everything you said here. It helped a lot!

I’m defining the PrimaryPart as a BasePart twice because:

Not looking for any orange lines in the code, even though it works just fine!