Cash increasing issue

Help and Feedback Scripting Support
#1

Hello! Currently having an issue in my game with increasing money for players.
Basically, every second they earn cash (1 cash update per sec) but after they buy most of their upgrades for their building, they earn 2 lots of this cash per second (cash gets added and almost a split second later another lot of cash gets added)

ServerSide Code, Note: I'm using DataStore2 to increment the saved cash. 
function addCashFunction(Player, Amount)
	local PlayerCash = DS2(Player.Team.Name.."CashRelease1", Player)
	PlayerCash:Increment(Amount)
	Player.LeaderBoardStats.TotalMoneyMade.Value = Player.LeaderBoardStats.TotalMoneyMade.Value + Amount
end
ClientSide Code 
local function incomeLooper(state)
if state == "Stop" then
	servingCounter = false
end

	for i, v in pairs (game.Workspace[player.Team.Name.."Parlour"]:GetDescendants()) do
		if v.Name == "ServingCounter" then
			servingCounter = true
			break
		else
			servingCounter = false
			script.CounterPurchased.Value = false
		end
	end
		if servingCounter == true then
			while servingCounter == true do
				incomeHandler.UpdateCash("Parlour", 10)
				wait(1)
		    	incomeLooper()
			break
		end
	end
end

counterPurchased:GetPropertyChangedSignal("Value"):Connect(function()
	if counterPurchased.Value == true then
		wait(1)
		incomeLooper()
	elseif counterPurchased.Value == false then
		wait(1)
		incomeLooper("Stop")
	end
end)

Help would be appreciated :slight_smile:

Cheers.

#2

Hi! Thanks for posting you issue here.

If you want some help, though, you’re going to have to be more direct with what your issue is. What are you expecting to happen? What actually happens? Are there any errors printed into the console? Answer these questions and we’ll be happy to help.

Another tip from just glancing at your code: Don’t allow the client to set the value of the cash without the server double checking it. Remember, anything a LocalScript is allowed to do in your game, an exploiter can do. Without implementing proper checks in the server to make sure the client is allowed to do what they’re trying to do, an exploiter can abuse your RemoteEvents and RemoteFunctions to give themselves infinite cash, among other things.

1 Like
#3

In general like Bready has said, cash shouldn’t be handled by RemoteEvents in the first place; always handle cash on the server. Creating the proper checks to confirm if the client was meant to call a RemoteEvent to ask for x amount of cash in the first place can get tricky.

2 Likes
#4

I think the issue here is that you’re doing it all asynchronously (all in the same thread), which is preventing your break code from running, as when you run the incomeLooper(state) function it is starting yet another loop.

I think this is what you mean with “they earn 2 lots of this cash per second (cash gets added and almost a split second later another lot of cash gets added)”, as with this code it’ll just be incrementing exponentially.

I don’t even think you should be using a while loop here, since you can simply call the function to do in a loop manner (which you already can stop using the servingCounter variable.

I’d recommend removing the while loop and keeping just the code that’s inside it:

incomeHandler.UpdateCash("Parlour", 10)
wait(1)
incomeLooper()

(Also make sure to remove the break)

Also, I have a security concern in your code. You’re basing the cash update on the client. This makes your code exploitable and invites exploiters to exploit your system to gain gigantic amounts of cash.

I’d recommend having the server handle cash, because that is always the most secure way. (Server handling it means exploiters can’t exploit it!)

Overall, I’d make your ClientSide code look like this:

local servingCounter = false

local function incomeLooper(state)
	for i, v in pairs (game.Workspace[player.Team.Name.."Parlour"]:GetDescendants()) do
		if v.Name == "ServingCounter" then
			servingCounter = true
			break
		else
			servingCounter = false
			script.CounterPurchased.Value = false
		end
	end

	if servingCounter then
		incomeHandler.UpdateCash("Parlour", 10)
		wait(1)
		incomeLooper()
	end
end

counterPurchased:GetPropertyChangedSignal("Value"):Connect(function()
	if counterPurchased.Value == true then
		wait(1)
                servingCounter = true
		incomeLooper()
	elseif counterPurchased.Value == false then
		wait(1)
		servingCounter = false
	end
end)

Hope this helped you out!

Cheers.

2 Likes
#5

Thank y’all for replying! I’ll add some more server side security checks rather than make the client do it. I’ll test these later as I don’t have the time right now. Will mark solutions when done.

1 Like