Potions (tools) of a specific 'Type' cannot be purchased more than once in a row

[darkcrazylord]

1. What do you want to achieve? Keep it simple and clear!
   I’d like to build a potion shop system that lets the player purchase a potion from a GUI only if the player owns less than 5 of the ‘type’ of potion they’re trying to purchase, and then send a specific key back to the client to update the Text on the UI

Please note that I’m using a StringValue called ‘Type’ to put potions into categories, each potion has this StringValue to identify it’s type.
2. What is the issue? Include screenshots / videos if possible!
Assuming player owns 0 tools, The logic of the script works and the 1st potion of any ‘type’ can be purchased with enough ‘Points’, however, after 2nd potion is attempted to be bought of the same ‘Type’, the script doesn’t seem to be looping through the allPotions list correctly and it ends up sending a ‘nil’ response back to the local script, however after purchasing the first potion of let’s say ‘Health’ ‘type’, a potion with a ‘type’ of ‘Speed’ will be bought fine 2nd time around or if you bought potion of type ‘Defence’ and try to buy a potion of type ‘Health’, it works. (basically, can’t buy the same potion type more than once in a row)
3. What solutions have you tried so far? Did you look for solutions on the Developer Hub?
I couldn’t find any relevant information about this issue.

I have done some testing and it doesn’t look like the script wants to carry on with the logic after the specificPotionType.Value is compared with potionType, and returns ‘Nil’ back to the local scripts (there are 9 local scripts, 1 for each button in the UI), I have put a print statement after the if i == #allPotions then to see if the script gets this far and continues running after the entire player backpack is checked, but it doesn’t, it just returns ‘Nil’ to the local scripts, I’d expect it to carry on with the algorithm honestly… I don’t see why it wouldn’t.

Here’s a link to a video to showcase the output and the GUI: IssueWithBuyingPotions.mp4 - Google Drive ,note that the buttons are a little glitchy due to the wait statements in the server script, trying to understand how to use the debounce with remote functions as it’s my first time using them: (Please note that the quality of the video will be poor during the first hour or 2 of this post going live as google drive has to process the video properly)

1 of the local scripts (other 8 only differ in print statements, buttons that are pressed and remote functions)

-- LOCAL SCRIPT
local replicatedStorage = game:GetService("ReplicatedStorage")
local shopEvents = replicatedStorage:WaitForChild("ShopEvents")
local buyPotionEvents = shopEvents:FindFirstChild("BuyPotionEvents")
local buySmallHealth = buyPotionEvents:FindFirstChild("BuySmallHealth")
local buyButton = script.Parent
local player = game.Players.LocalPlayer

buyButton.MouseButton1Click:Connect(function()
	local returnValue = buySmallHealth:InvokeServer(player)
	print(returnValue)
	if returnValue == 1 then
		buyButton.Text = "Bought!"
		wait(2)
		buyButton.Text = "Buy: 50 Points"
	elseif returnValue == 2 then
		buyButton.Text = "Not enough Points"
		wait(2)
		buyButton.Text = "Buy: 50 Points"
	elseif returnValue == 3 then
		buyButton.Text = "Max number of this item type owned"
		wait(2)
		buyButton.Text = "Buy: 50 Points"
	end
end)
------------------------------------
--SERVER SCRIPT
local replicatedStorage = game:GetService("ReplicatedStorage") -- assigning ReplicatedStorage service to a variable
local serverStorage = game:GetService("ServerStorage") -- assigning ServerStorage service to a variable
local potionsFolder = serverStorage:WaitForChild("Potions") -- assigning the folder containing all the potions (tools) to a variable
local shopEvents = replicatedStorage:WaitForChild("ShopEvents") -- assigning the folder containing child folders
local buyPotionsEvents = shopEvents:WaitForChild("BuyPotionEvents") -- assigning the folder containing all RemoteFunctions to a variable

-----------------

