How do I improve this script?

Hello,

(Game Link = (Bombalooza - Roblox))

I’m trying to make a script for a game of mine, the goal of the game is to break the most bricks than everyone else. I’ve already got a script that works for the game and how the game works, but the issue is I want to make the script look more clean than it is, and I’m looking on here to find anyone who can help me improve it.

The scripts are supposed to make a bomb that explodes, and when the explosion radius touches a part, that part will break all of its welds and then later be deleted.

Local Script

local MOUSE_ICON = 'rbxasset://textures/GunCursor.png' -- Changes the mouse to the GunCursor Icon.
local RELOADING_ICON = 'rbxasset://textures/GunWaitCursor.png'-- Changes the mouse icon to the Gun Wait Curso Icon.

local Tool = script.Parent
local cooldown = false

local Mouse = nil

local function UpdateIcon()
	if Mouse then
		Mouse.Icon = Tool.Enabled and MOUSE_ICON or RELOADING_ICON
	end
end

local function OnEquipped(mouse)
	Mouse = mouse
	UpdateIcon()
end

local function OnChanged(property)
	if property == 'Enabled' then
		UpdateIcon()
	end
end

Tool.Activated:Connect(function()
	if not cooldown then cooldown = true
		script.Parent.ActivateBomb:FireServer(game.Players.LocalPlayer) -- Starts the bomb creation process
		
		task.wait(.25)
		
		cooldown = false -- Resets cooldown
	end
end)

Tool.Equipped:connect(OnEquipped)
Tool.Changed:connect(OnChanged)

Server Script

local Tool = script.Parent
local Bomb = Tool:WaitForChild("Handle")

function playTick() -- Plays a ticking sounds
	local ticksound = Instance.new("Sound")
	ticksound.SoundId = "rbxasset://sounds\\clickfast.wav"
	ticksound.Parent = script.Parent

	ticksound:Play()
end

function explode(bomb, player)
	for i = 0, 3 do task.wait(1) -- Plays the ticking sound 3 times before it explodes
		playTick()
	end

	task.wait(1)

	local explosionSound = Instance.new("Sound")
	explosionSound.SoundId = "rbxasset://sounds\\Rocket shot.wav"
	explosionSound.Parent = script.Parent

	bomb.Anchored = true -- Anchores the bomb so that it doesn't go elsewhere due to the explosion.

	local explosionClone = bomb.Explosion:Clone() -- Makes a clone of the explosion particles and plays it.
	explosionClone.Parent = workspace
	explosionClone.Position = bomb.Position
	explosionClone.Parent = bomb
	explosionClone.Enabled = true

	explosionSound:Play() -- Plays the explosion sound

	bomb.Transparency = 1

	local region3 = Region3.new(bomb.Position - Vector3.new(5, 1, 5), bomb.Position + Vector3.new(5, 10, 5)) -- Creates a region3 that destroys any parts in the explosions radius.

	local region3Visual = Instance.new("Part") -- The visualized part for the region3.
	region3Visual.CanCollide = false
	region3Visual.Name = "blastRadiousVisual"
	region3Visual.Size = Vector3.new(math.abs(region3.Size.X), math.abs(region3.Size.Y), math.abs(region3.Size.X))
	region3Visual.CFrame = region3.CFrame
	region3Visual.Anchored = true
	region3Visual.Transparency = .5
	region3Visual.Parent = bomb
	
	local explosion = Instance.new("Explosion", bomb) -- The explosion instance
	explosion.Position = bomb.Position
	explosion.BlastRadius = 0
	explosion.DestroyJointRadiusPercent = 0
	
	explosion.BlastRadius = region3Visual.Size.X / 2 -- The math for the explosion so that it only has its blast impact affected on parts in the region3's radius.

	local overlapParams = OverlapParams.new()
	overlapParams.FilterDescendantsInstances = {Tool, Tool.Handle, region3Visual}

	local parts = workspace:GetPartBoundsInBox(region3.CFrame, region3.Size, overlapParams)
	
	explosion.Hit:Connect(function(part)
		if part.Name == "HumanoidRootPart" then
			local possiblePlayer = part.Parent
			
			if game.Players:GetPlayerFromCharacter(possiblePlayer).Team ~= player.Team then
				possiblePlayer:FindFirstChildOfClass("Humanoid").Health -= 50 -- Damages any players on the opposite team as the player who detonated the bomb.
				
				if possiblePlayer:FindFirstChildOfClass("Humanoid").Health == 0 then -- Plays the local player's kill sound after they kill the player.
					local killSound = player.killSound:Clone()
					killSound.Parent = possiblePlayer.HumanoidRootPart
					killSound:Play()
				end
			end
		end
	end)
	
	for i, v in pairs(parts) do
		if v.Name == "Brick" then
			v.Anchored = false
			
			v.CanTouch = false
			
			v.BrickColor = player.TeamColor
			
			game:GetService("Debris"):AddItem(v, 3)
		end
		
		if v.Name == "GameBrick" then v.Name = "TaggedGameBrick" -- TaggedGameBricks are GameBricks in the game that have been effected by the bomb, while regular Bricks are just bricks you can destroy by default.
			v.Anchored = false
			
			v.CanTouch = false
			
			player.leaderstats.Bricks.Value += 1 -- The total bricks the player has broken throughout the round.
			player.leaderstats["T. Bricks"].Value += 1 -- The total bricks that the player has destroyed throughout there playtime
			player.leaderstats.Studs.Value += .1 -- Increases the players currency (Studs) by 0.1.
			v.BrickColor = player.TeamColor -- Changes the taggedBricks color to the local players team color to show that they have broken the part.
			
			game:GetService("Debris"):AddItem(v, 3) -- later deletes the part.
		end
	end

	game:GetService("Debris"):AddItem(bomb, 7) -- later deletes the bomb after 7 seconds.

	task.wait(2)

	explosionClone.Enabled = false -- Disables the explosion particles.
