Reviewing WIP Party System

Help and Feedback Code Review
#1

I just started working on a party system for a fun a day ago, and I want to see if I can make any improvements to the current system. Feel free to point out any tweaks that I should make. Thank you!

Server Script:

-- By Sylvern
-- Party System
-- 4/5/2020

local Storage = game:GetService("ReplicatedStorage")

local Parties = Storage:WaitForChild("Parties")
local Event = Storage:WaitForChild("PartyEvent")
local CreationCooldown = false


local PassableActions = {

	['CreateParty'] = true,
	['DisbandParty'] = true,
	['AddMember'] = true,
	['LeaveParty'] = true,
	

}


game.Players.PlayerAdded:Connect(function(plr)
	local isInParty = Instance.new("BoolValue", plr)
	isInParty.Name = 'IsInParty'
	isInParty.Value = false
end)

Event.OnServerEvent:Connect(function(plr, action, recruitMember)
	local isInParty = plr:FindFirstChild('IsInParty')
	
	local filterAction = PassableActions[action]
	if not filterAction then
		plr:Kick('Stop exploiting.')
	end
	
	if action == 'CreateParty' then
		if CreationCooldown then return end
		
		if isInParty.Value == false and not Parties:FindFirstChild(plr.Name.. 's Party') then
			warn(plr.Name.. "'s Party has been created!")
			
			local newParty = Instance.new("Folder", Parties)
			newParty.Name = plr.Name.. 's Party'
			
			local owner = Instance.new("StringValue", newParty)
			owner.Name = 'Owner'
			owner.Value = plr.Name
			
			isInParty.Value = true
			
			local partyEvent = Instance.new("RemoteEvent", newParty)
			partyEvent.Name = 'PartyHandler'
			
			CreationCooldown = true
			wait(2)
			CreationCooldown = false
		else
				
			warn('Your party already exists.')
		end
	end
	
	if action == 'DisbandParty' then
		if CreationCooldown then return end
		
		if isInParty.Value == true and Parties:FindFirstChild(plr.Name.. 's Party') then
			local PlrParty = Parties[plr.Name.. 's Party']
			
			for _, partyMembers in pairs(PlrParty:GetChildren()) do
				local getPlayers = game.Players:FindFirstChild(partyMembers.Name)
				
				if not getPlayers then warn('This is a single party.') 
				
				isInParty.Value = false
				PlrParty:Destroy()
				
				warn('You party is been removed.')
				
				CreationCooldown = true
				wait(3)
				CreationCooldown = false
								
				return end
				
				getPlayers('IsIartnPy').Value = false
			end
			
			isInParty.Value = false
			PlrParty:Destroy()
			
			warn('works')
			
			CreationCooldown = true
			wait(3)
			CreationCooldown = false
		end
	end
	
	if action == 'LeaveParty' then
		
	end
	
	if action == 'AddMember' then
		
	end
	
end)

Local Script:

-- By Sylvern
-- Party System
-- 4/5/2020

local UI = script.Parent
local Player = game.Players.LocalPlayer

local Storage = game:GetService("ReplicatedStorage")
local Event = Storage:WaitForChild("PartyEvent")
local Parties = Storage:WaitForChild("Parties")

local DisbandBtn = UI:WaitForChild("DisbandBtn")
local CreateBtn = UI:WaitForChild("CreateBtn")
local LeaveBtn = UI:WaitForChild("LeaveBtn")


if Player:WaitForChild('IsInParty').Value == true then -- When the humanoid dies, it doesn't reset everything.

	if Parties:FindFirstChild(Player.Name.. 's Party') then
		local ownerParty = tostring(Parties[Player.Name.. 's Party'])
		ownerParty = ownerParty:lower()
		
		if ownerParty:find(Player.Name) then
		
			DisbandBtn.Visible = true
		end
	end
	
	LeaveBtn.Visible = true
	CreateBtn.Visible = false
end



CreateBtn.MouseButton1Click:Connect(function()
	Event:FireServer('CreateParty')
	CreateBtn.Visible = false
	DisbandBtn.Visible = true
end)

DisbandBtn.MouseButton1Click:Connect(function()
	Event:FireServer('DisbandParty')
	CreateBtn.Visible = true
	DisbandBtn.Visible = false
end)

LeaveBtn.MouseButton1Click:Connect(function()
	Event:FireServer('LeaveParty')
end)
4 Likes
#2

I’d be surprised if this actually works:

local ownerParty = tostring(Parties[Player.Name.. 's Party'])
ownerParty = ownerParty:lower()

if ownerParty:find(Player.Name) then
	DisbandBtn.Visible = true
end

If a player named ThanksRoBama joins, it will check if the string “ThanksRoBama” is in the string “thanksrobama’s party”, which it is not since string.find is case-sensitive. Also, you’re using tostring on an instance which always returns that instance’s name. But you just found that instance by searching for it by its name. Here’s a shorter, faster and simpler way of doing exactly the same except it will work for players with capital letters in their name:

local playerParty = Parties:FindFirstChild( string.format("%s's Party", Player.Name) )
if playerParty then
	DisbandBtn.Visible = true
end

Plus, your approach breaks for the players called “Party”, “Part” and “arty”, and if there are two players called e.g. “ThanksRoBama” and “anksRo”, or where any player’s name is a subset of another player’s name.

On the server-side it’s not clear why there’s a delay between creating or disbanding parties (the whole CreationCooldown thing). Seems like it would just make it feel a lot less responsive? But if you absolutely need it, I would make the delay per-player. Right now, players have to wait 3 seconds after any player has pressed the buttons before it actually works. If you have 20 players all trying to do it at the same time, some unlucky players might end up not getting their request through for minutes at a time, thinking your game is broken. Either get rid of the cooldown, make it per-player, or at least show the player that they have to wait a bit by e.g. showing a timer on their GUI.

Your approach of using a single RemoteEvent for many different action types is fine, I guess. But I would definitely split each action into a separate function. This lets them have different signatures (i.e. take different arguments or have different names for them), and let other parts of your game call those functions (e.g. you might want to remove players from groups or disband their groups when they leave the game). Something like this:

Event.OnServerEvent:Connect(function(plr, action, ...)
	if action == 'CreateParty' then
		createParty(plr, ...)
	end
	
	if action == 'DisbandParty' then
		disbandParty(plr, ...)
	end
	
	if action == 'LeaveParty' then
		leaveParty(plr, ...)
	end
	
	if action == 'AddMember' then
		addMember(plr, ...)
	end
	
end)
2 Likes
#3

Thank you for the response, I’ll try to improve on it. :slight_smile:

1 Like