local smallHealthPotion = potionsFolder:FindFirstChild("Small Health Potion")
local smallSpeedPotion = potionsFolder:FindFirstChild("Small Speed Potion")
local smallDefencePotion = potionsFolder:FindFirstChild("Small Defence Potion")
local mediumHealthPotion = potionsFolder:FindFirstChild("Medium Health Potion")
local mediumSpeedPotion = potionsFolder:FindFirstChild("Medium Speed Potion")
local mediumDefencePotion = potionsFolder:FindFirstChild("Medium Defence Potion")
local LargeHealthPotion = potionsFolder:FindFirstChild("Large Health Potion")
local LargeSpeedPotion = potionsFolder:FindFirstChild("Large Speed Potion")
local LargeDefencePotion = potionsFolder:FindFirstChild("Large Defence Potion")
-- assigning all the Potions (tools) to local variables
-----------------

local buySmallHealthPotion = buyPotionsEvents:FindFirstChild("BuySmallHealth")
local buySmallSpeedPotion = buyPotionsEvents:FindFirstChild("BuySmallSpeed")
local buySmallDefencePotion = buyPotionsEvents:FindFirstChild("BuySmallDefence")
local buyMediumHealthPotion = buyPotionsEvents:FindFirstChild("BuyMediumHealth")
local buyMediumSpeedPotion = buyPotionsEvents:FindFirstChild("BuyMediumSpeed")
local buyMediumDefencePotion = buyPotionsEvents:FindFirstChild("BuyMediumDefence")
local buyLargeHealthPotion = buyPotionsEvents:FindFirstChild("BuyLargeHealth")
local buyLargeSpeedPotion = buyPotionsEvents:FindFirstChild("BuyLargeSpeed")
local buyLargeDefencePotion = buyPotionsEvents:FindFirstChild("BuyLargeDefence")
local db = true
-- assigning all the remote functions to local variables
-----------------

local function potionBought(player,potion,potionType,potionCost) -- local function being fed information 
	clonePotion = potion:Clone() -- cloning a tool that the player wishes to buy and assigned it to a global variable
	local leaderstats = player:WaitForChild("leaderstats") -- leaderstats is a folder that holds the IntValues that are used and displayed for the player to showcase their 'Points'
	local points = leaderstats:FindFirstChild("Points") -- IntValue used to keep track of Points that players own, used to purchase items
	local allPotions = player.Backpack:GetChildren() -- getting all tools that the players own into a table
	if points.Value >= potionCost and #allPotions > 0 then -- check if the player can afford the potion and ensure that they at least own 1 item to avoid an empty allPotions table breaking the logic
		local numberOfPotion = 0 -- this variable will be used to keep track of how many potions of the 'type' the player wants to buy currently owns
		for i, specificPotion in pairs(allPotions) do -- for loop to check each potion (tool) in the allPoints list
			local specificPotionType = specificPotion:FindFirstChild("Type") -- finding the current potion's 'type' that is currently being checked from the list (the value of this variable should be replaced after each check)
			print("This is the type of potion in the list: ",specificPotionType.Value)
			print("This is the type of potion that the player is attempting to buy: ",potionType)
			if specificPotionType.Value == potionType then -- comparing the 'type' of potion from the list (potion player owns) and the type of the potion the player is trying to buy
				numberOfPotion = numberOfPotion + 1 -- if the potion type the player wishes to buy is the same type as the potion that is being checked from the list, add +1 to the variable
			else
				numberOfPotion = numberOfPotion + 0
				if i == #allPotions then -- waits until all items player owns have been looked through, then proceeds
					print("Managed to get this far into the logic")
					if numberOfPotion >= 5 then -- if the final value is greater or equal to 5 (player owns 5 or more potions of the same type as the one they wish to buy)
						print("max item of this type owned")
						message = 3 -- message variable hold a specific key number that will be fed back to the calling variable, will be used to determine what the client will see in text format in UI
						return message -- returns the value of message back to the variable that has called the function
					elseif numberOfPotion < 5 then -- if the final value is lower than 5 (player owns less than 5 of the same type of potion they wish to buy)
						print("enough points")
						points.Value = points.Value - potionCost -- remove the points it costs to buy the potion from the 'Points' variable
						clonePotion.Parent = player.Backpack -- assigning the parent of the potion that is bought to the player's backpack
						message = 1 -- message variable hold a specific key number that will be fed back to the calling variable, will be used to determine what the client will see in text format in UI
						return message -- returns the value of message back to the variable that has called the function
					end
				end
			end
		end
	elseif points.Value >= potionCost and #allPotions == 0 then -- check if the player can afford the potion and ensure that they don't have any tools/items in the allPotions table
		print("enough points")
		points.Value = points.Value - potionCost -- remove the points it costs to buy the potion from the 'Points' variable
		clonePotion.Parent = player.Backpack -- assigning the parent of the potion that is bought to the player's backpack
		message = 1 -- message variable hold a specific key number that will be fed back to the calling variable, will be used to determine what the client will see in text format in UI
		return message -- returns the value of message back to the variable that has called the function
	else
		print("not enough points")
		message = 2 -- message variable hold a specific key number that will be fed back to the calling variable, will be used to determine what the client will see in text format in UI
		return message -- returns the value of message back to the variable that has called the function
	end
