Problem with daily reward system skipping days

Help and Feedback Scripting Support
#1

so i made a daily reward system that gives cash each day but the thing is when my day 1 cash is a sunday and i claimed it and come back at tuesday,i should be getting day 2 cash but seems like it skips and gives me day 3 cash and i dont know how to fix it

The system works by sending a remote event with day amount as a param to player ui if the player has the cash then shows the ui then i would send a remote back to server with days param when player click claim then it gives me money in server

local config = require(game.ReplicatedStorage:WaitForChild("RewardSystem"):WaitForChild("RewardConfig"))
local DataStoreService = game:GetService("DataStoreService"):GetDataStore("abc") -- Define the Datastore service and define our datastore name
local id = 17373749
local mps = game:GetService("MarketplaceService")
local days = {}
game.Players.PlayerAdded:Connect(function(Player)


	local LastLogin -- Define a variable that we will use later.

	pcall(function()

		LastLogin = DataStoreService:GetAsync(Player.UserId) -- Check if the player joined already in the past 24 hours

	end)
	if LastLogin and (os.time() - LastLogin.Unix >= 86400) then

		-- os.time() returns how many seconds have passed since the Unix epoch (1 January 1970, 00:00:00)
		-- 86400 is the number of seconds in one day
		days[Player.UserId] = LastLogin.Days + 1
		if days[Player.UserId] > 7  then
			days[Player.UserId] = 1
		end
		if mps:UserOwnsGamePassAsync(Player.UserId,id) then
			game.ReplicatedStorage.RewardSystem.ShowGui:FireClient(Player, days[Player.UserId],true)
		else
			game.ReplicatedStorage.RewardSystem.ShowGui:FireClient(Player, days[Player.UserId],false)
		end

	elseif not LastLogin then -- If the player never joined the game before
		days[Player.UserId] = 1

		if mps:UserOwnsGamePassAsync(Player.UserId,id) then
			game.ReplicatedStorage.RewardSystem.ShowGui:FireClient(Player, days[Player.UserId],true)
		else
			game.ReplicatedStorage.RewardSystem.ShowGui:FireClient(Player, days[Player.UserId],false)
		end-- We fire an event ()
	else
	end
end)

game.ReplicatedStorage.RewardSystem.GiveMoney.OnServerEvent:Connect(function(plr,day,owns)
	if plr and day then
		local cash 
		if owns then
			for i,v in pairs(config.viprewards) do
				if i == tostring(day) then
					cash = v
					break
				end
			end
		else
			for i,v in pairs(config.rewards) do
				if i == tostring(day) then
					cash = v
					break
				end
			end
		end
		if cash ~= nil then
			plr.leaderstats.Cash.Value += cash
			game.ReplicatedStorage.Miscs.Ui.XpAndCashNotify:FireClient(plr,"daily reward",cash)
		else
			warn("error in DailyReward line 51")
		end
		DataStoreService:SetAsync(plr.UserId, {
			Unix = os.time(),
			Days = days[plr.UserId]
		}) -- We update the player datastoreto the current Unix epoch
	end
end)

Thanks for any help

#2

I am not sure what the problem is, as your math appears to be correct. I think the issue might be client-side. Can you show the client code that executes when the ShowGui event is activated and the code for the GiveMoney event?

I do want to warn you, however, that the system you set up is not protected against data loss. In fact, it’d actually be relatively easy to lose your daily rewards data. This is because you do not check to see if the datastore request ever succeeded in the first place. This means that if the request errors then the LastLogin variable will be nil regardless. With the way you have it set up, if the request errors then it will continue executing the rest of the code as if there is no player data to begin with. However, if it errors then you don’t have the data. To protect against this, you can just try getting the players data a certain amount of times.

On another note, your second event, GiveMoney, is receiving data from the client. Because the client can exploit and thus falsify this data, you should never implicitly trust the data it returns. Instead, you should do checks on the server to make sure that the clients request is valid (These are called sanity checks). You can also do the same checks on the client to check whether you should even send a request to the server in the first place. For example, since the client is returning the day and owns data, an exploiter could easily falsify this to a different day and say the player owns the gamepass when they don’t.

Variable Naming

