Suggestions to make my quest system scripts better

Help and Feedback Code Review
, ,
#1

My main objective of posting this is to see if there any shortcuts or ways to make my systems simpler and effecient overall.

Local script:

local rs = game:GetService("ReplicatedStorage")
local stg = game:GetService("StarterGui")
local plr = game.Players.LocalPlayer

local function checkQuestCompletion()
	local questProgress = plr:FindFirstChild("QuestProgress")

	if questProgress and questProgress.Value == true then
		print("Quest is complete! Player needs to return to NPC.")
	else
		print("Quest is not yet complete.")
	end
end

rs.RemoteEvents.quest1.OnClientEvent:Connect(function(state, newquest)
	print("Fired")
	local maxcount = 3
	local uihandler = require(rs:FindFirstChild("modules").UIColorhandler)
	local dialogoptions = require(rs:FindFirstChild("modules").QuestData)
		local questhandler = require(rs:FindFirstChild("modules").QuestHandler)

			local quest2 = rs:FindFirstChild("Quests")["Quest#2"]:Clone()
			local textFrame = plr.PlayerGui.QuestGui:FindFirstChild("Quest#1"):FindFirstChild("DialogFrame")
			local UI = {
				questframe2 = plr.PlayerGui.QuestGui:FindFirstChild("Quest#2").QuestFrame,
				dialogtxtbutton = textFrame.Dialog,
				txtlabel = textFrame.TextLabel,
				no = textFrame.Nah,
				yes = textFrame.Yes
			}

			local color1 = Color3.fromRGB(67, 123, 255)
			local color2 = Color3.fromRGB(45, 83, 172)

			if state == "NewValue" then
				uihandler.UiHandler(plr, textFrame, color1, color2)
				questhandler.QuestHandler(plr,newquest, UI.no, UI.yes, UI.dialogtxtbutton, dialogoptions["Quest#2"].Kate, dialogoptions["Quest#2"].player, maxcount)
				return
			end

			if stg.QuestGui.Enabled then
				uihandler.UiHandler(plr, textFrame, color1, color2)
				questhandler.QuestHandler(plr, quest2, UI.no, UI.yes, UI.dialogtxtbutton, dialogoptions["Quest#2"].Kate, dialogoptions["Quest#2"].player, maxcount)
			else
				print("Quest UI not enabled")
			end

			checkQuestCompletion() -- everytime a player opens the UI it checks the quest progress
end)

Quest Handler module

local module = {}

local rs = game:GetService("ReplicatedStorage")

module.savedvalues = {}

module.QuestHandler = function(plr,quest, branch1, branch2, questframe, npcdialogueoptions, playerdialogueoptions, maxcount)
	local questdata = require(script.Parent.QuestData)
	local slow = 0.1
	local state = module.savedvalues[plr.UserId] or false
	module.savedvalues[plr.UserId] = state
	print(module.savedvalues[plr.UserId])
	local questprogress = plr:FindFirstChild("QuestProgress")
	
	
	
	questframe.MouseButton1Click:Connect(function()
		slow = 0
		if questframe.Text == npcdialogueoptions.Unfinished or questframe.Text == npcdialogueoptions.Completed then
			questframe.Parent.Visible = false
		end
	end)
	local function disableButtons()
		branch1.Visible = false
		branch2.Visible = false
	end
	
	local function typewrite(obj, text)
		obj.Text = ""
		for i = 1, #text do
			task.wait(slow)
			obj.Text = string.sub(text, 1, i)
		end
	end
	
	local function wipe()
		for i, v in pairs(questframe.Parent:GetChildren()) do
			if v:IsA("TextButton") then
				v.Text = ""
				slow = 0.1
			end
		end
	end
	
	
	
		print(quest.Value)
		quest.Changed:Connect(function()
			print('dah daha ahaha')
		end)
	
	print(quest.Count.Value)
		if state and questprogress and not questprogress.Value  then 
			print('UNFINISHED')
			print(quest.Value)
			disableButtons() 
			typewrite(questframe, npcdialogueoptions.Unfinished)
			return
		elseif questprogress and questprogress.Value and state then
			print("DONE!")
			disableButtons()
			typewrite(questframe, npcdialogueoptions.Completed)
			return
		end

	
	
	typewrite(questframe, npcdialogueoptions.text.Question)
	
	branch1.MouseButton1Click:Once(function()
		questframe.Parent.Visible = false
	end)
	branch2.MouseButton1Click:Once(function()
		wipe()
		typewrite(questframe, npcdialogueoptions.text.Mission)
		if npcdialogueoptions.text.Mission then
			typewrite(branch1, playerdialogueoptions.Decline[2])
			typewrite(branch2, playerdialogueoptions.Accept)
			if branch2.Text == playerdialogueoptions.Accept then
				branch2.MouseButton1Click:Once(function()
					questframe.Parent.Visible = false
					state = true
					module.savedvalues[plr.UserId] = state
					rs.RemoteEvents.quest1:FireServer("QuestActivated", state)
				end)
			end
		end
	end)
	
	
	if npcdialogueoptions.text.Question then
		typewrite(branch1, playerdialogueoptions.Decline[1])
		typewrite(branch2, playerdialogueoptions.Next)
	
	end
	print(module.savedvalues[plr.UserId])
	
