Music script improvements

Help and Feedback Code Review
1

My music script is already working. But I just think it can get some improvements. Here’s the script.

local sound = script.Parent.Ambiences.Ambience
local rs = game:GetService("RunService")
local changeMusicEvent = game.ReplicatedStorage.Events.MusicChange
local changeModulationEvent = game.ReplicatedStorage.Events.ModulationChange
local prevTrack = nil
local modulations = {
	[1] = {
		Modulation = "FM"
	},
	[2] = {
		Modulation = "AM"
	},
	[3] = {
		Modulation = "Legacy"
	},
	[4] = {
		Modulation = "NonCopy"
	}
}
local modulationOrder = 1
local selectedModulation = modulations[modulationOrder].Modulation
local music = {
	["FM"] = {
		{
			Name = "Time To Relax",
			ID = 9044702906,
			Station = 110.3
		},
		{
			Name = "Busy Vibes",
			ID = 1836393138,
			Station = 76.2
		},
		{
			Name = "Soft Sounds",
			ID = 1840384233,
			Station = 44.5
		},
		{
			Name = "Lo Fi Bay",
			ID = 9040313420,
			Station = 146.8
		},
		{
			Name = "Crystal Forest",
			ID = 1840201434,
			Station = 123.7
		},
		{
			Name = "That Summer (B)",
			ID = 1840118707,
			Station = 93.4
		},
		{
			Name = "Happy-Go-Lively",
			ID = 1841476350,
			Station = 17.3
		},
		{
			Name = "Hang Loose",
			ID = 1845369962,
			Station = 52.8
		},
		{
			Name = "Natural Blonde",
			ID = 1842279786,
			Station = 23.6
		},
		{
			Name = "A Dream Of Bossa Nova",
			ID = 9043921564,
			Station = 93.4
		},
		{
			Name = "Chill Dream (Ol)",
			ID = 1840732704,
			Station = 74.7
		},
		{
			Name = "The Nice Things",
			ID = 1840384241,
			Station = 101.3
		},
		{
			Name = "Relaxing St Tropez",
			ID = 1837198630,
			Station = 67.1
		},
		{
			Name = "Tender Chillstep",
			ID = 1836098504,
			Station = 69.4
		},
		{
			Name = "Irish Love Song",
			ID = 1838878344,
			Station = 50.2
		},
		{
			Name = "Violet Clouds",
			ID = 9046864489,
			Station = 100.5
		},
	},
	["AM"] = {
		{
			Name = "Poolside",
			ID = 9046863253,
			Station = 148.1
		},
		{
			Name = "No Smoking",
			ID = 9047105533,
			Station = 49.7
		},
		{
			Name = "City Lights",
			ID = 9046863579,
			Station = 24.9
		},
		{
			Name = "Sunday In Bed",
			ID = 9047104336,
			Station = 83.7
		},
		{
			Name = "Beach Cushions",
			ID = 9047104411,
			Station = 36.1
		},
		{
			Name = "Moving Round The Block",
			ID = 9047105108,
			Station = 18.1
		},
		{
			Name = "Mellow Mind (Bed Version)",
			ID = 9046863235,
			Station = 133.6
		},
		{
			Name = "Home Town Easy",
			ID = 9047104571,
			Station = 139.5
		},
		{
			Name = "Light Dreamer",
			ID = 9047105702,
			Station = 27.5
		},
		{
			Name = "On The Verge",
			ID = 9047105584,
			Station = 102.5
		},
		{
			Name = "Decompression",
			ID = 9047104650,
			Station = 36.4
		},
	},
	["Legacy"] = {
		{
			ID = 9047104411,
			Station = 67.4
		},
		{
			ID = 1845764031,
			Station = 137.3
		},
		{
			ID = 9043854439,
			Station = 140.8
		},
		{
			ID = 9047104650,
			Station = 120.2
		},
		{
			ID = 9047104336,
			Station = 15.6
		},
		{
			ID = 1846271108,
			Station = 37.5
		},
		{
			ID = 108807600670194,
			Station = 105.8
		}
	},
	["NonCopy"] = {
		{
			ID = 80681322169773,
			Station = 141.3
		},
		{
			ID = 74431803670437,
			Station = 67.67
		},
		{
			ID = 101743382410732,
			Station = 105.6
		},
		{
			Name = "Forsaken - A GRAVE SOUL",
			ID = 123827238081285,
			Station = 22.5
		},
		{
			ID = 135105170344371,
			Station = 89.2
		}
	}
}

