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

Why does this script not work?

Asked by 8 years ago

Please make your question title relevant to your question content. It should be a one-sentence summary in question form.

My script is supposed to check if these things, "holders," are in workspace. If they are, remove them. It doesn't remove them and no errors pop up. Why?

function removeholders()
    local a1holder = game.ReplicatedStorage.a1holder
    local a2holder = game.ReplicatedStorage.a2holder
    local a3holder = game.ReplicatedStorage.a3holder
    local b1holder = game.ReplicatedStorage.b1holder
    local b2holder = game.ReplicatedStorage.b2holder
    local b3holder = game.ReplicatedStorage.b3holder
    if a1holder.Parent == workspace then
        a1holder:Remove()
    end
    if a2holder.Parent == workspace then
        a2holder:Remove()
    end
    if a3holder.Parent == workspace then
        a3holder:Remove()
    end
    if b1holder.Parent == workspace then
        b1holder:Remove()
    end
    if b2holder.Parent == workspace then
        b2holder:Remove()
    end
    if b3holder.Parent == workspace then
        b3holder:Remove()
    end
end

Note: This is only the function. The function is run later in the script.

3 answers

Log in to vote
2
Answered by 8 years ago

There's a better way of going about this; however, this will require chaning a bit of your code. :o

Firstly, instead of using multiple variables, we can use a For loop, which can be setup as:

local AHolderVariable = {} -- This is a table, fyi
local BHolderVariable = {}

Now, you may be wondering "where's the loop?" But we need Tables for the For loop(s).

Instead of repeatedly creating the same code over-and-over again, let's use a function instead:

function AddToList(list, key) -- New Function: List for the asset to be added to, Key for the A or B holder type
    for i, v in pairs(list) do -- Here's our for loop; this'll be used to loop through the assets within the ReplicatedStorage
        if key == 'A' then -- If the key is 'A' then run next chunk; if key is equal to 'A', then run next code
            if v.Name == 'a' .. i .. 'holder' then -- This may not be efficient, however, this is a way w/o having to stress out about having to add any more code, and complicating the whole thing
                table.insert(AHolderVariable, v) -- This'll add the asset into the table
            end -- Ends code
        elseif key == 'B' then -- Same as the last if statement, only different is the key
            if v.Name == 'b' .. i .. 'holder' then
            table.insert(BHolderVariable, v)
            end
        end
    end
end

AddToList(game:GetService('ReplicatedStorage'):GetChildren(), 'A') -- An example of how to use the function
-- Fyi, 'GetChildren' is a method that is used to add assets, within a specific parent, into a table, and will return the table after adding all assets

Now, for removing the assets, there's a way rather than having to create multiple if statements to remove them from your game, even so, 'remove' is deprecated, and is recommended not to use.

To remove the assets, we can, again, use the For loop to accomplish this, and it'll reduce the amount of code used; however, it will still use a bit of code, but hopefully not as much as before:

function CheckOrRemoveAssets(list) -- Btw, this is based on your code having to remove everything @ once; whether it's type 'A' or 'B'
    for i, v in pairs(list) do -- Will loop through the table
        if v.Parent == game:GetService('Workspace') then -- If the asset's parent is the 'Workspace' then...
            v:Destroy() -- Destroy the asset (Fyi, I recommend using 'Destroy' instead of 'remove', as Destroy is more efficient, and locks the assets; once called on an asset, it's parent prop. can not be changed, nor any of it's props. after being 'destroyed')
        end -- Ends code chunk
    end
end

CheckOrRemoveAssets(AHolderVariable) -- An example of using it; the variable in the function, list, is used to specify what 'AHolderVariable' (or any value/ argument given) is being given, and will be used in the code; something along those lines, anyway :P

After accomplishing all this, we now have the final product, but before we're done, I'd like to explain the problem w/ your original code:

1. You put your variables inside the function, which isn't necessarily bad, but in your case it was a big mistake; here's how it is working: Get the assets from ReplicatedStorage while checking @ the same time afterwards; however, to fix this, you can put the variables outside of the function. :)

2. You used a lot of unnecessary code, such as using a lot of If statements when you could've used a For loop to do that, and decrease the amount of code used; your way of doing it wasn't necessarily bad, but there's another way. :)

3. This is a bit tricky, especially if you're starting out, but you could use a For loop to add the assets into a table, then let another For loop check that table, such as I've in the code I used here; again, it may be tricky, but for me, it's a lot more efficient, and better. :)

