Scripting HelpersARCHIVE
A little piece of Roblox history. Preserved for everyone, one question at a time.Read about the archive →
✓ ANSWER ACCEPTED

Some kind of string error in a magnitude?

local Defplayers = {}
local Raidplayers = {}
local numDefplayers
local numRaidplayers

local function FixGuiSize()
	for i, v in pairs(script.Parent["Capture GUI"]:GetChildren()) do
		if v.Name:find'Capture' then
			v.Size = UDim2.new(0, 0, 1, 0)
		end
	end
end

coroutine.resume(coroutine.create(function()
	while wait() do
		Defplayers = {}
		Raidplayers = {}
		for i = 1, #game.Players:GetChildren() do
			if (game.Players:GetChildren()[i].Character.Torso.Position - script.Parent.Position).magnitude <= 10 then
				if game.Players:GetChildren()[i].TeamColor == BrickColor.new('Teal') then
					table.insert(Defplayers, game.Players:GetChildren()[i].Name)
				elseif game.Players:GetChildren()[i].TeamColor == BrickColor.new('Bright red') then
					table.insert(Raidplayers, game.Players:GetChildren()[i].Name)
				end
			end
		end
	end
end))

coroutine.resume(coroutine.create(function()
	while wait() do
		numDefplayers = #Defplayers
		numRaidplayers = #Raidplayers
		coroutine.resume(coroutine.create(function()
			if numDefplayers > 0 then
				local i = 1
				while i <= numDefplayers do
					wait()
					if (game.Players[Defplayers[i]].Character.Torso.CFrame.p - script.Parent.CFrame.p).magnitude > 10 then
						table.remove(Defplayers[i])						
					end
					i = i + 1
				end
			end
		end))
		coroutine.resume(coroutine.create(function()
			if numRaidplayers > 0 then
				local i = 1
				while i <= numRaidplayers do
					wait()
					if (game.Players[Raidplayers[i]].Character.Torso.Position - script.Parent.Position).magnitude > 10 then
						table.remove(Raidplayers[i])						
					end
					i = i + 1
				end
			end
		end))
	end
end))

coroutine.resume(coroutine.create(function()
	while wait() do
		if #Defplayers > 0 and #Raidplayers == 0 then
			while #Defplayers > 0 and #Raidplayers == 0 do wait()
				FixGuiSize()
				script.Parent["Capture GUI"].DefendersCapture.Size = script.Parent["Capture GUI"].DefendersCapture.Size + UDim2.new(0.1, 0, 0 ,0)
			end
		elseif #Defplayers == 0 and #Raidplayers > 0 then
			while #Defplayers == 0 and #Raidplayers > 0 do wait()
				FixGuiSize()
				script.Parent["Capture GUI"].RaidersCapture.Size = script.Parent["Capture GUI"].DefendersCapture.Size + UDim2.new(0.1, 0, 0 ,0)
			end
		end
	end
end))

bad argument #2 to '?' (string expected, got nil)

Lines 39 and 51

Don't say anything at how I use coroutines...

1 comment

0

When I'm calculating magnitude why does it say it expects a string :/

1 answer

✓ Accepted answer

Massive Cleanup

Coroutines

If you don't use coroutine.yield, there's no real point in using coroutine. Use spawn( instead of coroutine.resume(coroutine.create(.

Normally when you want use multiple threads, you don't need to, and probably shouldn't. In this case, you'll see, they were a bad idea. Coroutines add complexity (and bugs, in your case), and slow down your code. Don't use them if you don't need them. Use functions instead of background loops.

Use For Loops & Don't Repeat Yourself

Don't keep using game.Players:GetChildren()[i]. I would claim this is not even correct, let alone reasonable. Use a generic for loop to go over lists and you'll save a massive amount of typing and headaches. While you're at it, use :GetPlayers() instead of :GetChildren().

It's a bad idea to keep updating the lists in a loop. It'll easily be out-of-date and is a mess of unnecessary complexity. Instead, write a function that gives you everyone on a team:

function onTeam(color)
	local team = {}
	for _, player in pairs(game.Players:GetPlayers()) do
		if player.TeamColor == color then
			table.insert(team, player)
		end
	end
	return team
end

table.remove

Your use of table.remove is completely wrong. The first thing you have to give is the list, the second thing is the index (i).

Your way of going through the list and removing is incorrect. You should only increment i if you don't delete. Instead, you can go backwards, but use a for loop, which is so much cleaner:

		for i = #defending, 1, -1 do
			if (defending[i].Character.Torso.Position - script.Parent.Position).magnitude > 10 then
				table.remove(defending, i)
			end
		end

No spawning threads

This didn't need a coroutine or spawn at all. There was no reason for it to have a wait in it.


The third spawn is also redundant. Again, you shouldn't have a loop updating this table in the background. You should compute it where you need it.

No nested loops

The if shouldn't use while at all. Just check again after the next wait, but keep track of who has any points at all.

Here you have a script half the length, that does the same thing, but fixes a dozen bugs:

function onTeam(color)
	local team = {}
	for _, player in pairs(game.Players:GetPlayers()) do
		if player.TeamColor == color then
			table.insert(team, player)
		end
	end
	return team
end

local defendPoints = 0
local raidPoints = 0

while wait() do
	local raiding = onTeam(BrickColor.new("Bright red"))
	local defending = onTeam(BrickColor.new("Teal"))
	-- Filter down to those players capping:
	for i = #defending, 1, -1 do
		if (defending[i].Character.Torso.Position - script.Parent.Position).magnitude > 10 then
			table.remove(defending, i)
		end
	end
	for i = #raiding, 1, -1 do
		if (raiding[i].Character.Torso.Position - script.Parent.Position).magnitude > 10 then
			table.remove(raiding, i)
		end
	end
	-- Keep score:
	if #defending > 0 and #raiding == 0 then
		raidPoints = 0
		defendPoints = defendPoints + 1
	elseif #raiding > 0 and #defending == 0 then
		defendPoints = 0
		raidPoints = raidPoints + 1
	else
		defendPoints = 0
		raidPoints = 0
	end
	-- Update score display:
	script.Parent["Capture GUI"].DefendersCapture.Size = UDim2.new(0.1 * defendPoints, 0, 1, 0)
	script.Parent["Capture GUI"].RaidersCapture.Size = UDim2.new(0.1 * raidPoints, 0, 1, 0)
end

Error state checking

What's left to do is error handling. You are assuming that every person has a Character and that that Character has a Torso. That's not the case; you have to check for that. The easiest way is to check when adding in onTeam:

function onTeam(color)
	local team = {}
	for _, player in pairs(game.Players:GetPlayers()) do
		if player.TeamColor == color then
			if player.Character and player.Character:FindFirstChild("Torso") then
			table.insert(team, player)
		end
	end
	return team
end

2 comments

0

Why doesn't this have like 20 upvotes?

0

RIP my script

This discussion is preserved as it was. New answers, comments, and votes are closed.