Any tips on how to improve this weather code efficiently?

Help and Feedback Code Review
#1

I’m trying to improve this code, mainly by shorting it. It’s a raining script, I know many other developers like to use particles when creating rain for their games, however for me since I’m pretty new to scripting I thought I’d take the step further and create a harder script for learning purposes.

local RunService = game:GetService("RunService")

local RainSpawned = 0
local RainEnd = 300
local RainSound = game.Workspace.Rain

local Thunder = 0
local ThunderSound = game.Workspace.Thunder
local ThunderSound2 = game.Workspace.Thunder2

while true do
	RainSound:Play()
	for i = 1, RainEnd do 
		local RandomNumber = math.random(1, 40) 
		local RandomNumber2 = math.random(1, 60)
		local RandomNumber3 = math.random(1, 40)
		RainSpawned = RainSpawned + 1
		wait()
		local Rain = Instance.new("Part")
		local RainMesh = Instance.new("SpecialMesh")
		local Weld = Instance.new("WeldConstraint")
		Rain.Size = Vector3.new(0.05, 1, 0.05)
		Rain.Color = Color3.fromRGB(180, 210, 228)
		Rain.Anchored = false
		Rain.CanCollide = false
		Rain.Position = Vector3.new(RandomNumber, RandomNumber2, RandomNumber3)
		Rain.Parent = workspace.Droplets
		Weld.Parent = Rain
		RainMesh.MeshId = "rbxassetid://5171497068"
		RainMesh.Scale = Vector3.new(0.008, 0.028, 0.008)
		RainMesh.Parent = Rain
		
		if (math.random(1, 300)) == 10 then
			local Thunder = Instance.new("Part")
			local ThunderMesh = Instance.new("SpecialMesh")
			local Weld = Instance.new("WeldConstraint")
			local Light = Instance.new("PointLight")
			Thunder.Size = Vector3.new(1, 1, 1)
			Thunder.Color = Color3.fromRGB(255, 255, 0)
			Thunder.Anchored = true
			Thunder.CanCollide = false
			Thunder.Position = Vector3.new(RandomNumber, 36, RandomNumber3)
			Thunder.Name = "Thunder"
			Thunder.Parent = workspace	
			Weld.Parent = Thunder
			ThunderMesh.MeshId = "rbxassetid://5173801263"
			ThunderMesh.Scale = Vector3.new(1, 0.8, 1)
			ThunderMesh.Parent = Thunder
			Light.Brightness = 5.54
			Light.Color = Color3.fromRGB(255, 213, 3)
			Light.Range = 27
			Light.Parent = Thunder
			wait(0.1)
			Thunder:Destroy()
				
			if (math.random(1, 2)) == 1 then
				ThunderSound:Play()
			elseif (math.random(1, 2)) == 2 then
				ThunderSound2:Play()
			end
		end
	end
	RainSound:Stop()
	wait(math.random(1, 100))
end
#2

Looks like you are creating a lot of parts and meshes and then setting their properties. Could be done simpler with a function I think. Would take half the lines that you currently use for that.

1 Like
#3

You’re not using these variables: RunService, RainSpawned, Thunder (the one declared @ line 7). The weld you make does nothing because you don’t set Part0 or Part1.

Creating the droplet inside the loop makes it uncecessarily long and harder to read. I’d instead create a droplet “prefab” at the top of the script, and just clone that when a new one is needed. E.g.:

--At the outermost scope of the script
local Droplet = Instance.new("Part")
Droplet.Size = Vector3.new(0.05, 1, 0.05)
Droplet.Color = Color3.fromRGB(180, 210, 228)
Droplet.Anchored = false
Droplet.CanCollide = false

local DropletMesh = Instance.new("SpecialMesh")
DropletMesh.MeshId = "rbxassetid://5171497068"
DropletMesh.Scale = Vector3.new(0.008, 0.028, 0.008)
DropletMesh.Parent = Droplet

--In the loop
local Droplet = Droplet:Clone()
Droplet.Position = Vector3.new(RandomNumber, RandomNumber2, RandomNumber3)
Droplet.Parent = workspace.Droplets

Same with the thunder and lights.

Your way of naming variables when there’s a couple that are similar is weird. “Thing”, “Thing2”, “Thing3”. Why not “Thing1”? That seems more natural and readable to me.

You’re using math.random, which was replaced by a better Random class. You should probably use that instead. Your way of getting random numbers at the top of each loops is also weird.

I’d instead declare local r = Random.new() at the top of the script, and do e.g.

Rain.Position = Vector3.new(r:NextNumber(1, 40), r:NextNumber(1, 40), r:NextNumber(1, 40))

You’re using a couple of magic numbers (1, 40, 300). These should probably be declared at the top of the script instead, so it’s easier to see what they refer to and easier to configure your script to different situations.

Hope this helps :slight_smile: