Inventory System in a Module Script

[txcIove]

This is a simple inventory system I made which caches players inventories and includes other inventory functions. The inventory is simple and only made to store items and not interactive items such as swords/armours etc. What I want to know is: Is my code efficient? Is there a better way of doing this? Are there any large holes which exploiters could take advantage of? The module script will be stored inside server script service and the player will only have access to the fetch_inventory function which will be done via remote function.

local inventory_module = {}
local inventory_cache = {}


inventory_module.set_inventory = function(player : Player, saved_inventory : any)
	local name = player.Name
	
	if saved_inventory ~= nil then
		inventory_cache[name] = saved_inventory
		print(string.format("loaded in %s's inventory",name))
	else
		inventory_cache[name] = {}
		print(string.format("created inventory for %s",name))
	end
end

inventory_module.fetch_inventory =  function(player : Player)
	if inventory_cache[player.Name] ~= nil then
		print(string.format("returning %s's inventory",player.Name))
		return inventory_cache[player.Name]
	end
end

inventory_module.add_item = function(player : Player, item : string, amount : number)
	if inventory_cache[player.Name] ~= nil then
		if inventory_cache[player.Name][item] ~= nil then
			inventory_cache[player.Name][item] += amount
		else
			inventory_cache[player.Name][item] = amount
			print(string.format("adding %i %s to %s's inventory",amount,item,player.Name))
		end
	end
end

inventory_module.remove_item = function(player : Player, item : string, amount : any)
	if inventory_cache[player.Name] ~= nil then
		for i,v in pairs(inventory_cache[player.Name]) do
			if i == item then
				if amount == "all" then
					inventory_cache[player.Name][item] = nil
					print(string.format("removing all %ss from %s's inventory",item,player.Name))
				else
					inventory_cache[player.Name][item] = inventory_cache[player.Name][item] - amount
					print(string.format("removing %i items %s from %s's inventory",amount,item,player.Name))
					
					if inventory_cache[player.Name][item] == 0 then
						inventory_cache[player.Name][item] = nil
						print(string.format("item %s ran out from %s's inventory",item,player.Name))
					end
				end
				break
			end
		end
	end
end

inventory_module.terminate_inventory = function(player : Player)
	if inventory_cache[player.Name] ~= nil then
		inventory_cache[player.Name] = nil
		print(string.format("removing %s inventory from cache",player.Name))
	end
end

inventory_module.visualise_inventory =  function(player : Player) -- for debugging reasons
	if inventory_cache[player.Name] ~= nil then
		print(string.format("visualising %s's inventory",player.Name))
		for i,v in pairs(inventory_cache[player.Name]) do
			print(string.format("%s : %i",i,v))
		end
	end
end

--[[
inventory_cache = { -- just so I can visualise the dictionary
	tay_z3r = {
		block = 5,
		redblock = 6,
		blueblock = 2
	}
}
]]


return inventory_module

[MaloniPepperoni (Maloni)]

I dont really see any issue with it code wise, however formatting wise I have a few things that you could change that makes the code easier to read for at least me and probably other people, these are just styling suggestions and you can format your code however you want.

1. Firstly I personally never use “_” in variables or function names and rarely see anyone else doing it. Personal preference but I tend to name stuff “inventoryModule” instead of “inventory_module”

2. The way you define functions in your module works, but a simple more elegant way of doing it is the following

function inventory_module.remove_item(player, item, amount)
-- Code
end)

3. The fetch inventory function seems redundant as you can change inventory_cache to the following

local inventoryModule = {}
inventoryModule.inventoryCache = {}

-- Add stuff to inventoryCache
inventoryModule.inventoryCache[player] -- add stuff

-- Different script that isn't inventoryModule
local inventoryModule = require(inventoryModule)
local playerInventory = inventoryModule.inventoryCache[player]

They both work the same, its just a different way of structuring it.

[txcIove]

I actually didn’t know you could make functions in modules that way, and I agree it does look better. As for your 3rd point, I never thought about doing it that way, and it does seem like a more cleaner way of getting a players inventory. Thanks for your reply, I appreciate it.