Am I protecting my game well?

Hello, I am currently creating a simulator game and something that I know is very important for any game is to protect it against exploits, I know that it is impossible for a game in Roblox not to be hackable, but we as game developers must know how to avoid possible Client vulnerabilities, I mean RemoteEvents and RemoteFunctions. My question is if I am really doing well to protect or try to avoid exploiters by buying in my game, this is my structure:

In this way I store the main data of the articles:

return {
	[10031] = { -- Item1
		Currency = "Money";
		
		Name = "Tool1";
		Price = 500;
	};
	
	[10032] = { -- Item2
		Currency = "Money";
		
		Name = "Tool2";
		Price = 800;
	};
};

Then in the store to buy an Item verified from the Client and the Server, in this way:

-- Server
Events.BuyItem.OnServerInvoke = function(Player, ItemId, ItemType)
	if typeof(ItemId) ~= "number" or typeof(ItemType) ~= "string" or not IslandId then return false end
	if ItemModule[ItemId] then
		print("This Item Exists!")
		if Player.leaderstats[ItemModule[ItemId].Currency].Value >= ItemModule[ItemId].Price then
			Player.leaderstats[ItemModule[ItemId].Currency].Value = Player.leaderstats[ItemModule[ItemId].Currency].Value - ItemModule[ItemId].Price; 
			
			-- I give him the item
			
			print("Success!")
			return true;
		else
			print("You don't have enough money")
			return false;
		end
	else
		print("This item does not exist!")
		return false;
	end
end
-- Client
BuyButton.MouseButton1Click:Connect(function()
	if ItemModule[ItemId] then
		if Player.leaderstats[ItemModule[ItemId].Currency].Value >= ItemModule[ItemId].Price then
			if Events.BuyItem:InvokeServer(ItemId, ItemType) then
				TextLabel.Text = "Purchased!";
			else
				TextLabel.Text = "Hmmm Weird";
			end
		end
	end
end)

Am I doing it right? is there something wrong i am doing? How can I improve? do you have any other safer way?

Thanks so much for reading, :slight_smile:

1 Like

Should this be in #help-and-feedback:code-review or in #help-and-feedback:scripting-support

oh, sorry i don’t use much devforum

Just letting you know ok

30char

1 Like

This indeed belongs in #help-and-feedback:code-review.

I think you have done well. You only allow the purchase to go through if the player has enough money. You are already checking on the server if they have enough money, so it is redundant to do it on the client

The code is pretty good but there’s a couple things to note.

In your server script, the following check is weird. I don’t see where IslandId comes from and you’re not checking the type of ItemId. You also don’t seem to use ItemType either.

if typeof(IslandId) ~= "number" or typeof(ItemType) ~= "string" or not IslandId then return false end

You could also clean the code up a bit by using variables. Here’s what it might look like with some modifications:

-- Server
Events.BuyItem.OnServerInvoke = function(Player, ItemId)
	-- you actually don't need your check, because `ItemId` needs to be a number to find something in your table
	local item = ItemModule[ItemId] -- store in a variable

	if item then
		print("This Item Exists!")
		local currency = Player.leaderstats[item.Currency] -- also store in a variable

		if currency.Value >= item.Price then
			currency.Value = currency.Value - item.Price; 
			
			-- I give him the item
			print("Success!")
			return true;
		else
			print("You don't have enough money")
			return false;
		end
	else
		print("This item does not exist!")
		return false;
	end
end

For the client, you can follow a similar pattern and store things that you don’t want to re-calculate again and again.

Also, you can ditch a RemoteFunction altogether and use a RemoteEvent. If your local check on the amount of player currency checks out, you can automatically display that they purchased it after asking the server to purchase it. The server would then perform its own currency check and if it succeeds on the client and fails on the server it’s likely the person was exploiting or something went catastrophically wrong, so you shouldn’t worry about it incorrectly displaying that your item was purchased when it really wasn’t.

3 Likes

Sorry, the code I prepared was an example, it was not the real one but this is my structure, replace “IslandId” with “ItemId” it was my mistake and I use “ItemType” when there are several types of Items, for example: An Ax , a Backpack, a shovel and so on to make the event automatic and not create an event for each type of item…

I understand, if I could substitute RemoteFunction for RemoteEvent, my example has some errors, I will fix it, thanks for your reply :slight_smile: