Sanity Check Question

Simple question. Is this sufficient type checking? Is there anything I’m missing? Note that I make use of profile service, and module scripts.

Craft.OnServerInvoke = function(player,itemname,amount)
	if typeof(itemname) ~= "string" then
		return
	end
	if typeof(amount) ~= "number" then
		return
	end
	if amount%1 ~= 0 then
		return
	end
	if not ItemInfo.CraftingRecipes[itemname] then
		return
	end
	local profile = PlayerDataCache[player]
	if not profile then
		return
	end
	return CraftingModule.CraftItem(itemname,amount,player)
end
4 Likes

I think is should be like this, I think:

Craft.OnServerInvoke = function(player,itemname,amount)
	if type(itemname) ~= "string" then
		return
	end
	if type(amount) ~= "number" then
		return
	end
	if amount%1 ~= 0 then
		return
	end
	if not ItemInfo.CraftingRecipes[itemname] then
		return
	end
	local profile = PlayerDataCache[player]
	if not profile then
		return
	end
	return CraftingModule.CraftItem(itemname,amount,player)
end
1 Like

What did you even change?

Is there a check to make sure they have the required items?

1 Like

I changed typeof() to type(). This text will be blurred

1 Like

What lol

Why did you change that?

1 Like

Any errors in the output from your script?

1 Like

You might want to check that amount is >= 1. But you probably shouldn’t be returning, it’s much better to throw an error or at least a warning. The only 2 cases where the BindableFunction gets called with invalid parameters is if some other script calls it incorrectly or a cheater is calling it illegitimately. In the former case, you really want to know why your stuff isn’t working, so it’s much better to have the console give a big red or yellow message than having it just silently do nothing. In the latter case you don’t care that the console is getting spammed.

1 Like

Yes, that’s inside the crafting module

1 Like

Definitely will do for the >1 thing. It’s a remote function, and I’m returning a string which is displayed to the player about the success/failure of the l their crafting attempt. Also, it’s in a multi purpose server script, so not quite sure about making it error. Probably will add warnings though.

2 Likes