Broken Daily Reward Timer

  1. What do you want to achieve?
    A Working Daily Reward System

  2. What is the issue?
    As soon as the the daily reward timer hits 00:00:00, the daily reward frame doesn’t visible and the timer became negative and it will repeat and repeat and it the daily reward frame isn’t going to be visible again

    image

  3. What solutions have you tried so far?
    None

local DataStoreService = game:GetService("DataStoreService")

local DailyRewardDS = DataStoreService:GetDataStore("NewDailyReward")

local rewardsForStreak = 
	{
		[1] = {5, 0},
		[2] = {10, 0},
		[3] = {15, 0},
		[4] = {25, 50},
		[5] = {40, 100},
	}

game.Players.PlayerAdded:Connect(function(player)

	local success, dailyRewardInfo = pcall(function()
		return DailyRewardDS:GetAsync(player.UserId .. "-DailyRewards")
	end)
	if type(dailyRewardInfo) ~= "table" then dailyRewardInfo = {nil, nil, nil} end

	local Money = player:WaitForChild("Money")
	local Exp = player:WaitForChild("Exp")

	Money.Value = dailyRewardInfo[1] or 0

	local lastOnline = dailyRewardInfo[2] or os.time()
	local CurrentTime = os.time()

	local timeDifference

	if lastOnline then
		timeDifference = CurrentTime - lastOnline
	end
	
	while task.wait(1) do
		
		local previousTime = DailyRewardDS:GetAsync(player.UserId .. "-DailyRewards" , {os.time()})

		local timeReset = (24*60*60) - (os.time() - previousTime[1])

		local hours = math.floor(timeReset / 60 / 60)
		local mins = string.format("%0.2i", math.floor(timeReset / 60) - (hours * 60))
		local secs = string.format("%0.2i", timeReset - (hours * 60 * 60) - (mins * 60))

		local DrValue = game.ReplicatedStorage:WaitForChild("Value")

		DrValue.Value = hours ..":".. mins ..":" .. secs
	end
	
	local ScreenGui = player.PlayerGui:WaitForChild("ScreenGui")
	local DailyRewardsGUI = ScreenGui:WaitForChild("DailyRewards")
	local ClaimButton = DailyRewardsGUI:WaitForChild("ClaimButton")
	local Days = DailyRewardsGUI:WaitForChild("Days")
	local Timer = ScreenGui:WaitForChild("Timer")

	if not timeDifference or timeDifference >= 24*60*60 then
		
		local streak = dailyRewardInfo[3] or 1
		local reward = rewardsForStreak[streak]

		local rewardInfo = rewardsForStreak[streak]
		local coins = rewardInfo[1]
		local exp = rewardInfo[2]

		DailyRewardsGUI.Visible = true
		Days.Text = "Day " .. streak

		if streak > 5 then streak = 1 end

		ClaimButton.MouseButton1Click:Connect(function()
			ClaimButton.Sound:Play()

			Money.Value += coins
			Exp.Value += exp

			DailyRewardsGUI.Visible = false

			local streak = streak + 1

			if streak > 5 then streak = 1 end

			local success, errormsg = pcall(function()
				DailyRewardDS:SetAsync(player.UserId .. "-DailyRewards", {Money.Value, os.time(), streak})
			end)
		end)
	elseif timeDifference then
		task.wait((24*60*60) - timeDifference)
		if game.Players:FindFirstChild(player) then

			local streak = dailyRewardInfo[3] or 1
			local reward = rewardsForStreak[streak]

			local rewardInfo = rewardsForStreak[streak]
			local coins = rewardInfo[1]
			local exp = rewardInfo[2]

			local DailyRewardsGUI = player.PlayerGui:WaitForChild("DailyRewards")
			local ClaimButton = DailyRewardsGUI:WaitForChild("ClaimButton")
			local Days = DailyRewardsGUI:WaitForChild("Days")
			
			DailyRewardsGUI.Visible = true
			Days.Text = "Days " .. streak

			ClaimButton.MouseButton1Click:Connect(function()
				ClaimButton.Sound:Play()

				Money.Value += coins
				Exp.Value += exp

				DailyRewardsGUI.Visible = false

				local streak = streak + 1

				if streak > 5 then streak = 1 end

				local success, errormsg = pcall(function()
					DailyRewardDS:SetAsync(player.UserId .. "-DailyRewards", {Money.Value, os.time(), streak})
				end)
			end)
		end
	end
end)

There is a loop that won’t make the next lines run. To solve that you could put it in a separated thread like this:

local DataStoreService = game:GetService("DataStoreService")

local DailyRewardDS = DataStoreService:GetDataStore("NewDailyReward")

local rewardsForStreak = 
	{
		[1] = {5, 0},
		[2] = {10, 0},
		[3] = {15, 0},
		[4] = {25, 50},
		[5] = {40, 100},
	}

game.Players.PlayerAdded:Connect(function(player)

	local success, dailyRewardInfo = pcall(function()
		return DailyRewardDS:GetAsync(player.UserId .. "-DailyRewards")
	end)
	if type(dailyRewardInfo) ~= "table" then dailyRewardInfo = {nil, nil, nil} end

	local Money = player:WaitForChild("Money")
	local Exp = player:WaitForChild("Exp")

	Money.Value = dailyRewardInfo[1] or 0

	local lastOnline = dailyRewardInfo[2] or os.time()
	local CurrentTime = os.time()

	local timeDifference

	if lastOnline then
		timeDifference = CurrentTime - lastOnline
	end
	
	task.spawn(function() -- We put the loop in a separated thread to let the rest of the script run
		while task.wait(1) do

			local previousTime = DailyRewardDS:GetAsync(player.UserId .. "-DailyRewards" , {os.time()})

			local timeReset = (24*60*60) - (os.time() - previousTime[1])

			local hours = math.floor(timeReset / 60 / 60)
			local mins = string.format("%0.2i", math.floor(timeReset / 60) - (hours * 60))
			local secs = string.format("%0.2i", timeReset - (hours * 60 * 60) - (mins * 60))

			local DrValue = game.ReplicatedStorage:WaitForChild("Value")

			DrValue.Value = hours ..":".. mins ..":" .. secs
		end		
	end)

	local ScreenGui = player.PlayerGui:WaitForChild("ScreenGui")
	local DailyRewardsGUI = ScreenGui:WaitForChild("DailyRewards")
	local ClaimButton = DailyRewardsGUI:WaitForChild("ClaimButton")
	local Days = DailyRewardsGUI:WaitForChild("Days")
	local Timer = ScreenGui:WaitForChild("Timer")

	if not timeDifference or timeDifference >= 24*60*60 then

		local streak = dailyRewardInfo[3] or 1
		local reward = rewardsForStreak[streak]

		local rewardInfo = rewardsForStreak[streak]
		local coins = rewardInfo[1]
		local exp = rewardInfo[2]

		DailyRewardsGUI.Visible = true
		Days.Text = "Day " .. streak

		if streak > 5 then streak = 1 end

		ClaimButton.MouseButton1Click:Connect(function()
			ClaimButton.Sound:Play()

			Money.Value += coins
			Exp.Value += exp

			DailyRewardsGUI.Visible = false

			local streak = streak + 1

			if streak > 5 then streak = 1 end

			local success, errormsg = pcall(function()
				DailyRewardDS:SetAsync(player.UserId .. "-DailyRewards", {Money.Value, os.time(), streak})
			end)
		end)
	elseif timeDifference then
		task.wait((24*60*60) - timeDifference)
		if game.Players:FindFirstChild(player) then

			local streak = dailyRewardInfo[3] or 1
			local reward = rewardsForStreak[streak]

			local rewardInfo = rewardsForStreak[streak]
			local coins = rewardInfo[1]
			local exp = rewardInfo[2]

			local DailyRewardsGUI = player.PlayerGui:WaitForChild("DailyRewards")
			local ClaimButton = DailyRewardsGUI:WaitForChild("ClaimButton")
			local Days = DailyRewardsGUI:WaitForChild("Days")

			DailyRewardsGUI.Visible = true
			Days.Text = "Days " .. streak

			ClaimButton.MouseButton1Click:Connect(function()
				ClaimButton.Sound:Play()

				Money.Value += coins
				Exp.Value += exp

				DailyRewardsGUI.Visible = false

				local streak = streak + 1

				if streak > 5 then streak = 1 end

				local success, errormsg = pcall(function()
					DailyRewardDS:SetAsync(player.UserId .. "-DailyRewards", {Money.Value, os.time(), streak})
				end)
			end)
		end
	end
end)

This script is terrifying in more ways than one. Here’s some notes:

  • I guarantee you that you do not want to have a separate DataStore just for daily reward times. You should save that kind of information along with player data if you have other uses of DataStores in your experience. It’s much easier to fetch that way too.

  • You aren’t handling a critical race condition where the player may connect to the server before PlayerAdded is connected. You need to connect your function to PlayerAdded then loop over all players in the server and call it. This way you get players who joined before PlayerAdded connects.

  • You aren’t handling cases of DataStore failures despite using pcall. Simply overwriting their data to blank tables if there’s nothing in dailyRewardInfo will result in serious data loss. If daily rewards happens to fail but your other calls succeed, players could even game an outage to inflate stats.

  • You should use dictionaries instead of arrays when storing data! Your future self will thank you so much more when you’re trying to debug or inspect data and you need to know what value belongs to what player stat. Serious projects should follow best practices!

  • While wait is a bad idiom.

  • NEVER use a DataStore in a while loop with a low time between iterations. DataStores have noticeable limitations. They are designed with low throughput in mind and your code is treating them as though they are high throughput. You will get throttled frequently.

  • You also need to seriously look into caching session data and saving only at important times, not every time you want to update data. Again refer to my low/high throughput comment. Google throughput if you don’t know what that means, but quickly it refers to an amount/time delivery relationship for a service.

  • Your while loop is going to block the subsequent code from running. This is also the main source of your issue. You aren’t stopping that loop so it’s just going to count down infinitely.

  • Handling interface code from a server script is scary. The server should ONLY be concerned with loading data, determining if the user should get their daily reward and then giving that information to the client. Handle Gui code from a LocalScript instead.

