Level Determination Code

Help and Feedback Code Review
#1

This is pretty hard for me to explain but I will try my best. So I am making a sort of sandbox tycoon game where a player will have a limit on how many of each item they can build. So I have like a module script which has information about all items I have in-game

-- this is the module script
local getItemInfo = {}

local Info = {
	
	["Farmland"] = {
		["ImageId"] = 5900333028;
		["LevelRequired"] = 1.5;
		["AllowedDuringItemLevel1"] = 2;
		["AllowedDuringItemLevel2"] = 3;
		["AllowedDuringItemLevel3"] = 4;
		["AllowedDuringItemLevel4"] = 4;
		["AllowedDuringItemLevel5"] = 6;
		["WorkerMultiplier"] = 0;
		["Decoration Points"] = -5;
		["CostInCash"] = 100;
		["CostInCoin"] = 0;
	};

-- there are many more items in here I don't wanna lengthen the code, all of them have same properties.
	
}

getItemInfo.FindId = function(Name)
	local ItemInfo = Info[Name]
	
	return ItemInfo
end

return getItemInfo

[“LevelRequired”] is a multiplier which I use to determine whether a player can upgrade their item to the next level on player’s own level. For example, if a player is Level 3 they can upgrade the said item to level 2. [“AllowedDuringItemLevel1”] is the limit. [“AllowedDuringItemLevel2”] = 3, meaning the player can only place 3 of that item on their plot during when they have level 2 of that item unlocked. when they unlock level 3 the cap increases. So, to determine that the player has unlocked a particular level for that item I have created an equation which is

--- this is a local script
local TotalNumber

local Items = {}

local Player_sPlot = nil

local function GetPlot(Player)
	for i,plot in pairs(workspace.Plots:GetChildren()) do
		if plot and plot.Plot.Owner.Value == Player then
			Player_sPlot = workspace.Plots:FindFirstChild(plot.Name)
		end
	end
end

repeat
	wait()
	GetPlot(game.Players.LocalPlayer)
until Player_sPlot ~= nil

