How to make function keep looping until it returns something

I am trying to make a module script that spawns rocks on spawn locations. In the pickSpawnLocation() function at the top, I tried to make it run again if it didn’t find a spawn location, if it found one, it returned it. But when it found a spawn location that is occupied it doesn’t rerun the function until it found a spawn that a rock can spawn on.

local serverStorage = game:GetService("ServerStorage")
local minableObjects = serverStorage.MinableObjects

local Rock = {}

function pickSpawnLocation(rockType)
	local spawnLocation = workspace.RockSpawnLocations:GetChildren()[math.random(1, #workspace.RockSpawnLocations:GetChildren())]
	if spawnLocation:FindFirstChild(rockType) then -- I just made the rock that has spawned a child of a spawner cause that was the first thing I thought of to check if a spawn is occupied.
		pickSpawnLocation(rockType)
	else
		return spawnLocation -- A spawn location has been found.
	end
end

function Rock.Spawn(rockType, amount)
	if minableObjects:FindFirstChild(rockType) then --If the rock the script is trying to spawn exists
		for index = 1, amount do
			local newRockSpawnLocation = pickSpawnLocation(rockType)
			print(newRockSpawnLocation)
			if newRockSpawnLocation then
				local newRock = minableObjects:FindFirstChild(rockType):Clone()
				newRock.Parent = newRockSpawnLocation
				newRock.Position = newRockSpawnLocation.Position + Vector3.new(0,newRock.Size.Y/2,0)
			end
			task.wait()
		end
	else
		warn("The rock that tried to spawn does not exist.")
	end

end
	
return Rock
local returned
repeat returned = function() until returned

Replace function() with your function and that’s it

I tried implementing that in multiple different ways, all failed and caused same issue.
Here is my code now

local serverStorage = game:GetService("ServerStorage")
local minableObjects = serverStorage.MinableObjects

local Rock = {}

function pickSpawnLocation(rockType)
	local spawnLocation = workspace.RockSpawnLocations:GetChildren()[math.random(1, #workspace.RockSpawnLocations:GetChildren())]
	if spawnLocation:FindFirstChild(rockType) then -- I just made the rock that has spawned a child of a spawner cause that was the first thing I thought of to check if a spawn is occupied.
		local returned 
		repeat
			returned = pickSpawnLocation(rockType)
			task.wait()
			
		until returned
	else
		return spawnLocation -- A spawn location has been found.
	end
end

function Rock.Spawn(rockType, amount)
	if minableObjects:FindFirstChild(rockType) then --If the rock the script is trying to spawn exists
		for index = 1, amount do
			local newRockSpawnLocation = pickSpawnLocation(rockType)
			
			if newRockSpawnLocation then
				local newRock = minableObjects:FindFirstChild(rockType):Clone()
				newRock.Parent = newRockSpawnLocation
				newRock.Position = newRockSpawnLocation.Position + Vector3.new(0,newRock.Size.Y/2,0)
			else
				warn("Picked a bad spawner")
			end
			task.wait()
		end
	else
		warn("The rock that tried to spawn does not exist.")
	end

end
	
return Rock

I believe I got it to work, but there has to be a better CLEANER way to do it.

local Rock = {}

function pickSpawnLocation(rockType)
	local spawnLocation = workspace.RockSpawnLocations:GetChildren()[math.random(1, #workspace.RockSpawnLocations:GetChildren())]
	if spawnLocation:FindFirstChild(rockType) then -- I just made the rock that has spawned a child of a spawner cause that was the first thing I thought of to check if a spawn is occupied.
		--pickSpawnLocation(rockType)
	else
		return spawnLocation -- A spawn location has been found.
	end
end

function Rock.Spawn(rockType, amount)
	if minableObjects:FindFirstChild(rockType) then --If the rock the script is trying to spawn exists
		for index = 1, amount do
			local newRockSpawnLocation = pickSpawnLocation(rockType)
			if newRockSpawnLocation == nil then
				repeat newRockSpawnLocation =  pickSpawnLocation(rockType) until newRockSpawnLocation 
			end
			if newRockSpawnLocation then
				local newRock = minableObjects:FindFirstChild(rockType):Clone()
				newRock.Parent = newRockSpawnLocation
				newRock.Position = newRockSpawnLocation.Position + Vector3.new(0,newRock.Size.Y/2,0)
			else
				warn("Picked a bad spawner")
			end
			task.wait()
		end
	else
		warn("The rock that tried to spawn does not exist.")
	end

end
	
return Rock

1 Like

Cant you just put everything inside the pickSpawnLocation into a repeat until spawnLocation or something with spawn location being nil at the beginning of the the function like:

function pickSpawnLocation(rockType)
    local foundLocation = nil
    repeat
	local spawnLocation = workspace.RockSpawnLocations:GetChildren()[math.random(1, #workspace.RockSpawnLocations:GetChildren())]
	if spawnLocation:FindFirstChild(rockType) then -- I just made the rock that has spawned a child of a spawner cause that was the first thing I thought of to check if a spawn is occupied.
		pickSpawnLocation(rockType)
	else
		foundLocation = spawnLocation -- A spawn location has been found.
	end
    until foundLocation ~=nil
    return spawnLocation
end

I probably messed something up in that script but if I didn’t it should work

Well for one no, because “spawnLocation” is defined inside the repeat loop so it’s always nil, you should’ve returned foundLocation.

And the OP already has this setup for a very simple solution: Recursion.

Not the greatest solution, but it WILL work. unless of course you don’t find a spawn in I believe 30 tries… Then it’ll just error because of recursion depth.
Anyways I don’t know why I brought up recursion since making this Event-based is so much nicer on performance and less error prone.

Try this code:

local serverStorage = game:GetService("ServerStorage")
local minableObjects = serverStorage.MinableObjects

local CollectionService = game:GetService("CollectionService")
local SpawnTag = "ValidRockSpawn"

-- I'm going to assume that these are Ores, and that they simply get Deleted when they've been mined.
-- Meaning, we can use CollectionService to make this a lot simpler.

-- We're going to make this Event-based since it's faster and more maintainable.

local function WrapChildAdded(Spawn)
	-- We have to wrap it so we have reference to which spawn it is.
	return function(Child)
		-- Remove tag so it removes it from the list of CollectionService:GetTagged(SpawnTag) Instances.
		CollectionService:RemoveTag(newRockSpawnLocation, SpawnTag)
	end
end

local function WrapChildRemoved(Spawn)
	-- Again, we have to wrap it so we have reference to which spawn it is.
	return function(Child)
		-- Add tag back so it shows up in the list of CollectionService:GetTagged(SpawnTag) Instances.
		CollectionService:AddTag(newRockSpawnLocation, SpawnTag)
	end
end

-- Set up all Spawns as "Empty" by default.

for Index, Spawn in pairs(workspace.RockSpawnLocations:GetChildren()) do
	CollectionService:AddTag(Spawn, SpawnTag)
	Spawn.ChildAdded:Connect(WrapChildAdded(Spawn))
	Spawn.ChildRemoved:Connect(WrapChildRemoved(Spawn))
	-- Let's hook up the Event handlers.
end

local Rock = {}

function pickSpawnLocation(rockType)
	-- Now we can use CollectionService:GetTagged(SpawnTag) to get all the empty spawn nodes.
	local spawnLocation = CollectionService:GetTagged(SpawnTag)[math.random(1, #CollectionService:GetTagged(SpawnTag))]
	-- These shouldn't matter anymore but for safety's sake, we'll leave them in.
	if spawnLocation:FindFirstChild(rockType) then -- I just made the rock that has spawned a child of a spawner cause that was the first thing I thought of to check if a spawn is occupied.
		pickSpawnLocation(rockType) -- For reference, this didn't actually do anything but cause unnecessary tailcalls. You need to return the value of the call like you did in the else chunk of the if statement.
	else
		return spawnLocation -- A spawn location has been found. (Should always be the case now.)
	end
end

function Rock.Spawn(rockType, amount)
	if minableObjects:FindFirstChild(rockType) then --If the rock the script is trying to spawn exists
		for index = 1, amount do
			local newRockSpawnLocation = pickSpawnLocation(rockType)
			print(newRockSpawnLocation)
			if newRockSpawnLocation then
				local newRock = minableObjects:FindFirstChild(rockType):Clone()
				newRock.Parent = newRockSpawnLocation
				newRock.Position = newRockSpawnLocation.Position + Vector3.new(0,newRock.Size.Y/2,0)
			end
			task.wait()
		end
	else
		warn("The rock that tried to spawn does not exist.")
	end
end
	
return Rock

you can use something like this


local Rock = {}

function pickSpawnLocation(rockType)
    local spawnLocation = workspace.RockSpawnLocations:GetChildren()[math.random(1, #workspace.RockSpawnLocations:GetChildren())]
    if spawnLocation:FindFirstChild(rockType) then -- I just made the rock that has spawned a child of a spawner cause that was the first thing I thought of to check if a spawn is occupied.
        repeat
            spawnLocation = pickSpawnLocation(rockType)
        until spawnLocation and not spawnLocation:FindFirstChild(rockType)
    end
    return spawnLocation -- A spawn location has been found.
end

function Rock.Spawn(rockType, amount)
    if minableObjects:FindFirstChild(rockType) then --If the rock the script is trying to spawn exists
        for index = 1, amount do
            local newRockSpawnLocation = pickSpawnLocation(rockType)
            if newRockSpawnLocation then
                local newRock = minableObjects:FindFirstChild(rockType):Clone()
                newRock.Parent = newRockSpawnLocation
                newRock.Position = newRockSpawnLocation.Position + Vector3.new(0,newRock.Size.Y/2,0)
            else
                warn("Picked a bad spawner")
            end
            task.wait()
        end
    else
        warn("The rock that tried to spawn does not exist.")
    end
end

return Rock

the way I would do this would be to store occupied and unoccupied spawn locations in separate tables that way you don’t have to waste time looping lots of spawn locations all the time

here is a demo project that might help
Spawn Locations.rbxl (36.3 KB)

and here is the script in the demo project

local module = {}

local spawnLocations = {}

for i, child in workspace.Spawns:GetChildren() do
	table.insert(spawnLocations, child.Position)
	--child:Destroy() --its safe to Destroy the parts if you want
end

module.Spawn = function(amount)
	for i = 1, amount do
		-- try to remove a spawn location from the spawnLocations table
		local spawnLocation = table.remove(spawnLocations)
		
		-- if there are no more unoccupied locations return out of the function
		-- this is the part where you would make the function wait if you want the function to block
		if spawnLocation == nil then return end
		
		-- create a part at the spawn Location
		local part = Instance.new("Part")
		part.Position = spawnLocation
		part.Size = Vector3.new(2, 2, 2)
		part.Parent = workspace
		part.Touched:Connect(function(touchedPart)
			local player = game.Players:GetPlayerFromCharacter(touchedPart.Parent)
			if player == nil then return end
			part:Destroy()
		end)
		part.Destroying:Connect(function()
			-- if the part is destroyed then put the spawn location back into the spawnLocations table so it can be used again
			table.insert(spawnLocations, spawnLocation)
		end)
	end
end

return module

you could use ChildRemoved

workspace.Rocks.ChildRemoved:Wait()

to detect when a spawn location becomes vacant

2 Likes

This is a good alternative implementation to my solution. Depends on OP’s preference in the end I suppose.

I think this is a great way to go at it that I never thought of.