Biggest problem is the Gui and the while loop which are causing your issue.

For the daily rewards timer Gui, use client-side prediction on the timer and RemoteEvents (or RemoteFunctions if the server should respond to the client after executing some code) for the claim button. The server does not and should never need to touch interfaces at all.

For the timer, you can have the server send the amount of time until the next daily reward and the client can count down the time it received while also updating the next reward text.

For the claim button, use remotes so the client can make a request to the server to claim the reward. The server can then check if they’re eligible and if so grant the reward. If you want to design for immediate UX then both the client and the server should check eligibility.

2 Likes

Thank you for your information, I will try to fix my script and fix some lines like while wait do and others.

Hey Im very sorry for the late reply, so when the timer hits 0, the daily reward frame is visible and I can claim it but number still on negative and the players be able to collect the reward whenever they want

Oh and also I just updated my code a bit

local DataStoreService = game:GetService("DataStoreService")

local DailyRewardDS = DataStoreService:GetDataStore("NewDailyReward")

local rewardsForStreak = 
	{
		[1] = {5, 0},
		[2] = {10, 0},
		[3] = {15, 0},
		[4] = {25, 50},
		[5] = {40, 100},
	}

game.Players.PlayerAdded:Connect(function(player)

	local success, dailyRewardInfo = pcall(function()
		return DailyRewardDS:GetAsync(player.UserId .. "-DailyRewards")
	end)
	if type(dailyRewardInfo) ~= "table" then dailyRewardInfo = {nil, nil, nil} end

	local Money = player:WaitForChild("Money")
	local Exp = player:WaitForChild("Exp")

	Money.Value = dailyRewardInfo[1] or 0

	local lastOnline = dailyRewardInfo[2] or os.time()
	local CurrentTime = os.time()

	local timeDifference

	if lastOnline then
		timeDifference = CurrentTime - lastOnline
	end
	
	task.spawn(function() -- We put the loop in a separated thread to let the rest of the script run
		while task.wait(1) do

			local timeReset = (24*60*60) - (os.time() - lastOnline)

			local hours = math.floor(timeReset / 60 / 60)
			local mins = string.format("%0.2i", math.floor(timeReset / 60) - (hours * 60))
			local secs = string.format("%0.2i", timeReset - (hours * 60 * 60) - (mins * 60))

			local DrValue = game.ReplicatedStorage:WaitForChild("Value")

			DrValue.Value = hours ..":".. mins ..":" .. secs
		end		
	end)

	local ScreenGui = player.PlayerGui:WaitForChild("ScreenGui")
	local DailyRewardsGUI = ScreenGui:WaitForChild("DailyRewards")
	local ClaimButton = DailyRewardsGUI:WaitForChild("ClaimButton")
	local Days = DailyRewardsGUI:WaitForChild("Days")
	local Timer = ScreenGui:WaitForChild("Timer")

	if not timeDifference or timeDifference >= 24*60*60 then

		local streak = dailyRewardInfo[3] or 1
		local reward = rewardsForStreak[streak]

		local rewardInfo = rewardsForStreak[streak]
		local coins = rewardInfo[1]
		local exp = rewardInfo[2]

		DailyRewardsGUI.Visible = true
		Days.Text = "Day " .. streak

		if streak > 5 then streak = 1 end

		ClaimButton.MouseButton1Click:Connect(function()
			ClaimButton.Sound:Play()

			Money.Value += coins
			Exp.Value += exp

			DailyRewardsGUI.Visible = false

			local streak = streak + 1

			if streak > 5 then streak = 1 end

			local success, errormsg = pcall(function()
				DailyRewardDS:SetAsync(player.UserId .. "-DailyRewards", {Money.Value, os.time(), streak})
			end)
		end)
	elseif timeDifference then
		task.wait((24*60*60) - timeDifference)
		if game.Players:FindFirstChild(player) then

			local streak = dailyRewardInfo[3] or 1
			local reward = rewardsForStreak[streak]

			local rewardInfo = rewardsForStreak[streak]
			local coins = rewardInfo[1]
			local exp = rewardInfo[2]

			local DailyRewardsGUI = player.PlayerGui:WaitForChild("DailyRewards")
			local ClaimButton = DailyRewardsGUI:WaitForChild("ClaimButton")
			local Days = DailyRewardsGUI:WaitForChild("Days")

			DailyRewardsGUI.Visible = true
			Days.Text = "Days " .. streak

			ClaimButton.MouseButton1Click:Connect(function()
				ClaimButton.Sound:Play()

				Money.Value += coins
				Exp.Value += exp

				DailyRewardsGUI.Visible = false

				local streak = streak + 1

				if streak > 5 then streak = 1 end

				local success, errormsg = pcall(function()
					DailyRewardDS:SetAsync(player.UserId .. "-DailyRewards", {Money.Value, os.time(), streak})
				end)
			end)
		end
	end
end)

How would I do that? I am not very good at remote events and remote functions something like that.