local function getRandomTrack()
	local track = music[selectedModulation][math.random(1, #music[selectedModulation])]
	return track
end

local function pSound()
	local randomTrack = getRandomTrack()
	local MarketplaceService = game:GetService("MarketplaceService")
	local TrackName = randomTrack.Name
	if prevTrack == randomTrack then
		repeat
			randomTrack = getRandomTrack()
		until prevTrack ~= randomTrack
	end
	if not TrackName then
		TrackName = MarketplaceService:GetProductInfo(randomTrack.ID).Name
	end
	script.Parent.Modulation.Value = selectedModulation
	script.Parent.Station.Value = randomTrack.Station
	sound.SoundId = "rbxassetid://"..randomTrack.ID
	prevTrack = randomTrack
	sound.TimePosition = 0
	sound:Play()
	script.Parent.Creator.Value = MarketplaceService:GetProductInfo(randomTrack.ID).Creator.Name
	script.Parent.SongName.Value = TrackName
	sound.Ended:Wait()
	pSound()
end

local function pSoundNoTimePos()
	local randomTrack = getRandomTrack()
	local MarketplaceService = game:GetService("MarketplaceService")
	local TrackName = randomTrack[selectedModulation].Name
	if prevTrack == randomTrack then
		repeat
			randomTrack = getRandomTrack()
		until prevTrack ~= randomTrack
	end
	if not TrackName then
		TrackName = MarketplaceService:GetProductInfo(randomTrack.ID).Name
	end
	script.Parent.Modulation.Value = selectedModulation
	script.Parent.Station.Value = randomTrack.Station
	sound.SoundId = "rbxassetid://"..randomTrack.ID
	prevTrack = randomTrack
	script.Parent.Creator.Value = MarketplaceService:GetProductInfo(randomTrack.ID).Creator.Name
	script.Parent.SongName.Value = TrackName
	sound.Ended:Wait()
	pSound()
end

local function pSoundRandomTimePos()
	local randomTrack = getRandomTrack()
	local MarketplaceService = game:GetService("MarketplaceService")
	local TrackName = randomTrack.Name
	if prevTrack == randomTrack then
		repeat
			randomTrack = getRandomTrack()
		until prevTrack ~= randomTrack
	end
	if not TrackName then
		TrackName = MarketplaceService:GetProductInfo(randomTrack.ID).Name
	end
	script.Parent.Modulation.Value = selectedModulation
	script.Parent.Station.Value = randomTrack.Station
	sound.SoundId = "rbxassetid://"..randomTrack.ID
	prevTrack = randomTrack
	if not sound.IsLoaded then
		sound.Loaded:Wait()
	end
	local random = math.random(5, sound.TimeLength - 5)
	print(random)
	sound.TimePosition = random
	script.Parent.Creator.Value = MarketplaceService:GetProductInfo(randomTrack.ID).Creator.Name
	script.Parent.SongName.Value = TrackName
	sound.Ended:Wait()
	pSound()
end

local function changeModulationByOrder()
	modulationOrder += 1
	if modulationOrder > #modulations then
		modulationOrder = 1
	end
	selectedModulation = modulations[modulationOrder].Modulation
	pSoundRandomTimePos()
end

task.spawn(pSound)

changeMusicEvent.OnServerEvent:Connect(function(player, Music, Radio, Debounce)
	if Music.Value == script.Parent.Ambiences.Ambience then
		changeMusicEvent:FireAllClients()
		task.wait(1.9)
		pSoundRandomTimePos()
	end
end)

changeModulationEvent.OnServerEvent:Connect(function(player, Music, Radio)
	if Music.Value == script.Parent.Ambiences.Ambience then
		changeModulationByOrder()
	end
end)

And here’s the playtest.


Radio script:

local CollectionService = game:GetService("CollectionService")
local Radios = CollectionService:GetTagged("Radio")
local TweenService = game:GetService("TweenService")
local RunService = game:GetService("RunService")
local ChangeMusicEvent = game.ReplicatedStorage.Events.MusicChange
local ChangeModulationEvent = game.ReplicatedStorage.Events.ModulationChange

for i,Radio in pairs(Radios) do
	local PrevImage = Radio.Screen.SurfaceGui.Frame.ImageLabel.Image
	local Debounce = false
	local ClickDetector = Radio.Screen:WaitForChild("ClickDetector") :: ClickDetector
	ClickDetector.MouseClick:Connect(function()
		if Debounce == false then
			Debounce = true
			ChangeMusicEvent:FireServer(Radio.Music, Radio, Debounce)
		end
	end)
	ChangeMusicEvent.OnClientEvent:Connect(function()
		local PrevImage = Radio.Screen.SurfaceGui.Frame.ImageLabel.Image
		Radio.Screen.Static.Volume = 1
		Radio.Screen.SurfaceGui.Frame.ImageLabel.Image = "rbxassetid://17687447043"
		task.wait(2)
		Radio.Screen.SurfaceGui.Frame.ImageLabel.Image = PrevImage
		Radio.Screen.Static.Volume = 0
		Debounce = false
	end)
	Radio.ModulationButton.ClickDetector.MouseClick:Connect(function()
		Radio.Screen.Switch:Play()
		ChangeModulationEvent:FireServer(Radio.Music, Radio)
	end)
end

game["Run Service"].RenderStepped:Connect(function()
	for i,Radio in pairs(Radios) do
		local Music = Radio.Music.Value
		Radio.Screen.SurfaceGui.Frame.Creator.Text = Music.Parent.Parent.Creator.Value
		Radio.Screen.SurfaceGui.Frame.SongName.Text = Music.Parent.Parent.SongName.Value
		Radio.Screen.SurfaceGui.Frame.ImageLabel.Rotation += 1
		Radio.Screen.SurfaceGui.Frame.Station.Text = Music.Parent.Parent.Station.Value.. " ".. Music.Parent.Parent.Modulation.Value
		TweenService:Create(Radio.Screen.SurfaceGui.Frame.ImageLabel, TweenInfo.new(0.25, Enum.EasingStyle.Cubic, Enum.EasingDirection.Out), {Size = UDim2.new(0,Music.PlaybackLoudness*0.7 + 100,0,Music.PlaybackLoudness*0.7 + 100)}):Play()
	end
end)
1 Like
2

I would say you should definitely not be using RenderStepped here. Can that function be consolidated into ChangeMusicEvent.OnClientEvent, such that the SurfaceGuis update when the client gets a response for a new song?

Otherwise most of my feedback would be nitpicks.

3

I like to keep the settings and stats of things in a module just so that I don’t have to get overwhelmed by how many lines there are its a very small thing but yeah.

4

Alright. Now my script looks like this.

local sound = script.Parent.Ambiences.Ambience
local rs = game:GetService("RunService")
local changeMusicEvent = game.ReplicatedStorage.Events.MusicChange
local changeModulationEvent = game.ReplicatedStorage.Events.ModulationChange
local prevTrack = nil
local modulations = require(script.Modulations)
local modulationOrder = 1
local selectedModulation = modulations[modulationOrder].Modulation
local music = require(script.Music)

local function getRandomTrack()
	local track = music[selectedModulation][math.random(1, #music[selectedModulation])]
	return track
end

local function pSound(soundType)
	local randomTrack = getRandomTrack()
	local MarketplaceService = game:GetService("MarketplaceService")
	local TrackName = randomTrack.Name
	if prevTrack == randomTrack then
		repeat
			randomTrack = getRandomTrack()
			TrackName = randomTrack.Name
		until prevTrack ~= randomTrack
	end
	if not TrackName then
		TrackName = MarketplaceService:GetProductInfo(randomTrack.ID).Name
	end
	sound.SoundId = "rbxassetid://"..randomTrack.ID
	prevTrack = randomTrack
	if not sound.IsLoaded then
		sound.Loaded:Wait()
	end
	if soundType == "reset" then
		sound.TimePosition = 0
	elseif soundType == "randomTimePos" then
		sound.TimePosition = math.random(5, sound.TimeLength - 5)
	else
		warn("Didn't set SoundType. Not setting TimePos.")
	end
	script.Parent.Modulation.Value = selectedModulation
	script.Parent.Station.Value = randomTrack.Station
	script.Parent.Creator.Value = MarketplaceService:GetProductInfo(randomTrack.ID).Creator.Name
	script.Parent.SongName.Value = TrackName
	sound:Play()
end

sound.Ended:Connect(function()
	pSound("reset")
end)

local function changeModulationByOrder()
	modulationOrder += 1
	if modulationOrder > #modulations then
		modulationOrder = 1
	end
	selectedModulation = modulations[modulationOrder].Modulation
	pSound("randomTimePos")
end

task.spawn(pSound, "reset")

changeMusicEvent.OnServerEvent:Connect(function(player, Music, Radio, Debounce)
	if Music.Value == script.Parent.Ambiences.Ambience then
		changeMusicEvent:FireAllClients()
		task.wait(1.9)
		pSound("randomTimePos")
	end
end)

changeModulationEvent.OnServerEvent:Connect(function(player, Music, Radio)
	if Music.Value == script.Parent.Ambiences.Ambience then
		changeModulationByOrder()
	end
end)
5

nice it looks simple and not overwhelming other than that It looks nice and clean I feel like adjusting format of the code after this would be a little extra kind of unless you add more stuff judging it by a quick glance.

6

Very small styling thing, but I personally prefer organizing the variables at the top into sections depending on what they do.

Another thing, you don’t need to call game:GetService("MarketplaceService") every time pSound is called; just declare it at the top.

Another small style thing: keeping all variables in the same style makes it just look cleaner, to me at least. Here you use both camelCase and PascalCase.

Finally, just to be safe, you will probably want to check all the values that are sent in the RemoteEvents to make sure they are valid. Clients might send bad data which could just throw errors.

7

I’d put the music variable in a separate module to prevent clutter.

8

You can consolidate this logic into a single loop and remove redundancy. Note how you’re setting TrackName immediately after changing randomTrack, but this means you’re wasting CPU time (and more importantly, lines of code) on storing a value that might not matter.

The answer 
	local randomTrack
	repeat
		randomTrack = getRandomTrack()
	until randomTrack ~= prevTrack
	local TrackName = randomTrack.Name

Regardless of how you approach that loop, you should know it’s pretty dangerous to simply loop infinitely based on random chance like this. It may be extremely unlikely, but we can approach this a lot smarter.

Consider that, mathematically, the only time when y, the random number, is equal to itself, is if the difference between y1 and y2 is 0. In other words, for y1 + x = y2, x must be 0. Therefore, all we need to do is make sure x is not 0 and yet also that y + x is within the bounds [1, n] where n is #music[selectedModulation].

If you’re catching my drift here, you need to use modulo and add math.random(1, n - 1) to the previous random track number. You want n - 1 because otherwise you have x = n, (y + x) % n = (y + n) % n = y.

This will give you a perfectly random number every time without any loops or branches.

The answer 
local function getRandomTrack()
	local tracks = music[selectedModulation]
	prevTrack =
		((prevTrack -- The previous value
			+ math.random(1, #tracks - 1) -- plus a number between [1, n - 1]
			- 1) --Subtract 1 here to change our range to [0, n - 2] for mod
 		% #tracks) -- mod that sum by n
		+ 1 -- Add 1 to normalize back to range [1, n - 1]
	return tracks[prevTrack]
end
1 Like
9

When I tried the function you made, it gave me this error.

Screenshot 2025-09-05 155837
Screenshot 2025-09-05 1558371295×108 7.85 KB

This is now my current script.

local MarketplaceService = game:GetService("MarketplaceService")
local zone = script.Parent
local sound = zone.Ambiences.Ambience
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local changeMusicEvent = ReplicatedStorage.Events.MusicChange
local changeModulationEvent = ReplicatedStorage.Events.ModulationChange
local prevTrack = nil
local modulations = require(script.Modulations)
local modulationOrder = 1
local selectedModulation = modulations[modulationOrder].Modulation
local music = require(script.Music)

local function getRandomTrack()
	local track = music[selectedModulation][math.random(1, #music[selectedModulation])]
	return track
end

local function pSound(soundType)
	local randomTrack = getRandomTrack()
	local TrackName = randomTrack.Name
	if prevTrack == randomTrack then
		repeat
			randomTrack = getRandomTrack()
			TrackName = randomTrack.Name
		until prevTrack ~= randomTrack
	end
	if not TrackName then
		TrackName = MarketplaceService:GetProductInfo(randomTrack.ID).Name
	end
	sound.SoundId = "rbxassetid://"..randomTrack.ID
	prevTrack = randomTrack
	if not sound.IsLoaded then
		sound.Loaded:Wait()
	end
	if soundType == "reset" then
		sound.TimePosition = 0
	elseif soundType == "randomTimePos" then
		sound.TimePosition = math.random(5, sound.TimeLength - 5)
	else
		warn("Didn't set SoundType. Not setting TimePos.")
	end
	zone.Modulation.Value = selectedModulation
	zone.Station.Value = randomTrack.Station
	zone.Creator.Value = MarketplaceService:GetProductInfo(randomTrack.ID).Creator.Name
	zone.SongName.Value = TrackName
	sound:Play()
end

sound.Ended:Connect(function()
	pSound("reset")
end)

local function changeModulationByOrder()
	modulationOrder += 1
	if modulationOrder > #modulations then
		modulationOrder = 1
	end
	selectedModulation = modulations[modulationOrder].Modulation
	pSound("randomTimePos")
end

task.spawn(pSound, "reset")

changeMusicEvent.OnServerEvent:Connect(function(player, Music, Radio, Debounce)
	if Music.Value == zone.Ambiences.Ambience then
		changeMusicEvent:FireAllClients()
		task.wait(1.9)
		pSound("randomTimePos")
	end
end)

changeModulationEvent.OnServerEvent:Connect(function(player, Music, Radio)
	if Music.Value == zone.Ambiences.Ambience then
		changeModulationEvent:FireAllClients()
		changeModulationByOrder()
	end
end)
10

… And you couldn’t figure out how to fix that by yourself?

11

Well you said perfectly so I thought it had no bugs.