This Bug Killed Millions of Neurons, HELP

[Herades]

Basically, I created a simple module to handle quests and wanted to do some tests. At first, it was working perfectly.

However, I noticed that when I cancel a quest and try to do it again, it kind of recreates the old folder and the old StringValue again, and creates a new folder and a new StringValue, becoming “duplicated”.

I tried several ways to solve it but I couldn’t and I didn’t find out the reason for this bug to happen. Could any kind soul help me solve this problem that made me lose billions of neurons? I want to thank you in advance.

Module:

local QuestFramework = {}

local Remotes = game:GetService("ReplicatedStorage").Remotes
local RunService = game:GetService("RunService")
local Debounce = false

local function SetText(Player, Text)
	local TextLabel = Player.PlayerGui.QuestUI.NPCTalking.TextNPC
	for i = 1, #Text do
		TextLabel.Text = string.sub(Text, 1, i)
		task.wait(0.04)
	end
end

local function CreateKillsCounter(Player)	
	local CounterFolder = Instance.new("Folder", Player)
	CounterFolder.Name = "CounterFolder"
	
	local KillsCounter = Instance.new("IntValue", CounterFolder)
	KillsCounter.Name = "KillsCounter"
	KillsCounter.Value = 0
end

local function DeleteKillsCounter(Player)
	Player.CounterFolder:Destroy()
end

local function DefineEnemy(Player, EnemyType)
	local LookingFor = Instance.new("StringValue", Player)
	LookingFor.Name = "LookingFor"
	LookingFor.Value = EnemyType
end

local function StopLookingFor(Player)
	Player.LookingFor:Destroy()
end

QuestFramework.Mission = function(EnemyType, MissionType, XPQuantity, Player)
	if MissionType == "Kill" then
		Player.Character:AddTag("TalkingToNpc")
		
		local EnemyToKill = EnemyType
		local QuestUI = Player.PlayerGui.QuestUI
		local MissionUI = Player.PlayerGui.MissionUI
		
		QuestUI.Enabled = true
		SetText(Player, "Hello stranger! Are you interested in doing me a small favor in exchange for some reward? You kill 5 " .. tostring(EnemyToKill) .. "'s and you'll get some XP!")
		
		QuestUI.NPCTalking.Accept.Visible = true
		QuestUI.NPCTalking.Refuse.Visible = true
		
		Remotes.AnswerQuest.OnServerEvent:Connect(function(Player, Answer)
			if Answer == "Accept" then
				Player.Character:RemoveTag("TalkingToNpc")
				Player.Character:AddTag("DoingQuest")
				
				QuestUI.NPCTalking.Accept.Visible = false
				QuestUI.NPCTalking.Refuse.Visible = false
				QuestUI.Enabled = false
				
				MissionUI.Enabled = true
				MissionUI.Mission.Objective.Text = "OBJECTIVE: Kill 5 " .. tostring(EnemyType) .. "'s!"
				
				CreateKillsCounter(Player)
				DefineEnemy(Player, EnemyType)
				
				Remotes.CancelQuest.OnServerEvent:Connect(function()
					MissionUI.Enabled = false
					Player.Character:RemoveTag("DoingQuest")
					
					StopLookingFor(Player)
					DeleteKillsCounter(Player)
				end)
				
				RunService.Heartbeat:Connect(function()
					if Debounce then return end
					Debounce = true
					MissionUI.Mission.Kills.Text = tostring(Player.CounterFolder.KillsCounter.Value) .. "/5"
					task.wait(1.5)
					
					if Player.CounterFolder.KillsCounter.Value == 5 then
						Player.Character:RemoveTag("DoingQuest")
						StopLookingFor(Player)

						MissionUI.Enabled = false
						DeleteKillsCounter(Player)
						
						Player.UserData.XP.Value = Player.UserData.XP.Value + XPQuantity
						return
					end
					
					Debounce = false
				end)
				
			elseif Answer == "Refuse" then
				Player.Character:RemoveTag("TalkingToNpc")
				QuestUI.NPCTalking.Accept.Visible = false
				QuestUI.NPCTalking.Refuse.Visible = false
				QuestUI.Enabled = false
			end
		end)
	end
end

return QuestFramework

Here’s the problem (folder and stringvalue being duplicated).

[Ihaveash0rtname]

I died the moment I looked at this code.

The reason it does this is because you creat a new connection each time and don’t clean it up. As in every time the quest framework’s mission function is called, you create a new OnServerEvent function.

Also may not be, I literally took a quick look at that’s what stuck out to me as the issue. (May be something else I overlooked.)

[7z99 (cody)]

I believe it has to do with how your connections are structured, you have nested connections and you aren’t ever disconnecting them, so this function will create new connections once the function is called, as well as call the old functions since they aren’t ever being disconnected.

You also have an issue where when the connection is created, if another player fires the remote event, it will create new values for each of the players who have an existing connection if that makes sense.

When remote event connections are made, they exist for all players who can access that instance, and will persist forever unless the relevant instance is destroyed or unless the connection is disconnected.

In general, I would advise restructuring how your quest system is laid out.

1- You should do everything related to the UI on the client, including showing the players the text, going through the dialogue, etc… There’s no need to wait for the server to set the text. It’s also bad practice to access the player’s GUI on the server, and writing to so many properties so often (particularly your typewriter function) adds a lot of unnecessary network overhead.

If you need to tell the player they have a new quest pending, you can use a remote event and create the necessary UI instances on the client.

2- Only fire the server/create the values when accepting the quest. If the player refuses the quest, there is no point in firing the server, then having the server have to access the playergui to set more properties

3- Get rid of your nested connections (the connections inside of connections, as well as the connections made directly inside of the Mission function). You should use the player argument of .OnServerEvent to find which player has fired the remote event, then write to the value properties inside of the player that way. This goes back to my point where remote event connections exist for all players who can access the instance.

4- If you need to replicate a status (particularly where you’re adding a tag “talkingtonpc”, “doingquest”, etc…), it’s fine to fire the server telling the server to add these statuses.

on a side note please use value.Changed to detect property changes, not a RunService connection