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

-ANSWERED- What is wrong with my math.random and collision script?

Asked by 6 years ago
Edited 6 years ago

What is wrong with my math.random script? I'm very new to math.random and I know I'm completely wrong. Basically I want to make it so if the number 1 or 10 is picked in a random selection out of 10 numbers (1-10) then the name of a part in the model the mesh part is in changes. Basically I also only want this to activate if the part is touched by one of the parts listed. Any ideas?

function OnTouched(Part)
if Part.Name == 'F1' or Part.Name == 'F2'or Part.Name == 'F3' or Part.Name == 'F4' or Part.Name == 'F5' or Part.Name == 'F6' or Part.Name == 'F7' or Part.Name == 'F8' or Part.Name == 'F9' or Part.Name == 'F10' or Part.Name == 'B1' or Part.Name == 'B2' or Part.Name == 'B3' or Part.Name == 'B4' or Part.Name == 'B5'then
        local mag = Part.Velocity.Magnitude
    if mag > 30 and mag < 60 then
math.randomseed(tick())
for _ = 1, 10 do
    print(math.random(10))
    wait(1)
end

if math.random == 1 or 10 then
script.Parent.Parent.FR.Name = "DFR"
end

I know it's a mess but hopefully you can understand.

2 answers

Log in to vote
1
Answered by 6 years ago

Your scriptis VERY MESSY. To make this much more clean, you could make an array storing the parts. Then, it would loop the array(or another name for tableand list) and check the name. If the part name is equal to the current part name, then, it would break the loop and continue on. If it looped the whole thing and the name isn't equal to any part, then, it would do recursion until it is equal. This is a better alternative than a huge if statement with a lot of or's

I'm assuming you don't understand what the argument of the touched event is(because you are using it to check F1,F2 etc. even though there are no such parts in the character). The argument of the Touched event is the part that is touched. Since the player is touching

For the loop at lines 6 through 9, I am guessing you want to check if any of the 10 numbers listed is the number 1 or 10. But I am also thinking that you just want to find one random number and check if it's 1-10. Please clarify in the comments.

local parts = {"F1","F2","F3","F4","F5","F6","F7","F8","F9","B1","B2","B3","B4","B5"} -- stores the array of parts
-- the argument Part is the object that touched the part the event is connected to, not the opposite
-- i.e if the left leg of the character touched the part, that would be the argument
function OnTouched(Part)
    if Part.Parent then -- makes sure that the part's parent is not nil
        print(tostring(Part))
        local function recursiveFunc() -- recursion is a better way to go rather than an if statement with multiple or's
        for index,partName in pairs(parts) do -- loops the array
            if Part.Name == partName then -- if the part name is equal to the current part
                    break -- break just ends the loop
            elseif index == #parts then -- checks if the index reached to it's maximum value
        -- I used recursion because after the loop ends, it would continue on in the code
            -- without knowing that there is a part equal to one of the string values in the parts array 
        -- if there is a part equal to one of the strings in the array, it wouldn't call itself again 
                    wait() -- very small delay just in case
                    recursiveFunc() -- call itself again
                end
            end
        end
        recursiveFunc()
        local mag = Part.Velocity.Magnitude
        -- I put lines 12 and 13 outside of the magnitude if statement because the if statement on the bottom
        -- would not be able to access randomNum
        math.randomseed(tick()) 
        local randomNum = math.random(1,10) -- since you are going to use this again, store this in a variable
        if mag > 30 and mag < 60 then
        --[[
            -- going to comment this for now
            for i = 1, 10 do
                print(math.random(1,10))
                wait(1)
            end
        --]]
        end
           if randomNum == 1 or randomNum == 10 then
             script.Parent.Parent.FR.Name = "DFR"
           end
       end
end
-- connects the event to the function so it would work when the part(script.Parent) is touched
script.Parent.Touched:Connect(OnTouched) 

Sorry if my indenting is bad. I had to put a lot of comments

0
I put a lot of time into this answer saSlol2436 716 — 6y
0
Thank you so much! TheBeaver101 28 — 6y
0
Your welcome saSlol2436 716 — 6y
Ad
Log in to vote
1
Answered by 6 years ago

CLEAN CODE

To start off, you really need read-able code if you want to get help and become a more organized, successful scripter. Your problem with the code you posted is mainly indentation and an ugly conditional that, though may work, makes it very hard to read.

INDENTATION

When indenting, think of your code as layers separated by loops, conditions or events. Whenever you declare a function, call an event, or write a conditional indent the code beneath it.

An example: Notice where I indent

local function indentExample() -- function declaration
    local var1 = "Apple" -- variables
    local var2 = "Pear"

    -- no indentation, we're just calling variables! -- 

    for i = 1, 100 do -- loop
        if i > 50 then -- conditonal
            print("This number is greater than 50")

            if i > 75 then
                print("This number is greater than 75")
            end
        else
            print("This number is", i)
        end
    end
end

LENGTHY CONDITIONALS

To avoid lengthy conditionals, many things can be done. Whether it be adding a "Marker" child to the objects that you want allowed through or using a little bit of string manipulation to find the same. For your case and simplicity reasons, I'll just create a table of strings that we want allowed.

By simply looping through the table of strings, we can achieve the same in a much cleaner fashion.

local allowedNames = {"F1", "F2", "F3", "F4", "B1", "B2", "B3", "B4"}

function onTouch(part)
    local match = false

    for _, names in pairs (allowedNames) do
        if part.Name == names then
            match = true

            break
        end
    end

    if match then
        -- stuff -- 
    end
end

RANDOM

Lastly, the solution to your problem with math.random() is quite simple. You just need to store the random value in a variable.

local randomValue = math.random(1, 10)

if randomValue == 1 or randomValue == 10 do 

end

if there are any questions, feel free to ask!

0
Wow this was very in depth. Thank you so much! TheBeaver101 28 — 6y

Answer this question