Need help with fixing level randomization system

[Zda_rc]

Hello, for the past few days I’ve been trying to create a level randomization system that starts at a level called “level start”, chooses random levels from a folder in replicated storage and moves them into the workspace, also aligning themselves with the previous level that was placed.

The system works by having a dictionary with the object and 2 other variables that let the system only randomly choose individual levels a certain amount of times.
I essentially use

getObject()

to choose a random level from the “Levels” dictionary by creating an array to allow the math.random to find a random level (since it doesnt work with dictionaries due to random only finding numerical values)
I also use a function called

getOldObject

which creates an array that stores the object of each previously randomly chosen level, and if there isnt a previous level already in that array, it returns the start level as the “old_Object” variable.

What is the issue?
my code works at first glance but has some major issues. To start off, the levels don’t actually get placed where the last level’s (“End_Point” ) CFrame is.

Along with that, spaces will be left between levels to show that the system isn’t working and isn’t actually placing each level in order (despite the output printing both the previous object and the current object successfully… i think)

Screenshot of issue:

Finally, as seen above in the image aswell, not all levels will be always chosen, appearing as if the script just stops itself prematurely for no reason.

Module_Script code: (with functions)

local module = {}


local repStorage = game:GetService("ReplicatedStorage")

local level_Storage = repStorage:FindFirstChild("Level_Storage")
local level_Workspace = game.Workspace:FindFirstChild("World").Levels

local startPoint = level_Workspace:FindFirstChild("LevelStart")


local Levels = {
	
	["Level1"] = {
		Object = level_Storage:FindFirstChild("Level1"),
		allowedUse = 1,
		Used = 0
		
	},
	
	["Level2"] = {
		Object = level_Storage:FindFirstChild("Level2"),
		allowedUse = 1,
		Used = 0

	},
	
	["Level3"] = {
		Object = level_Storage:FindFirstChild("Level3"),
		allowedUse = 1,
		Used = 0

	},
	
	["Level4"] = {
		Object = level_Storage:FindFirstChild("Level4"),
		allowedUse = 1,
		Used = 0

	}
	
}

local levelsArr = {}

for i, _ in pairs(Levels) do 
	table.insert(levelsArr, i)
end

local previousLevels = {}