And Wa-la! We have shortened your code by so much, while making it more efficient for use!

Note: I have not personally tested any of this code; I typed it w/o going into the studio, so it may be buggy, but if it is breaking or is too buggy, please contact me immediately. :)

Other code touched on, but may needed to be explained further: Destroy, GetChildren, If statements

I hope this helped, as it's been a while since I've been on! :D

0
Honestly, this feels like one of the worst answers I've ever given. XD TheeDeathCaster 2368 — 8y
0
Hey you're back! The last I checked you were gone for around two months or something. User#11440 120 — 8y
0
@wfvj014 Ya. :P The reason is because I was tired of my questions not getting answered, so I went to the ROBLOX forums to ask questions there for a while, and it was pretty fun. :) TheeDeathCaster 2368 — 8y
Ad
Log in to vote
1
Answered by 8 years ago

If that object is in ReplicatedStorage is in Workspace instead, what is the point of linking the variable to inside of ReplicatedStorage, variables access items at the time they are fired, they are not stationary variables that will stay put on one value once it's given, so a better way is to use FindFirstChild(), a event that lets you look for a certain/specific object by name. If your at least half experienced, especially growing newbies, should by know this event already.

function removeholders()
    local a1holder = game.Workspace:FindFirstChild("a1holder")
    local a2holder = game.Workspace:FindFirstChild("a2holder")
    local a3holder = game.Workspace:FindFirstChild("a3holder")
    local b1holder = game.Workspace:FindFirstChild("b1holder")
    local b2holder = game.Workspace:FindFirstChild("b2holder")
    local b3holder = game.Workspace:FindFirstChild("b3holder")
    if a1holder then -- you can use the .Parent, but this part checks if it is available or existing first.
        a1holder:Remove() -- so if you want to use Parent which would be unnessecary because it
                                                -- is already found in the child of Workspace
    end
    if a2holder then
        a2holder:Remove() -- a alternate way of using a event like this is Destroy(), just a fun helping fact.
    end
    if a3holder then
        a3holder:Remove()
    end
    if b1holder then
        b1holder:Remove()
    end
    if b2holder then
        b2holder:Remove()
    end
    if b3holder then
        b3holder:Remove()
    end
end

Here's a more shorter way, but will leave you more clueless if you don't understand it, it will use concatenation as a huge help to shorten out the variables and amount of script lines used.

This collects all children in the Workspace and checks the name of it, but is combined into one 'If' inside a for ipairs loop, specifically.

function findholders()
    for i,v in ipairs(game.Workspace:GetChildren()) do
        if v.Name == "a1holder" or v.Name == "a2holder" or v.Name == "a3holder" or v.Name == "b1holder" or "b2holder" or "b3holder"
        end
    end
end
                      -- run this function later or inside the script using findholders()

By the way, I do not criticize you if you don't know any topics or things about scripting. I am fairly alike as you if you believe I am offensive.

TheAlphaStigma's way is more complex then mines, if you choose a better understandable script, this may be the ones for you, and hopefully you won't get confused. I am just answering easy questions like these the easy way.

0
Ya; my code got more complex over the period of me being gone. ;-; TheeDeathCaster 2368 — 8y
Log in to vote
0
Answered by 8 years ago

Please provide explanation with your answers. Simply posting code does not spread knowledge of integral scripting processes which helps people understand the logic and reasoning behind your answer.
--You're welcome BB
--Help from SkilledFlame
local holders = {
    a1holder = game.ReplicatedStorage.a1holder;
    a2holder = game.ReplicatedStorage.a2holder;
    a3holder = game.ReplicatedStorage.a3holder;
    b1holder = game.ReplicatedStorage.b1holder;
    b2holder = game.ReplicatedStorage.b2holder;
    b3holder = game.ReplicatedStorage.b3holder;
}

function removeholders()
 for _, i in pairs (holders) do
    if i then
        if i.Parent = workspace then
        i:remove()
            end
        end
    end
end
0
Have you ever heard that doing something for someone doesn't help as much as teaching them how to do it themselves? User#11440 120 — 8y
0
You also spelled "you're" wrong. Just a thing. User#11440 120 — 8y
1
Yeah I kind of agree with wfvj just like the saying goes "Catch a fish for a person feed them for one day but, teach them how to fish feed them for a life time" However, I don't get how he spelled "You're" wrong.... It's a joining of 'You' and 'Are' which gives you "You're"..... KingLoneCat 2642 — 8y

Answer this question