Is this an efficient/good way to pick a random item with rarities?

Hey! I saw many people overcomplicating this, so I tried to make my own system for this.
Now the question is, is this efficient/good or not? It should be pretty easy to understand.

• Efficient, itâ€™s pretty good!
• Not efficient, will tell you why in comments.

0 voters

Code:

local Items = { --ItemsTable, lower = rarer
{"FirstItem", 1};
{"SecondItem", 2};
{"ThirdItem", 3};
{"FourthItem", 4};
{"FifthItem", 5};
{"SixthItem", 6};
{"SeventhItem", 7};
{"EigthItem", 8};
{"FifthiestItem", 50};
}

local function getNums() --Gets the lowest and highest numbers of the table[2]
local highest = 0
local lowest = math.huge
for i,v in next, Items do
if v[2] > highest then
highest = v[2]
end
if v[2] < lowest then
lowest = v[2]
end
end
return {highest, lowest}
end

local function getRandomItem()
local lowest = getNums()[2] --Get the lowest number for math.random
local highest = getNums()[1] --Get the highest number for math.random
local randomNumber = math.random(lowest, highest) --Set the randomNumber to math.random the lowest and highest number
for i, item in next, Items do --Loop through the Items table
if randomNumber <= item[2] then --If the math.random number is smaller or equal to the value, then return that item
return item[1] --Return that item
end
end
end

for i = 1, 10000 do --Loops 10000 and executes that function. Prints the item for testing
print(getRandomItem())
end

Thanks!
Edit: Also, if you want to know, it does work fine when testing it.

2 Likes

Iâ€™m not well versed with weighted random pickers but I notice there are two issues that should be addressed. First is to stop using â€śin nextâ€ť and instead use â€śipairsâ€ť because youâ€™re working with a contiguous array, not a dictionary, and the former is weird and bad for readability. Second is to not call getNums twice in getRandomItem. Pick one of these two options:

local lowest, highest = table.unpack(getNums())

local nums = getNums()
local highest = nums[1]
local lowest = nums[2]

Itâ€™d probably be more readable as well if the lower number was returned first instead of the second. On that note: you know you can return more than one thing right? You could then use the first option I supplied above for alternatives but without the table unpacking and without wrapping your returns in a table.

local function foobar()
return 1, 2
end

local first, second = foobar()
print(first, second)
2 Likes