Also, it’s good practice to name your variables so that it’s easy to read and understand. Your DataStoreService variable is therefore misleading as it might make someone think that it is the DataStoreService | Roblox Creator Documentation instead of a DataStore | Roblox Creator Documentation. Usually DataStore variables are named with the datastore name and a keyword that identifies it as a datastore at the end. So in this case it could be something like abcDataStore, abcDatastore, or abcStore.

With all that said, your code should now look like this:

local RewardSystem = game:GetService("ReplicatedStorage"):WaitForChild("RewardSystem")
local XpAndCashNotify = game:FindService("ReplicatedStorage"):WaitForChild("Misc"):WaitForChild("Ui"):WaitForChild("XpAndCashNotify")
local config = require(RewardSystem:WaitForChild("RewardConfig"))
local abcStore = game:GetService("DataStoreService"):GetDataStore("abc") -- Define the Datastore service and define our datastore name
local mps = game:GetService("MarketplaceService")

local gamepassId = 17373749
local PlayerRecord = {} -- data cache
local FailedSaves = {}

local attempts = 3 -- how many attempts are made to retrieve the data
local function PlayerData(service, method, ...)
	local Success, Data -- store variables for later

	for i=1,attempts do
		Success, Data = pcall(method, service, ...)
		if Success then break end
	end
	return Success, Data
end

game:GetService("Players").PlayerAdded:Connect(function(Player)
	local Success, PlayerRecord = PlayerData(abcStore, abcStore.GetAsync, Player.UserId)

	if Success then -- the request succeeded
		local curTime = os.time()
		local success, ownsGamepass = PlayerData(mps, mps.UserOwnsGamePassAsync, Player.UserId, gamepassId) -- web calls should be wrapped in a pcall
		local receiveDailyReward = false -- flag

		if PlayerRecord then -- they already have data
			receiveDailyReward = ((curTime - PlayerRecord.Unix) >= 86400) -- 86400 is the number of seconds in one day
			if receiveDailyReward then
				PlayerRecord.OwnsGamepass = if success then ownsGamepass else (PlayerRecord.OwnsGamepass==true) -- you can define your own behavior if the request fails. This is just an example.
			end
		else -- they are new
			PlayerRecord = {
				Days = 0,
				Unix = curTime,
				OwnsGamepass = if success then ownsGamepass else false
			}
			receiveDailyReward = true
		end
		PlayerRecords[Player.UserId] = PlayerRecord --cache their data
		
		RewardSystem.ShowGui:FireClient(Player, if PlayerRecord.Days >= 7 then 1 else PlayerRecord.Days+1, PlayerRecord.OwnsGamepass) -- we fire an event
	else -- the request failed
		--however you want to handle player data not loading. Perhaps teleporting them to their current server and therefore rejoining the game?
	end
end)

game:FindService("Players").PlayerRemoving:Connect(function(Player)
	local failedSave = table.find(FailedSaves, Player.UserId) -- flag
	if failedSave then -- check if their save failed earlier
		PlayerData(abcStore.SetAsync, abcStore, Player.UserId, PlayerRecords[Player.UserId]) -- try saving again
		table.remove(FailedSaves, failedSave) -- remove
	end
	
	PlayerRecords[Player.UserId] = nil -- remove record from cache
end)

RewardSystem.GiveMoney.OnServerEvent:Connect(function(Player)
	local PlayerRecord = PlayerRecords[Player.UserId]
	local dailyRewards = if PlayerRecord.OwnsGamepass then config.viprewards else config.rewards
	
	PlayerRecord.Days = if PlayerRecord.Days >= 7 then 1 else PlayerRecord.Days+1
	PlayerRecord.Unix = os.time()
	
	local cash = dailyRewards[tostring(PlayerRecord.Days)]
	
	if cash then
		Player.leaderstats.Cash.Value += cash
		XpAndCashNotify:FireClient(Player, "daily reward", cash)
	else
		warn("cash value not defined")
	end
	
	if not PlayerData(abcStore.SetAsync, abcStore, Player.UserId, PlayerRecord) then table.insert(FailedSaves, Player.UserId) end -- save current record and if save failed try later when player leaves the game
end)
#3

I think it has something to do with your days > 7 expression.
Not too sure though