function module.getOldObject(random_Object) --returns the previous object selected from getObject()
	table.insert(previousLevels, random_Object)
	local old_Object = previousLevels[#previousLevels - 1]
	if old_Object then 
		
		return old_Object
	else
		old_Object = startPoint
		print(old_Object)
		return old_Object
	end
end

 function module.getObject() --returns the current randomly selected Object
	local getRandom = levelsArr[math.random(1, #levelsArr)]
	local random_Object = Levels[getRandom]
	
	if not random_Object.Object then return end
	if random_Object.Used < random_Object.allowedUse then
		module.getOldObject(random_Object)
		random_Object.Used = random_Object.Used + 1
		return random_Object
	end
end

function module.moveObject()
	local currentObject = module.getObject()
	if not currentObject then return end
	local oldObject = module.getOldObject()
	local oldObjectCFrame = oldObject.Object.End_Point.Position
	if not oldObject then return end
	if oldObject == startPoint then
		currentObject:Clone()
		currentObject.Parent = level_Workspace
		oldObjectCFrame = oldObject.End_Point.Position
		currentObject.Object:SetPrimaryPartCFrame(CFrame.new(oldObjectCFrame))
		
	else
		currentObject.Object:Clone()
		currentObject.Object.Parent = level_Workspace
		oldObjectCFrame = oldObject.Object.End_Point.Position
		currentObject.Object:SetPrimaryPartCFrame(CFrame.new(oldObjectCFrame))
		
	end
end
	
return module

Script code: (that runs the functions)

local serverScriptService = game:GetService("ServerScriptService")

local main_module = serverScriptService.Level_System:FindFirstChild("Main_L")
local module = require(main_module)

local ran = false

local function runObject()
	if ran == false then
		module.getObject()
		wait(1)
		module.moveObject()
		ran = true
	else
		ran = false
		return 
	end
end

while task.wait() do
	runObject()
end

Output:

The “Old Object:” should return the Start_Level, not the first level chosen, and not all levels were ran even though they were supposed to.

I have looked for solutions on the forums, but nothing really applies to my code.

[azqjanna]

local function runObject()
	if ran == false then
		module.getObject()
		wait(1)
		module.moveObject()
		ran = true
	else
		ran = false
		return 
	end
end

You’re not using the return value of getObject(). The return value of getObject is also not always non-nil. Why is there a wait between getObject and moveObject? There are several ways that getObject and moveObject can “fail” by returning nil early, but you don’t handle them in any way or print any errors or warnings.

Can you tell me what this does?:

local levelsArr = {}

for i, _ in pairs(Levels) do 
	table.insert(levelsArr, i)
end

[Zda_rc]

I do use the return value of getObject(). It is used as the “currentObject” variable in the moveObject() function. The wait I just left there on accident, I was testing something. I do kinda see what you mean but you may have to elaborate on getObject returning nil.

That turns my dictionary into an array so I can actually randomize it. (Since math.random only works with numerical values)
Basically it puts “Level 1”, “Level 2”, and so on into an array and I can then randomize what level is chosen.

[azqjanna]

Yeah but you don’t do anything different if local currentObject = module.getObject() fails to get a value, you just skip it. This order of operations doesn’t make sense and its mostly because the rules of what your functions do is way too loose. You should back up and make some kind of chart that shows each step that has to be taken.

Right now there are several places that don’t use the return value of a function they call that has a return value, this doesn’t make sense. A function should either have a return value or a side effect, preferably not both (you can’t avoid it easily in this case so don’t worry)

[Zda_rc]

So should I rewrite the functions and maybe split them up individually making sure I make only certain functions preform certain actions. The problem is, the code was fairly okay until I needed to get the previous level chosen and it kinda turned messy.

In terms of the local currentObject = module.getObject(), should I just use module.getObject() and not make it a variable? I’m pretty new to using return so definitely a little confusing here.

How come I’m getting nil being returned from random_Object by the way?

I aplogize for the repeated questions.

[azqjanna]

A function is supposed to have a “contract” which is a fancy way of saying you are guaranteeing its inputs have a certain condition and it will output a certain condition. If you put a bagel in a bagel slicer, you are guaranteed to get two bagel slices out. If you put in a rock, there is absolutely no way of knowing what will happen but it probably will not be good. This is a contract.

return is used to pass a value calculated inside the function back to the caller. In Lua you can actually pass multiple. However, if you do not make sure that your function always returns something that fits the contract, then that function’s contract is no longer accurate. In most applications you say this is a separate “error” condition that the caller has to handle. Alternately, you can just crash the program by calling error(“my error message”), in the case that the error is impossible to fix. You should use these liberally to catch errors you think might be possible but aren’t sure, which you’ve kind of done here, but you don’t react to them.

getOldObject has an ok contract. It also always returns a “correct” value. This is a good function. However, here: module.getOldObject(random_Object) You don’t use the return value.

So the code has the same effect as only running the table insert. This is using the function not according to its contract, which is bad. If you intended to only add something to previousLevels, you should just put table.insert(previousLevels, random_Object) there.

getObject returns a random object, Levels[levelsArr[random]]. but CAN return nil, which your caller has not handled in any way. This first check should actually cause an error, since what its checking for is never supposed to happen. You want to stop instantly and see whats wrong:

if not random_Object.Object then
error("Tried to get object that's not in Levels"
end

The second check makes sure there are uses left for that level piece, but if there aren’t it returns nil, since reaching the end of a function returns nil. You could remove levels from levelsArr as they run out of uses, then this can’t happen, but you should keep the check and throw an error if it does!

moveObject has similar problems, it does not always do what it should, and the caller doesn’t react to it failing any differently.

There are probably CFrame math issues, but you should make these changes first and use a simplified case until that’s sorted out, such as using only square rooms which limits the number of things that could go wrong as you’re testing.

[azqjanna]

A cool way to help visualize whats happening would be to color code / name each room based on how many items were in previousLevels when it was generated. This would show you the order they are being generated in.