end

script.Parent.ActivateBomb.OnServerEvent:Connect(function(player) -- This is the function that makes the bomb go off, which connects the ActivateBomb Remote Event to this script after it gets activated by the local script.
	local bombClone = Bomb:Clone()
	bombClone.Parent = game.Workspace
	bombClone.CanCollide = true
	bombClone.CFrame = Bomb.CFrame * CFrame.new(0, 2, 2)

	explode(bombClone, player)
end)
1 Like

client.lua

  1. The Mouse instance is deprecated, and you should use UserInputService.MouseIcon to change the icon instead. This will also replace the need to value-hoist the Mouse.

  2. Use an if-expression instead of ternary-like operators as they are more predictable (see here).

  3. You can use the Instance/GetPropertyChangedSignal to filter out when only the ‘Enabled’ property changes. Infact, since there is no need to include extra logic, you could hook up the UpdateIcon function directly to it.

  4. You can hold ActivateBomb and LocalPlayer as constants instead of indexing for them every time.

  5. Just a few general style tips:

    A. Try to keep a consistent casing (Tool and cooldown look weird together)
    Double-Quotes (") are preferred more than Sigle-Quotes (') when writing strings.

    B. Always try to avoid lambda functions, especially when passing callback functions. The function in Activated could be its own OnActivated function.

    C. Keep the logic in an if-statement underneath the condition. At first glance, the cooldown = true looks like it’s part of the condition.

    D. Use the PascalCase variants of Roblox methods (Connect instead of connect).

server.lua

  1. The ‘ticksound’ and ‘explosionSound’ can be constants that you can just call Sound/Play on

  2. You can make a separate function that clones the explosion and parts.

  3. You should split operations inside a constructor to make it more readable.

local region3 = Region3.new(
	bomb.Position - Vector3.new(5, 1, 5),
	bomb.Position + Vector3.new(5, 10, 5)
)
  1. The OverlapsParams can be a constant and you could use OverlapParams/AddToFilter to add the region3Visual to it. (Not sure if this is activated yet since it isn’t autocompleted in Studio)

  2. The callback for Explosion.Hit can be changed to be a bit more defensive. I don’t know what really to put more here so here’s just a possible rendition.

if Part.Name == "HumanoidRootPart" then
	
	local Character = Part.Parent
	local Humanoid = Character:FindFirstChildWhichIsA("Humanoid")
	local Player = Players:GetPlayerFromCharacter(Character)
	
	-- I recommend changing 'player' in the explode parameters
	-- to 'User' as it is more descriptive of its role
	if Player and Player.Team ~= User.Team then
		-- You can also use Humanoid:TakeDamage
		-- if you want to respect Forcefields
		Humanoid.Health -= 50
		
		if Humanoid.Health == 0 then
			-- Play Sound
		end
	end
	
end
  1. Instead of Debris, there’s this trick I do that performs faster with task.delay. You don’t need to understand how it works, but essentially it’s like I’m calling Instance/Destroy after a few seconds.
task.delay(3, game.Destroy, Part)
  1. Again, just keep ActivateBomb a constant, store the OnServerEvent callback as a separate function, and stay consistent with your casing. You can also use workspace instead of game.Workspaceas they refer to the same thing.
2 Likes

What is a ternary operator / ternary-like operators?

What is a constant referred to as in programing? And also, what do you mean by indexing?

what is a “lambda” and callback function?

And sorry if I’m asking all of this instead of searching it up, some of these terms I’ve just never learned before, and when I searched up some of them, different programming languages popped up.


Server Script

I don’t think it has been activated yet because I’ve tried finding the method in studio and it never came up. And also, was there a post about this new method?

When you put “a bit more defensive”, are you referring to it being more defensive in the sense that its more durable to exploits?

And if you are, can you go a bit more in depth about why this would be the case?

Here’s a more edited version of both scripts, I do agree, your recommendations made them look more cleaner and easier to edit. If I ever need to look back and them, they look much less complicated then before.

client.lua

local MOUSE_ICON = "rbxasset://textures/GunCursor.png" -- Changes the mouse to the GunCursor Icon.
local RELOADING_ICON = "rbxasset://textures/GunWaitCursor.png" -- Changes the mouse icon to the Gun Wait Curso Icon.

local Player = game.Players.LocalPlayer

local Tool = script.Parent
local cooldown = false
local UserInputService = game:GetService("UserInputService")
local RemoteEvent = script.Parent:WaitForChild("ActivateBomb")

Tool.Equipped:Connect(function()
	UserInputService.MouseIcon = MOUSE_ICON
end)

Tool.Unequipped:Connect(function()
	UserInputService.MouseIcon = "rbxassetid://1"
end)

Tool.Activated:Connect(function()
	if not cooldown then 
		cooldown = true
		
		RemoteEvent:FireServer(Player)
			
		UserInputService.MouseIcon = RELOADING_ICON
		
		task.wait(.5)

		UserInputService.MouseIcon = MOUSE_ICON
				
		cooldown = false
	end
end)

server.lua

local tool = script.Parent
local bombHandle = tool:WaitForChild("Handle")
local ActivateBombRemote = script.Parent:WaitForChild("ActivateBomb")

local tickSound = Instance.new("Sound")
tickSound.SoundId = "rbxasset://sounds\\clickfast.wav"
tickSound.Parent = tool

local explosionSound = Instance.new("Sound")
explosionSound.SoundId = "rbxasset://sounds\\Rocket shot.wav"
explosionSound.Parent = tool

function explosionParticles(bomb)
	local explosionClone = bomb.Explosion:Clone()
	explosionClone.Parent = bomb
	explosionClone.Enabled = true
	
	return explosionClone
end

function explode(bomb, user)
	for i = 1, 3 do task.wait(1)
		tickSound:Play()
	end

	task.wait(1)

	bomb.Anchored = true -- Anchores the bomb so that it doesn't go elsewhere due to the explosion.
	
	local explosionParticle = explosionParticles(bomb)
	
	explosionSound:Play()

	bomb.Transparency = 1

	local region3 = Region3.new(
		bomb.Position - Vector3.new(5, 1, 5),
		bomb.Position + Vector3.new(5, 10, 5) -- Makes a square that is 5x5 long and wide, and 10 studs high (hence the 1 to 10)
	) -- Creates a region3 that destroys any parts in the explosions radius.

	local region3Visual = Instance.new("Part") -- The visualized part for the region3.
	region3Visual.Name = "blastRadiousVisual"
	region3Visual.CanCollide = false
	region3Visual.Size = Vector3.new(math.abs(region3.Size.X), math.abs(region3.Size.Y), math.abs(region3.Size.X))
	region3Visual.CFrame = region3.CFrame
	region3Visual.Anchored = true
	region3Visual.Transparency = .5
	region3Visual.Parent = bomb
	region3Visual.BrickColor = BrickColor.new("Persimmon")
	
	local explosion = Instance.new("Explosion", bomb) -- The explosion instance
	explosion.Position = bomb.Position
	explosion.BlastRadius = 0
	explosion.DestroyJointRadiusPercent = 0
	explosion.BlastPressure = 1000000
	
	explosion.BlastRadius = region3Visual.Size.X / 2 -- The math for the explosion so that it only has its blast impact affected on parts in the region3's radius.

	local overlapParams = OverlapParams.new()
	overlapParams.FilterDescendantsInstances = {tool, tool.Handle, region3Visual}

	local parts = workspace:GetPartBoundsInBox(region3.CFrame, region3.Size, overlapParams)
	
	explosion.Hit:Connect(function(Part)
		if Part.Name == "HumanoidRootPart" then
			local character = Part.Parent
			local humanoid = character:FindFirstChildWhichIsA("Humanoid")
			local player = game.Players:GetPlayerFromCharacter(character)
			
			if humanoid.Health ~= 0 then -- Doing this to avoid the kill sound playing after a player who is already dead gets damaged by a bomb again
				if player and player.Team ~= user.Team then
					humanoid:TakeDamage(25)

					if humanoid.Health == 0 then
						local killSound = user.killSound:Clone()
						killSound.Parent = player.HumanoidRootPart
						killSound:Play()
					end
				end
			end
		end
	end)
	
	for i, v in pairs(parts) do
		if v.Name == "Brick" then
			v.Anchored = false
			
			v.CanTouch = false
			
			v.BrickColor = user.TeamColor
			
			task.delay(3, game.Destroy, v)
		end
		
		if v.Name == "GameBrick" then v.Name = "TaggedGameBrick" -- TaggedGameBricks are GameBricks in the game that have been effected by the bomb, while regular Bricks are just bricks you can destroy by default.
			v.Anchored = false
			
			v.CanTouch = false
			
			user.leaderstats.Bricks.Value += 1 -- The total bricks the player has broken throughout the round.
			user.leaderstats["T. Bricks"].Value += 1 -- The total bricks that the player has destroyed throughout there playtime
			user.leaderstats.Studs.Value += .1 -- Increases the players currency (Studs) by 0.1.
			v.BrickColor = user.TeamColor -- Changes the taggedBricks color to the local players team color to show that they have broken the part.
			
			task.delay(3, game.Destroy, v) -- later deletes the part.
		end
	end

	task.delay(3, game.Destroy, bomb)

	task.wait(2)

	explosionParticle.Enabled = false -- Disables the explosion particles.
end

ActivateBombRemote.OnServerEvent:Connect(function(user) -- This is the function that makes the bomb go off, which connects the ActivateBomb Remote Event to this script after it gets activated by the local script.
	local bombClone = bombHandle:Clone()
	bombClone.Parent = game.Workspace
	bombClone.CanCollide = true
	bombClone.CFrame = bombHandle.CFrame * CFrame.new(0, 2, 2)

	explode(bombClone, user)
end)