Connection is not disconnecting

So basically, when the gun is unequipped, the gun still shoots and for a solution I tried using a connection. Unfortunately, this did not work for some reason

-- local script in gun
local UserInputService = game:GetService("UserInputService")
local player = game.Players.LocalPlayer
local Character = player.Character
local HEADSHOT = 20
local BODY_DAMAGE = 10
local SHOOTY_PART = script.Parent.TopBarrel
local Mouse = player:GetMouse()
local HandleOfGun = script.Parent.Handle
local Part_1 = script.Parent.MeshPart
local equipped = false
local connection = nil
local MouseUp = UserInputService:IsMouseButtonPressed(Enum.UserInputType.MouseButton1)

local Shots = script.Parent.Ammo
local BurstShotsAllowed = 0
local Max = script.Parent.Max

while true do
	
local function Reload()
	wait(2)
	if Max.Value < 0 then
		script.Disabled = true
	else
		Shots.Value = 25
		Max.Value = Max.Value - Shots.Value
	end
end
	
local function Fire()
	local MousePos = Mouse.Hit.p
	Shots.Value = Shots.Value - 1
	game.ReplicatedStorage.G18:FireServer(Character, HEADSHOT, BODY_DAMAGE, SHOOTY_PART, MousePos, HandleOfGun, Part_1)
end
	
script.Parent.Equipped:Connect(function() 
  equipped = true
  local Gui = script.Parent.G18:Clone()
  if Gui.Parent ~= player.PlayerGui then
	Gui.Parent = player.PlayerGui
  end
end)

script.Parent.Unequipped:Connect(function()
	equipped = false
	local Gui = player.PlayerGui.G18
	Gui:Destroy()
	if (connection) then
		connection:Disconnect()
		connection = nil
	end
end) 
	
if (not connection) then
repeat
	connection = Fire()
    wait(0.1)
until (Shots.Value == BurstShotsAllowed) or MouseUp or equipped == false
   Reload()
end
end

why do you have the entire script in a while true do loop?

For so that the reload function runs more than once

what? that makes no sense at all. You shouldn’t put any core functions such as reload (and fire) inside of a while true do loop, and you DEFINITELY shouldn’t have any connections inside of a loop

1 Like

But you’re defining the function every second that’s not very good for performance

“Connection” is a function, not an actual connection

Then how am I supposed to stop the gun from firing?

The script is fine other than the while loop.

The while loop in conjunction with the repeat loop means 1 iteration of the repeat loop occurs every time, because the until statement is evaluated after each iteration.

A while true do loop can almost always be replaced with event based programming. Why would you want to reload when the gun is not equipped? It doesn’t need to be in a loop all the time.

Is this better?

local UserInputService = game:GetService("UserInputService")
local player = game.Players.LocalPlayer
local Character = player.Character
local HEADSHOT = 20
local BODY_DAMAGE = 10
local SHOOTY_PART = script.Parent.TopBarrel
local Mouse = player:GetMouse()
local HandleOfGun = script.Parent.Handle
local Part_1 = script.Parent.MeshPart
local equipped = false
local connection = nil
local MouseUp = UserInputService:IsMouseButtonPressed(Enum.UserInputType.MouseButton1)

local Shots = script.Parent.Ammo
local BurstShotsAllowed = 0
local Max = script.Parent.Max
	
local function Reload()
	wait(2)
	if Max.Value <= 0 then
		script.Disabled = true
	else
		Shots.Value = 25
		Max.Value = Max.Value - Shots.Value
	end
end
	
local function Fire()
	local MousePos = Mouse.Hit.p
	Shots.Value = Shots.Value - 1
	game.ReplicatedStorage.G18:FireServer(Character, HEADSHOT, BODY_DAMAGE, SHOOTY_PART, MousePos, HandleOfGun, Part_1)
end
	
script.Parent.Equipped:Connect(function() 
  equipped = true
  local Gui = script.Parent.G18:Clone()
  if Gui.Parent ~= player.PlayerGui then
	Gui.Parent = player.PlayerGui
  end
end)

script.Parent.Unequipped:Connect(function()
	equipped = false
	local Gui = player.PlayerGui.G18
	Gui:Destroy()
	if (connection) then
		connection:Disconnect()
		connection = nil
	end
end) 

while true do
if (not connection) then
repeat
	connection = Fire()
    wait(0.1)
until (Shots.Value == BurstShotsAllowed) or MouseUp or (equipped == false)
   Reload()
end
end

I used connections to start and stop shooting

It will still run 1 iteration of the repeat loop before checking the condition and it will still try to reload every 0.1 seconds regardless of whether it’s equipped or not.

Connect to the user pressing their mouse, do the firing loop in that connection, reload once the mouse is released or the shot count reached. This should all be in events.

Only problem is I’m using

 repeat
	connection = Fire()
    wait(0.1)
until (Shots.Value == BurstShotsAllowed) or MouseUp or (equipped == false)

Because I’m making a gun with burst, so how can I apply that to burst

local equipped = false
local amIBursting = false

function theUserHasPressedTheMouseDown()
    amIBursting = true
    repeat
        Fire()
        wait(0.1)
    until not amIBursting or Shots.Value == BurstShotsAllowed
end

function theUserHasReleasedTheMouse()
    amIBursting = false
end

script.Parent.Equipped:Connect(function()
    equipped = true
end)

script.Parent.Unequipped:Connect(function()
    amIBursting = false
    equipped = false
end)

UserInputService.InputBegan:Connect(function(input)
    if not equipped then
        return
    end
    if input.UserInputType == Enum.UserInputType.MouseButton1 then
        theUserHasPressedTheMouseDown()
    end
end)

UserInputService.InputEnded:Connect(function(input)
    if input.UserInputType == Enum.UserInputType.MouseButton1 then
        theUserHasReleasedTheMouse()
    end
end)

Add in your Fire and Release function, and all the other bits you do on equip and unequip (minus anything to do with connection) and you’re good to go.