local function setTextLabel()
	local R = 1 -- imaginary number
	local Module = require(game.ReplicatedStorage.Modules.ItemInfo)
	local ItemInfo = Module.FindId(script.Parent.Parent.Name)
	local Requirement = nil
	
	repeat -- the actual equation is in this loop
		wait()
		local Level = game.Players.LocalPlayer.mainFolder.Level.Value
		local LevelRequired = ItemInfo["LevelRequired"]
		
		if Level > math.ceil(R*LevelRequired) then
			Requirement = R
		elseif Level == math.ceil(R*LevelRequired) then
			Requirement = R + 1
		else
			R = R+1
		end
	until Requirement ~= nil
	
	TotalNumber = ItemInfo["AllowedDuringItemLevel" .. tostring(Requirement)]
	
	script.Parent.TextLabel.Text = tostring(#Items) .. "/" .. tostring(TotalNumber)
end

while wait() do
	setTextLabel()
	if Player_sPlot then
		for i,Item in pairs(Player_sPlot.PlacedItems:GetChildren()) do
			if Item.Name == script.Parent.Parent.Name then -- script.Parent.Parent.Name is Item's Name
				table.insert(Items, Item)
			end
		end
	end
end

So I have tried to basically interconnect the item’s level and the item’s build cap with one variable which is [“LevelRequired”]. I take one imaginary number ‘R’ which is 1 by default and if math.ceil(R*LevelRequired) is greater than the player’s level then that means they have only unlocked ‘R’ level on that item and that also connects to [“AllowedDuringItemLevel” … R].

I wanted to know if there’s any improvements I can make on this system.
I tried my best to explain the whole thing if you are confused by all this and have questions I’ll try my best to answer it. (English is not my main language so I might have made a lot of grammar mistakes so I am sorry for that)

#2

One improvement is to shorten the index name, ["AllowedDuringItemLevel1"] is super long and can often clog up code(it won’t impact performance, just harder to read). A better name could be ADIL1, an abbreviated form of it, or simply, 1 or Level1.

The repeat wait() is highly frown upon because it’s not the most efficient way of doing this, instead, you should either have a remote event that fires when the player gets a plot or have the script run when they get the plot.

This can be done with a Changed event. So Level.Changed:Connect(function()

1 Like
#3

I would rather have a long variable name than a variable that’s abbreviated, because it can be easy to forget what an abbreviated variable is for.

@Dan_E56
You could try putting all of the AllowedDuringItemLevel values in a table inside of the Farmland table to make it better organized.

1 Like
#4

I’ve been chatting with Dan_E56 privately to understand how the math works, and i made this code to optimise the math functions, instead of using the REPEAT to do that.

--Gets what 'level' of the item is unlocked, based on the player's level
function getItemLevelUnlocked(item,playerLevel)
    local multiplier = item["LevelRequired"]
    return math.floor(playerLevel/multiplier)
    --I recommend also adding math.min here, to assert a MAXIMUM level of an item
    --e.g. return math.min(item["MaxLevel"],math.floor(playerLevel/multiplier))
end

--Gets the max limit of an item, based on the player's level
function getMaxItemLimit(item,playerLevel)
    local itemLevelUnlocked = getItemLevelUnlocked(item,playerLevel)
    TotalNumber = ItemInfo["AllowedDuringItemLevel" .. tostring(itemLevelUnlocked )]
    return TotalNumber
end

function setText()
      script.Parent.TextLabel.Text = tostring(#Items) .. "/" .. tostring(getMaxItemLimit(item,playerLevel))
end

--Other recommendations:
--Don't put a localscript in every single textlabel, just use ONE local script
--Don't update textlabel texts using a loop. I recommend doing it when the player's level changes
--It looks like the "Items" array keeps getting bigger and bigger every time the while loop runs

And I agree with the post above, for example [“AllowedDuringItemLevel5”] = 6, could be changed to something like this.

ItemLimits = {2,3,4,4,6}

3 Likes
#5

Thanks for all the feedback, after organizing some stuff I am left with these scripts

Module Scripts

local getItemInfo = {}

local Info = {
	
	["Farmland"] = {
		["ImageId"] = 5900333028;
		["LevelRequired"] = 1.5;
		["itemLimits"] = {2,3,4,4,6}; -- created a table for this instead as recommended 
		["MaxLevel"] = 5;
		["Dwelling"] = false;
		["WorkerMultiplier"] = 0;
		["Decoration Points"] = -5;
		["CostInCash"] = 100;
		["CostInCoin"] = 0;
		["Description"] = "[Level 1] Used for harvesting crops which you can sell for some money.";
	};
	
}

getItemInfo.FindId = function(Name)
	local ItemInfo = Info[Name]
	
	return ItemInfo
end

getItemInfo.CreateProperties = function(Name, Item)
-- very lengthy code
end

getItemInfo.CreateTemplate = function(ItemInfo, Frame, Name)
-- very lengthy code
end


return getItemInfo

Local Script

local remote = game.ReplicatedStorage.Events.UpdateItemButtons

script.Parent.MouseButton1Click:Connect(function()
	local Module = require(game.ReplicatedStorage.Modules.ItemInfo)
	
	local ItemInfo = Module.FindId(script.Parent.Parent.Name)
	local Frame = script.Parent.Parent.Parent.Parent.ItemInfo
	local Name = script.Parent.Parent.Name
	
	Module.CreateTemplate(ItemInfo, Frame, Name)
end)

local PlayersPlot = nil

remote.OnClientEvent:Connect(function(pltnm)
	PlayersPlot = workspace.Plots:FindFirstChild(pltnm)
end)

while wait() do
	if PlayersPlot ~= nil then
		break
	end
end

print("got plot")

local Items = {}

local ItemInfo = require(game.ReplicatedStorage.Modules.ItemInfo)

function getItemLevelUnlocked(item,playerLevel)
	local multiplier = item["LevelRequired"]
	return math.min(item["MaxLevel"], math.ceil(playerLevel/multiplier))
	-- I used ceil because floor was returning with 0
end

function getMaxItemLimit(item,playerLevel)
	local itemLevelUnlocked = getItemLevelUnlocked(item,playerLevel)
	print(itemLevelUnlocked)
	local TotalNumber = item["itemLimits"][itemLevelUnlocked]
	return TotalNumber
end

local playerLevel = game.Players.LocalPlayer.mainFolder.Level

function setText()
	local item = ItemInfo.FindId(script.Parent.Parent.Name)
	script.Parent.TextLabel.Text = tostring(#Items) .. "/" .. tostring(getMaxItemLimit(item,playerLevel.Value))
end

local function getItems()
	for k in pairs(Items) do -- clearing the table as it was brought to my attention by ultraw
		Items[k] = nil
	end
	
	for i,Item in pairs(PlayersPlot.PlacedItems:GetChildren()) do
		if Item.Name == script.Parent.Parent.Name then
			table.insert(Items, Item)
		end
	end
end

playerLevel.Changed:Connect(function()
	getItems()
	setText()
end)

-- this gets fired when the player places an item
game.ReplicatedStorage.Events.ItemAdded.OnClientEvent:Connect(function() 
	getItems()
	setText()
end)

getItems()
setText()

Let me know if there’s anything else. Thanks for all the feedback, it really helped me organize the code. One thing that I am only using one local script which gets cloned based on how many models are there.

#6

Looks a lot better well done. Good work

1 Like