end

return module

Quest Accept Script:

local rs = game:GetService("ReplicatedStorage")
local questremote = rs:FindFirstChild("RemoteEvents").quest1
local stg = game:GetService("StarterGui")

script.Parent.Triggered:Connect(function(plr)
	questremote:FireClient(plr) -- this keeps the interaction active but doesnt update the quest

	rs.RemoteEvents.quest1.OnServerEvent:Connect(function(plr, state, boolean)
		if state == "QuestActivated" then
			print("HAJIME STARTOOOO")

			local mxcount = 3
			local accept = require(rs:FindFirstChild("modules").QuestAccept)
			local quest2 = rs:FindFirstChild("Quests")["Quest#2"]:Clone()

			-- store the questp progress as a boolvalue inside the Player
			if not plr:FindFirstChild("QuestProgress") then
				local questProgress = Instance.new("BoolValue")
				questProgress.Name = "QuestProgress"
				questProgress.Value = false
				questProgress.Parent = plr
			end

			local textFrame = plr.PlayerGui.QuestGui:FindFirstChild("Quest#1"):FindFirstChild("DialogFrame")
			local UI = {
				questframe2 = plr.PlayerGui.QuestGui:FindFirstChild("Quest#2").QuestFrame,
				dialogtxtbutton = textFrame.Dialog,
				txtlabel = textFrame.TextLabel,
				no = textFrame.Nah,
				yes = textFrame.Yes
			}

			quest2.Parent = plr
			accept.Accept(plr, UI.questframe2, textFrame, quest2, mxcount, boolean)
			print("Quest started for " .. plr.Name)

		elseif state == "NewValue" then
			print("NEWWWWW")
		end
	end)
end)

Quest Accept module

local module = {}
local rs = game:GetService("ReplicatedStorage")

module.Accept = function(plr,objectiveframe, questframe, quest, maxcount, state)
	local fired = rs.RemoteEvents.quest1
	print("YOSHAAA")
	objectiveframe.Visible = true

	quest.Count:GetPropertyChangedSignal("Value"):Connect(function()
		local newcount = quest.Count.Value
		objectiveframe.Objective.Text = "• Kill 3 zombies: "..newcount.."/"..maxcount

		if newcount == maxcount then
			objectiveframe.Visible = false
			quest.Value = true
			state = false

			-- update the server side quest progress
			local questProgress = plr:FindFirstChild("QuestProgress")
			if questProgress then
				questProgress.Value = true
			end
			

			print("Quest completed for " .. plr.Name)
		end
	end)
end

return module

Quest Data(quest module table)

local quests = {
	
	["Quest#2"] = {
		Kate = {	
				text = {Question = "Hey can you come help me out with this problem I have?", Mission = "Can you kill 3 zombies for me please!"},
				Unfinished = "You haven't finished my quest yet",
				Completed = "Thank you for killing those zombies. I was so scared!"
				},
		player = {
			Next = "Alright what do you got for me?",
			Decline = {"Nah, but good luck though!", "Nah I'm good"},
			Accept = "Alright sure",
		}
				
	}
}



return quests
How do I optimize my quest gui script to be better?
#2

I like how you took into consideration the ideas me and the other guy gave you but i think you have done everything required for a quest system but it looks rushed as its unreadable to any new developer and an advanced developer even though i understand reading over and its poorly organised in the modulescripts.
The best modulescript you displayed is the quest module table but you can reinnovate the module to have default data for instance declining and accepting a quest is pretty much gonna be used all the time in the quests options which will be similar to previous quests.

#3

Additionally,i feel like to make your code more efficient and optimized is first by initializing the variables at the top of the script which are gonna be used for the script then underneath can be anonymous functions.And try turning reducing the number of parameters in and making a seperate modulescript for some of the parameters in the quest handler module because some are unecessary.For instance, the Frames branch1,branch2 and questframe.However, data like the quest and npcdialogue etc can all be stored in a dictionary to make the code more cleaner.As well as the quest accept module has the same problem.

#4

Your first localscript is fantastic but its just the way organised the code by not following the basic structure of writing code and the same with Quest accept script.Its honestly like you know what to write but dont know how to structure it as later down the path of your game or any game it will be hard to implement new quests and optimize game performance to make it easier for you to change things and connect it with rest of the game.

#5

So to sum up write readable code, for example, like actually writing the names of the variables instead of abbreviating it and dont use a lot of parameters for the module. For the parameters it would be better to use a table that already has the gui in it right?

#6

essentially yes just making your code presentable and more effective.Your quest system looks fine but can use further improvements in making it optimizable as you can work on each script to work well with each other.

#7

are there any other tips you can give besides the one you already gave me. Also is it better to use one gui frame for different quests or different guis frames for different quests?

#8

Try learning how to use bindable events to make data sharing easier and ideally one gui frame is good for different guis but you can have a uipagelayout to add different quest frames.

1 Like
#9

Can you give me an example of where I would need to use bindable events since I know they connect two same type scripts together, but thats the extent of my knowledge on bindable events.

#10

The typewrite function as you may have other ways to give out quests to make it quicker on the client side and connecting the buttons to a event.