end

-----------------
buySmallHealthPotion.OnServerInvoke = function(player) -- defining the Remote Function call from the client to a function
	if db == true then -- using the bounce to attempt to give a cooldown on the amount of requests that can be sent through (which isn't working at the moment)
		db = false
		local message = potionBought(player, smallHealthPotion, "Health", 50) -- player is the player name, then the variable referencing the potion that will be bought, potion type, and potion cost
		return message -- message variable hold a specific key number that will be fed back to the calling variable in a local script, this will determine what the client will see in text format in UI
	elseif db == false then -- if cooldown is currently true, force the request to be held captive until the cooldown is up
		wait(2)
		db = true
	end
end

buyMediumHealthPotion.OnServerInvoke = function(player)
	if db == true then
		db = false
		local message = potionBought(player, mediumHealthPotion, "Health", 100) -- player is the player name, then the variable referencing the potion that will be bought, potion type, and potion cost
		return message
	elseif db == false then
		wait(2)
		db = true
	end
end

buyLargeHealthPotion.OnServerInvoke = function(player)
	if db == true then
		db = false
		local message = potionBought(player, LargeHealthPotion, "Health", 150) -- player is the player name, then the variable referencing the potion that will be bought, potion type, and potion cost
		return message
	elseif db == false then
		wait(2)
		db = true
	end
end

buySmallSpeedPotion.OnServerInvoke = function(player)
	if db == true then
		db = false
		local message = potionBought(player, smallSpeedPotion, "Speed", 25) -- player is the player name, then the variable referencing the potion that will be bought, potion type, and potion cost
		return message
	elseif db == false then
		wait(2)
		db = true
	end
end

buyMediumSpeedPotion.OnServerInvoke = function(player)
	if db == true then
		db = false
		local message = potionBought(player, mediumSpeedPotion, "Speed", 50) -- player is the player name, then the variable referencing the potion that will be bought, potion type, and potion cost
		return message
	elseif db == false then
		wait(2)
		db = true
	end
end

buyLargeSpeedPotion.OnServerInvoke = function(player)
	if db == true then
		db = false
		local message = potionBought(player, LargeSpeedPotion, "Speed", 75) -- player is the player name, then the variable referencing the potion that will be bought, potion type, and potion cost
		return message
	elseif db == false then
		wait(2)
		db = true
	end
end

buySmallDefencePotion.OnServerInvoke = function(player)
	if db == true then
		db = false
		local message = potionBought(player, smallDefencePotion, "Defence", 50) -- player is the player name, then the variable referencing the potion that will be bought, potion type, and potion cost
		return message
	elseif db == false then
		wait(2)
		db = true
	end
end

buyMediumDefencePotion.OnServerInvoke = function(player)
	if db == true then
		db = false
		local message = potionBought(player, mediumDefencePotion, "Defence", 100) -- player is the player name, then the variable referencing the potion that will be bought, potion type, and potion cost
		return message
	elseif db == false then
		wait(2)
		db = true
	end
end

buyLargeDefencePotion.OnServerInvoke = function(player)
	if db == true then
		db = false
		local message = potionBought(player, LargeDefencePotion, "Defence", 150) -- player is the player name, then the variable referencing the potion that will be bought, potion type, and potion cost
		return message
	elseif db == false then
		wait(2)
		db = true
	end
end

-----------------


-----------------

-----------------

[ifkpop]

@darkcrazylord

numberOfPotion = numberOfPotion + 0

May I ask why you update the value here?

@darkcrazylord

buySmallHealthPotion.OnServerInvoke = function(player) -- defining the Remote Function call from the client to a function
	if db == true then -- using the bounce to attempt to give a cooldown on the amount of requests that can be sent through (which isn't working at the moment)
		db = false
		local message = potionBought(player, smallHealthPotion, "Health", 50) -- player is the player name, then the variable referencing the potion that will be bought, potion type, and potion cost
		return message -- message variable hold a specific key number that will be fed back to the calling variable in a local script, this will determine what the client will see in text format in UI
	elseif db == false then -- if cooldown is currently true, force the request to be held captive until the cooldown is up
		wait(2)
		db = true
	end
end

Have you considered using one remote function for all purchases? I believe it makes more sense to have 1 function handle all purchase attempts from a specific shop rather than a function for each potion type.

@darkcrazylord

else
				numberOfPotion = numberOfPotion + 0
				if i == #allPotions then -- waits until all items player owns have been looked through, then proceeds
					print("Managed to get this far into the logic")

Moving this outside the for loop will save you some if-checks and make a clearer code.

for i, specificPotion in ipairs(allPotions) -- ipairs is faster than pairs, pairs is the slowest way
    if specificPotionType.Value == potionType then
        numberOfPotion += 1 -- Increment by one
    end
end

print("Looped through all items! Found", numberOfPotions, "of same type.")
if numberOfPotions < 5 then
    -- Whatever happens when they can buy
else
    -- Whatever happens when they can't buy
end

Try this and see what it prints.

Also,

@darkcrazylord

clonePotion = potion:Clone() -- cloning a tool that the player wishes to buy and assigned it to a global variable

Global variables (same with your debounce) will not work well with multiple players. For example, if player1 requested to buy a potion db will be set to false, which prohibits player2 from buying a potion.

I’m having a hard time reading through your code sadly, it is a bit too much nesting. I’ll copy it over to studio and append to this message if I can see the issue better there.

[darkcrazylord]

@darkcrazylord

numberOfPotion = numberOfPotion + 0

I’ve done this to test whether the loop was being thrown off if the ‘type’ from the list was not meeting the condition, the weird thing is before I have added this, I was able to use the GUI buttons to purchase up to 5 of the same ‘type’ of potions fine, BUT once you bought 1 of a type, you could only purchase more of that type, meaning it wouldn’t let me buy any other type of potions as it would just send Nil responses back to the local script.

@ifkpop

Have you considered using one remote function for all purchases? I believe it makes more sense to have 1 function handle all purchase attempts from a specific shop rather than a function for each potion type.

Yes, but it seemed simpler to me to use multiple remote functions, that way I can be sure that the client can’t tamper with the request - even if they were manually being triggered by some injector the server script would still run correctly, I’m aware that this is pretty poor logic, but it’s my first time using remote functions…so I thought I’d give it a go. But you’re right, I’ll probably have to come up with a way to only utilise 1 local script and 1 remote function, but make checks on the server rather than set pre-determined functions for each remote function.

@ifkpop

Moving this outside the for loop will save you some if-checks and make a clearer code.

That’s a really good suggestion, I’ll definitely try utilising this as I wasn’t aware that ipairs was even a method

@ifkpop

Global variables (same with your debounce) will not work well with multiple players. For example, if player1 requested to buy a potion db will be set to false, which prohibits player2 from buying a potion.

That’s a good point…I didn’t realise this would affect multiple players, will definitely have to change this.

@darkcrazylord

buySmallHealthPotion.OnServerInvoke = function(player) -- defining the Remote Function call from the client to a function
	if db == true then -- using the bounce to attempt to give a cooldown on the amount of requests that can be sent through (which isn't working at the moment)
		db = false
		local message = potionBought(player, smallHealthPotion, "Health", 50) -- player is the player name, then the variable referencing the potion that will be bought, potion type, and potion cost
		return message -- message variable hold a specific key number that will be fed back to the calling variable in a local script, this will determine what the client will see in text format in UI
	elseif db == false then -- if cooldown is currently true, force the request to be held captive until the cooldown is up
		wait(2)
		db = true
	end
end

I have tried putting debounces in those 2 if statements because when the ‘message’ is returned to the function and to the local scripts, with multiple requests it’d start breaking the text on the buttons, but I think I should move these debounces to the local scripts as the button text can be tampered with by the local player anyway, and since players will be able to only buy 5 of each type of potion at one given time, there wouldn’t be a real need for a debounce on the server?

Thank you for the pointers, I’ll try your suggests and see how I get on with optimising the code but the main issue is likely going to persist as those are quality changes from my point of view.

[ifkpop]

Yes, sadly all my changes were regarding readability and quality of life things. I read through it in studio, but I wasn’t able to find the issue there either.

I’m having a slightly hard time following what is happening in your code in that video, since it returns nil and the things are quite delayed. If you are able to pinpoint the area of where it is happening better, feel free to ping me and I’ll take a look at it, best of luck!

(Or if you manage to solve it)

[FangScripting]

local potionsFolder = serverStorage:WaitForChild("Potions"):GetChildren()

You aren’t getting the contents of the folder unless you use GetChildren()

[darkcrazylord]

Hello there,

I have made quite a few changes to the quality and readability of both the server and local scripts, now there is only 1 each.

There is only 1 remote function handling the communication between the server and the client, and only 2 functions on the server script that handle the logic.

I have managed to fix the main issue and all the potions regardless of type can be bought without issues as long as the player has sufficient funds, however there’s a new issue and I’m struggling to understand why it’s happening… here’s the code for both scripts:

-- LOCAL SCRIPT
local replicatedStorage = game:GetService("ReplicatedStorage")
local shopEvents = replicatedStorage:WaitForChild("ShopEvents")
local buyPotionFolder = shopEvents:FindFirstChild("BuyPotionEvents")
local buyPotionEvent = buyPotionFolder:FindFirstChild("BuyPotion")
local player = game.Players.LocalPlayer
local mainShopFrame = player:WaitForChild("PlayerGui").ConsumableShop.MainShopFrame
local buyButtons = mainShopFrame:WaitForChild("BuyPotionButtons"):GetChildren()
local db = false

for i, button in ipairs(buyButtons) do
	button.MouseButton1Click:Connect(function()
		local buttonName = button.Name
		local potionType = button:FindFirstChild("Type")
		if db == false then
			local returnMessageValue, price = buyPotionEvent:InvokeServer(buttonName, potionType)
			if db == false and returnMessageValue == 1 then
				db = true
				button.Text = "Bought!"
				wait(2)
				button.Text = "Buy: "..price.." Points"
				db = false
			elseif db == false and returnMessageValue == 2 then
				db = true
				button.Text = "Not Enough Points"
				wait(2)
				button.Text = "Buy: "..price.." Points"
				db = false
			elseif db == false and returnMessageValue == 3 then
				db = true
				button.Text = "Max item of this type owned"
				wait(2)
				button.Text = "Buy: "..price.." Points"
				db = false
			end
		else
			print("Cooldown is still on - not sending anything to the server")
		end
	end)
end
----------
--SERVER SCRIPT
local replicatedStorage = game:GetService("ReplicatedStorage")
local serverStorage = game:GetService("ServerStorage")

local potionsList = serverStorage:WaitForChild("Potions"):GetChildren()
local shopEvents = replicatedStorage:WaitForChild("ShopEvents")

local buyPotionsEvents = shopEvents:WaitForChild("BuyPotionEvents")
local BuyPotion = buyPotionsEvents:FindFirstChild("BuyPotion")

local function getPotionPriceAndTool(potionName, player)
	for i, specificPotion in ipairs(potionsList) do
		if potionName == specificPotion.Name then
			local potionCost = specificPotion:FindFirstChild("Price").Value
			local potionClone = specificPotion:Clone()
			return potionCost, potionClone
		end
	end
end

BuyPotion.OnServerInvoke = function(player, potionName, potionType)
	local numberOfPotion = 0
	local message = 0
	local allPotions = player.Backpack:GetChildren()
	local levelSelection = player:WaitForChild("levelSelection")
	local leaderstats = player:WaitForChild("leaderstats")
	local points = leaderstats:FindFirstChild("Points")
	
	local potionCost, potionPlayerWants = getPotionPriceAndTool(potionName, player)
	if points.Value >= potionCost and #allPotions > 0 then
		for i, specificPotion in ipairs(allPotions) do
			local specificPotionType = specificPotion:FindFirstChild("Type") -- Finding the type of potion from the list (FOR WHATEVER REASON This variable does not get it's value updated after the first Index in the table is checked, which is causing the loop to never add the potions of a specific type)
			if specificPotionType.Value == potionType then
				numberOfPotion = numberOfPotion + 1
			end
			if numberOfPotion >= 5 then
				print("max item of this type owned")
				message = 3
				return message, potionCost
			elseif numberOfPotion < 5 then
				print("enough points")
				points.Value = points.Value - potionCost
				potionPlayerWants.Parent = player.Backpack
				message = 1
				return message, potionCost
			end
		
		end
	elseif points.Value >= potionCost and #allPotions == 0 then
		print("enough points")
		points.Value = points.Value - potionCost
		potionPlayerWants.Parent = player.Backpack
		message = 1
		return message, potionCost
	else
		print("not enough points")
		message = 2
		return message, potionCost
	end	
end

I was under impression that the local variable ‘specificPotionType’ found on the server script would be updated EACH time a new index is being checked from the table, however it looks like once the variable finds the value of ‘Type’ from the first index, it never checks it again? or it checks it but it’s not updating it’s value… do you have any idea why?

As the result of this issue, players can currently buy infinite number of potions without a cap.

Thank you for your help thus far.

[darkcrazylord]

@ifkpop I have made some changes to the logic of the code, could you please let me know if you’d be able to review them? Thank you for your help.

[ifkpop]

Have you tried printing the variables? It doesn’t look like there should be any issues with checking the type like that.

Also, have you considered using Instance Attributes | Roblox Creator Documentation instead of a Number value and String value? These are the preferred way to do these types of things.

Below is a slightly modified version of your script, mostly just variable name changes and structure changes.

-- LOCAL SCRIPT
local replicatedStorage = game:GetService("ReplicatedStorage")
local shopEvents = replicatedStorage:WaitForChild("ShopEvents")
local buyPotionFolder = shopEvents:FindFirstChild("BuyPotionEvents")
local buyPotionEvent = buyPotionFolder:FindFirstChild("BuyPotion")
local player = game.Players.LocalPlayer
local mainShopFrame = player:WaitForChild("PlayerGui").ConsumableShop.MainShopFrame
local buyButtons = mainShopFrame:WaitForChild("BuyPotionButtons"):GetChildren()
local db = false


local messageLookup = { -- Something very important is to keep *data* and *code* seperate.
	[1] = "Bought",
	[2] = "Not Enough points",
	[3] = "Max item of this type owned"
}

--			V - Declare the type of button to help with autocomplete
for _, button: TextButton in ipairs(buyButtons) do -- Replace "i" with "_" to indicate it isn't used
	button.MouseButton1Click:Connect(function()
		-- Moved declaration of buttonName and potionType to inside the if-statement, no need to declare them before that.
		if not db then
			db = true
			local buttonName = button.Name
			local potionType = button:FindFirstChild("Type")
			local returnMessageValue, price = buyPotionEvent:InvokeServer(buttonName, potionType)
			
			local statusText = messageLookup[returnMessageValue] -- Get the correct message
			local defaultText = string.format("Buy: %.1f Points", price) -- Print with 1 decimal precision (100.0)
			
			button.Text = statusText
			task.wait(2)
			button.Text = defaultText
			db = false
			
		else
			print("Cooldown is still on - not sending anything to the server")
		end
	end)
end

----------
--SERVER SCRIPT
local replicatedStorage = game:GetService("ReplicatedStorage")
local serverStorage = game:GetService("ServerStorage")

local potionsList = serverStorage:WaitForChild("Potions"):GetChildren()
local shopEvents = replicatedStorage:WaitForChild("ShopEvents")

local buyPotionsEvents = shopEvents:WaitForChild("BuyPotionEvents")
local BuyPotion = buyPotionsEvents:FindFirstChild("BuyPotion")

local function getPotionPriceAndTool(potionName, player) -- Why does it take player as input? It doesn't use it
	for _, potion in ipairs(potionsList) do -- Easier to say "for potion in potionsList" than "for specificPotion in potionsList". (Avoid long variable names when possible/reasonable.)
		if potionName == potion.Name then
			local potionCost = potion:FindFirstChild("Price").Value
			return potionCost, potion:Clone() -- Moved the clone down here
		end
	end
end

-- Gets the amount of potions matching input type present in a players inventory
local function getPotionTypeCount(potionType, player: Player)
	local potionCount = 0
	for _, potion in ipairs(player.Backpack:GetChildren()) do -- i is replaced by "_" to indicate it isn't used
		local thisPotionType = potion:FindFirstChild("Type") -- Finding the type of potion from the list (FOR WHATEVER REASON This variable does not get it's value updated after the first Index in the table is checked, which is causing the loop to never add the potions of a specific type)

		if thisPotionType.Value == potionType then
			potionCount += 1 -- Increment by 1. Same as potionCount = potionCount + 1
		end		
	end
	return potionCount
end


BuyPotion.OnServerInvoke = function(player: Player, potionName, potionType)
	local message = 0
	local levelSelection = player:WaitForChild("levelSelection")
	local leaderstats = player:WaitForChild("leaderstats")
	local points = leaderstats:FindFirstChild("Points")

	local potionCost, potionPlayerWants = getPotionPriceAndTool(potionName, player)
	
	if points.Value < potionCost then
		print("not enough points")
		return 2, potionCost -- They didn't have enough points, return messageID 2
	end
	
	-- They have a sufficient amount of points, time to check if they have too many of the type
	if getPotionTypeCount(potionType, player) >= 5 then -- No need to check after every item in the inventory
		print("max item of this type owned")
		return 3, potionCost
	end
	
	-- If they have too many potions or not enough points they would have returned by now, therefore they should be allowed to purchase
	print("enough points")
	points.Value = points.Value - potionCost -- Might be rewritable as  points.Value -= potionCost
	potionPlayerWants.Parent = player.Backpack
	return 1, potionCost
end

As you can see, the issue will most likely stay after this as this is just a readability change. I’m not sure why you would have any issues with the type though.

[darkcrazylord]

@ifkpop Hello, Thank you for your reply,

I have utilised the changes you’ve made and it has taught me a lot,

I’ve found the main reason for the potions not having a cap, I wasn’t sending the value of the potion type from the client to the server…but the instance of type itself…which is why it wasn’t being checked against the condition. I have now fixed this and everything is working fine,

I’m not too sure how I’m going to incorporate sound effects in the local script as I’ve always used conditions in the past to determine if something was a success or a failure and from there; trigger a sound effect, but I’m sure I’ll figure it out.

Thank you for your help, you sir are a life saver.

[ifkpop]

You can still do that! Add a condition to check if returnMessageValue == 1. If it is 1, then it was a success, otherwise it failed.

Glad you found your issue and glad you felt like you learned from this! Always happy to help. Best of luck on your project!