Scripting Helpers is winding down operations and is now read-only. More info→
Ad
Log in to vote
0

Some kind of string error in a magnitude?

Asked by 8 years ago
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...

0
When I'm calculating magnitude why does it say it expects a string :/ PureConcept 0 — 8y

1 answer

Log in to vote
4
Answered by
BlueTaslem 18071 Moderation Voter Administrator Community Moderator Super Administrator
8 years ago

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
0
Why doesn't this have like 20 upvotes? ScriptGuider 5640 — 8y
0
RIP my script PureConcept 0 — 8y
Ad

Answer this question