My on off script doesn't work for my FPS bar gui?

The problems

Well I can see a few problems with the script.

You are declaring 2 on variables. One is holding the function, one is holding the bool value.This may lead to multitude of problems later on, so make sure you are naming all of your variables differently.

Second problem, you absolutely have to use tabs. Making scripts that function as you want them to is difficult if you don't have tabs set correctly.

Here is your code properly tabbed.

on = false

function on(key) 
    if on == false then
        on = true
        key:lower()
        if key == "m" then
            script.Parent.Visible = false
            print("m")
        end
    end

    local function onSelected(mouse) 
        mouse.KeyDown:connect(on) 
    end
end

The hidden problems?

With tabbing fixed, you can tell that this is not what you wanted to do. Everything is defined in a single function and logic of the code is surely not what you wanted.

Another thing that I see is that you are using KeyDown event of mouse. While this is fine, I highly recommend using User Input Service as it is more flexible, reliable and easier to add functionality to your code later on.

The fixing

Now that we see some of the problems, let's try to redo it properly.

You basically need 3 things. A on variable, a function that handles the on/off states of the gui, aka. toggle and an event that connects the key press to the function.

local on = false -- Variable that holds the state

function toggle() -- A function that will change the state
    -- code
end

game:GetService('UserInputService').InputEnded:connect(function(input,processed)
    -- code
end)

The explanation of backbone code

Now we have the backbone of the code we need. But you might not know what is going on in the last lines.

game:GetService() is a method of the game that returns to you Instance of the Service. What you put in the parenthesis is the service's name.

'UserInputService' is a service that handles user input and is great for tracking keystrokes, mouse clicks, mouse movement and more.

.InputEnded is an event of 'UserInputService' that fires every time an input has ended. This applies to when you were holding down a key and you release it, then it fires. Or if you clicked a mouse button, the moment you release it, this event will fire.

:connect I am not sure if you know what connect does, but it is self explanatory. On the left of connect, you will have an event, that will be connected to the right side, where a function will be. Most events return parameters, which will be passed to the function that is connected to that event.

We have covered the event. We have connected that event to the function function(input,processed).

InputEnded event of the UserInputService returns 2 parameters. input object and a bool value, which I called processed. The InputObjectholds everything you need to know about the input that fired the event.

processed is how I called the second parameter. This is a bool value, so it is either true or false. This parameter will be false if there aren't any interruptions happening with the input. For example, you don't want to do something if player pressed m key while they are chatting. So, if processed parameter is false, you can safely assume that you want to process that input and not throw it away.

I hope you understood this. If you didn't please comment this answer and I will try my best to explain it further.

Code in the backbone

Now that we have the backbone written and explained, we need to put some actual code in.

Toggling

First thing you need to do, is make sure that you are tracking the on variable correctly. In the function toggle, we will handle this first.

local on = false -- Variable that holds the state

function toggle() -- A function that will change the state
    if on == false then
        on = true
    elseif on == true then
        on = false
    end

    -- code
end

game:GetService('UserInputService').InputEnded:connect(function(input,processed)
    -- code
end)

Processing input

Now, the toggle function will actually toggle the onvariable every time it is called. What we need to do now, is process the input and see if the player has pressed the mkey.

local on = false -- Variable that holds the state

function toggle() -- A function that will change the state
    if on == false then
        on = true
    elseif on == true then
        on = false
    end

    -- code
end

game:GetService('UserInputService').InputEnded:connect(function(input,processed)
    if not processed then
        if input.UserInputType == Enum.UserInputType.Keyboard then
            if input.KeyCode == Enum.KeyCode.M then
                toggle()
            end
        end
    end
end)

The explanation of the input processing

Like I said above, if not processed then makes sure that the player is not chatting while he pressed the key. That way, the gui won't go on/off while you don't want it to.

input.UserInputType is a variable that the input has. This returns the type of the input, which can range from keyboard, mouse, mouse pad, touch screen to acceleration of mobile device, rotation, and so on. What you need here is keyboard press, so we check if it is equal to Enum.UserInputType.Keyboard.

input.KeyCode When you are checking keyboard input, you want to check if the right key was pressed. The KeyCode variable holds the key that was pressed, when the event has fired (in your case, it was released as we connected the function to InputEnded event).

Finally, we can toggle the on variable.

This is a lot of code and maybe too much for doing a simple thing. But using UserInputServicethis way is a good practice and it makes it easy to add new functionality later on.

The actual toggling

What we now need to do, is actually toggle the gui. We do that inside the togglefunction.

local on = false -- Variable that holds the state

function toggle() -- A function that will change the state
    if on == false then
        on = true
    elseif on == true then
        on = false
    end

    script.Parent.Visible = on
end

game:GetService('UserInputService').InputEnded:connect(function(input,processed)
    if not processed then
        if input.UserInputType == Enum.UserInputType.Keyboard then
            if input.KeyCode == Enum.KeyCode.M then
                toggle()
            end
        end
    end
end)

The final words

There are a lot of things I did there and not everything might be perfect for your needs or easy to understand. But I helped to my best and hope this helps you. If you have any questions, please comment and I will try to help you further.

Here are some useful links:

The input service

http://wiki.roblox.com/?title=API:Class/UserInputService

The input events

http://wiki.roblox.com/index.php?title=API:Class/UserInputService/InputBegan

http://wiki.roblox.com/index.php?title=API:Class/UserInputService/InputChanged

http://wiki.roblox.com/index.php?title=API:Class/UserInputService/InputEnded

The input object

http://wiki.roblox.com/index.php?title=API:Class/InputObject

Input variables

http://wiki.roblox.com/index.php?title=API:Enum/KeyCode

http://wiki.roblox.com/index.php?title=API:Enum/UserInputType