Scripting HelpersARCHIVE
A little piece of Roblox history. Preserved for everyone, one question at a time.Read about the archive →
All questions/Question #21496
✓ ANSWER ACCEPTED

Why does my reload script crash studio?[SOLVED]

Asked 213 views2 answers

For no explicable reason, it just crashes. No loops or anything obvious

--[[SETTINGS]]--
 
damage = 30 -- Set this to how much damage you want the gun to do
 
 
 
RPM = 600 -- How many rounds a minute can the gun fire?
ammo = 10 -- How much ammo in a clip do you want?
ammostore = 100 -- How many stored rounds do you want?
reload = "r" -- Change this to what button you want to use to reload
reloadtime = 2 -- Set this to how long you want to take to reload
 
--[[SETTINGS/]]--
 
-- Weld script by SCARFACIAL (I'm lazy and this is a good script)
local Welds = {}
local Handle = script.Parent:WaitForChild("Handle")
 
for _,Part in pairs(script.Parent:GetChildren()) do
        if Part:IsA("BasePart") and Part ~= Handle then
                local Weld = Instance.new("Weld")
                Weld.C1 = Part.CFrame:toObjectSpace(Handle.CFrame)
                Welds[Weld] = Part
                Part.Anchored = false
                Part.CanCollide = false
        end
end
 
Handle.Anchored = false
 
local function ApplyWelds()
        for Weld, Part in pairs(Welds) do
                Weld.Part0 = Handle
                Weld.Part1 = Part
                Weld.Parent = game:FindFirstChild("JointsService") or Handle
        end
end
 
script.Parent.AncestryChanged:connect(ApplyWelds)
--End of weld
 
 
 
local tool = script.Parent
local main = script.Parent.Main
local user
reloading = false
 
local firingrate = 60 / RPM
local canfire = true
auto = false
 
tool.Equipped:connect(function(mouse)
        print("Equipped")
user = tool.Parent
if script.Parent:FindFirstChild("ammo") == false then
print("There ain't no ammo vals! better make 'ne")
 
    local a = Instance.new("NumberValue", script.Parent)
    a.Name = "ammo"
    a.Value = ammo
    local b = Instance.new("NumberValue", script.Parent)
    b.Name = "storedammo"
    b.Value = ammostore
end
    mouse.Button1Down:connect(function()
        print("I should fire now")
        auto = true
        while auto == true do
        print("I can fire now")
        if canfire == false and reloading == false then return end
                print("I love donkeys")
        canfire = false
        if ammo > 0 then
            ammo = ammo - 1
                        print(ammo .. " shots left")
 
        -- Mah first ray evar
        local ray = Ray.new(tool.Handle.CFrame.p, (mouse.Hit.p -  tool.Handle.CFrame.p).unit*300)
        local hit, position = game.Workspace:FindPartOnRay(ray, user)
                print(position)
                                        print(ammo .. " shots left")
 
        print(hit) -- This is used for debugging
        local hitted = hit and hit.Parent and hit.Parent:FindFirstChild("Humanoid")
 
        if hitted then
            hitted:TakeDamage(damage)
 
 
 
        wait(firingrate)
        canfire = true
else
        print("You have no ammo.")
        end
 
mouse.Button1Up:connect(function()
        print("Button is up")
        auto = false
end)


 --Offending function
        local CAS = game:GetService("ContextActionService")
 
function reloadGun(name, state, input)
        if state == Enum.UserInputState.Begin then
                print("key has been pressed")
                        if ammo < 10 and ammostore > 10 then
                        reloading = true
                        local difference = ammo - 10
                        ammo = ammo + difference
                        print(ammo .. " " .. difference)
                        ammostore = ammostore - difference
                        print(ammostore)
                        wait(reloadtime)
                        reloading = false
               
                else
                        print("No ammo left! Da fuzz home boi! Da fuzz")
                end
end
end
 
CAS:BindAction("ReloadGun", reloadGun, true, Enum.KeyCode.R, Enum.KeyCode.ButtonX)
 
 
 
    end
       
end
   
end
 
 
 
)
end
)
AnonymousEdited

2 comments

0

You do know that most of us are too lazy to fix scripts as long as these.

Anonymous#
0

Especially when it is not tabbed correctly and looks like a mess at places...

Anonymous#

2 answers

✓ Accepted answer

This code is messy and complicated. That hides a lot of issues. You have to clean it up first!

First: tab your code correctly. It makes it immensely easier to understand.

Second: pull functions out to where they belong. Functions defined inside of functions (or loops, as it were also) are only desirable when you actually need it. I'm talking about reloadGun, and Button1Up.

Third: eliminate == false and == true. It's harder to read -- just use if thing and if not thing. It's also wrong in the case of FindFirstChild -- FindFirstChild produces nil, not false when the object doesn't exist.

Fourth: separate your constants (reloadtime) from your variables (ammo). I suggest naming your constants IN_ALL_UPPERCASE to distinguish them. reload isn't used.

Fifth: remove and move unnecessary definitions. main isn't used anywhere. user is only used inside the Equipped event, so define it there, not outside. auto, too.

Another problem is that you are repeatedly binding to the ContextActionService. You only should do that once -- and it should be done separately from everything else. It has no business being nested into the Button1Down event.

Here's what I have at this point (one bug fixed, much cleaner, two problematic things removed... so far):

--[[SETTINGS]]--

DAMAGE = 30 -- Set this to how much damage you want the gun to do
RPM = 600 -- How many rounds a minute can the gun fire?
FIRING_RATE = 60 / RPM
RELOAD_TIME = 2 -- Set this to how long you want to take to reload

--[[SETTINGS/]]--

ammo = 10 -- How much ammo in a clip do you want?
ammostore = 100 -- How many stored rounds do you want?

-- <WELD STUFF>

local tool = script.Parent

reloading = false

function reloadGun(name, state, input)
	if state == Enum.UserInputState.Begin then
		print("key has been pressed")
		if ammo < 10 and ammostore > 10 then
			reloading = true
			local difference = ammo - 10
			ammo = ammo + difference
			print(ammo .. " " .. difference)
			ammostore = ammostore - difference
			print(ammostore)
			wait(RELOAD_TIME)
			reloading = false
		else
			print("No ammo left! Da fuzz home boi! Da fuzz")
		end
	end
end
-- Listen for reload action
local CAS = game:GetService("ContextActionService")
CAS:BindAction("ReloadGun", reloadGun, true, Enum.KeyCode.R, Enum.KeyCode.ButtonX)

local canfire = true
tool.Equipped:connect(function(mouse)
	print("Equipped")
	local user = tool.Parent
	if not script.Parent:FindFirstChild("ammo") then
		print("There ain't no ammo vals! better make 'ne")
		local a = Instance.new("NumberValue", script.Parent)
		a.Name = "ammo"
		a.Value = ammo
		local b = Instance.new("NumberValue", script.Parent)
		b.Name = "storedammo"
		b.Value = ammostore
	end
	local auto = false
	mouse.Button1Up:connect(function()
		print("Button is up")
		auto = false
	end)
	mouse.Button1Down:connect(function()
		print("I should fire now")
		auto = true
		while auto do
			print("I can fire now")
			if not canfire and not reloading then return end
			print("I love donkeys")
			canfire = false
			if ammo > 0 then
				ammo = ammo - 1
				print(ammo .. " shots left")

				-- Mah first ray evar
				local ray = Ray.new(tool.Handle.CFrame.p, (mouse.Hit.p -  tool.Handle.CFrame.p).unit*300)
				local hit, position = game.Workspace:FindPartOnRay(ray, user)
				print(position)
				print(ammo .. " shots left")

				print(hit) -- This is used for debugging
				local hitted = hit and hit.Parent and hit.Parent:FindFirstChild("Humanoid")

				if hitted then
					hitted:TakeDamage(DAMAGE)
					wait(FIRING_RATE)
					canfire = true
				else
					print("You have no ammo.")
				end
			end
		end
	end)
end)

Problems

This line looks wrong:

if not canfire and not reloading then return end

You should probably continue if not reloading, not give up. Of course, it seems like it makes more sense to just add these to the while.

The else for if hitted then is completely off. You have ammo -- you just didn't hit a person. canfire should be set whether or not you hit someone. Similarly, the wait has to be out of the if.

canfire, though, is only necessary to prevent multiple loops from happening at the same time. The waits already stop this one. Why not just stop the loop from happening in the first place if there's already one going on -- which you know from auto?

Technically, auto needs to be a bit stronger because of the delay before checking mouse up. You could use a local variable and connect the MouseUp inside (but remember to disconnect it!) or you can use an identifier to know this is the right loop. Here's what I have now:

-- <weld stuff before me>
local tool = script.Parent

reloading = false

function reloadGun(name, state, input)
	if state == Enum.UserInputState.Begin then
		print("key has been pressed")
		if ammo < 10 and ammostore > 10 then
			reloading = true
			local difference = ammo - 10
			ammo = ammo + difference
			print(ammo .. " " .. difference)
			ammostore = ammostore - difference
			print(ammostore)
			wait(RELOAD_TIME)
			reloading = false
		else
			print("No ammo left! Da fuzz home boi! Da fuzz")
		end
	end
end
-- Listen for reload action
local CAS = game:GetService("ContextActionService")
CAS:BindAction("ReloadGun", reloadGun, true, Enum.KeyCode.R, Enum.KeyCode.ButtonX)

local firing = false
tool.Equipped:connect(function(mouse)
	print("Equipped")
	local user = tool.Parent
	if not script.Parent:FindFirstChild("ammo") then
		print("There ain't no ammo vals! better make 'ne")
		local a = Instance.new("NumberValue", script.Parent)
		a.Name = "ammo"
		a.Value = ammo
		local b = Instance.new("NumberValue", script.Parent)
		b.Name = "storedammo"
		b.Value = ammostore
	end
	mouse.Button1Up:connect(function()
		print("Button is up")
		firing = false
	end)
	mouse.Button1Down:connect(function()
		if firing then
			return -- The mouse is already down, and a loop is still running
		end
		local me = {}
		firing = me
		while firing == me and not reloading and ammo > 0 do
			-- Stop if reloading or someone else is firing, or I run out of ammo
			ammo = ammo - 1
			print(ammo .. " shots left")

			local ray = Ray.new(tool.Handle.CFrame.p, (mouse.Hit.p -  tool.Handle.CFrame.p).unit*300)
			local hit, position = game.Workspace:FindPartOnRay(ray, user)
			print(position)

			print(hit) -- This is used for debugging
			local hitted = hit and hit.Parent and hit.Parent:FindFirstChild("Humanoid")
			if hitted then
				hitted:TakeDamage(DAMAGE)
			end
			wait(FIRING_RATE)
		end
	end)
end)

reloadGun doesn't look right. If you just want to move ammostore into ammo, do that:

			ammo = ammo + ammostore
			ammostore = 0

If you only want to move MAX_AMMO, then that just looks like this:

			 -- move the lesser of what you have and what you want
			local move = math.min(ammostore, MAX_AMMO)
			ammo = ammo + move
			ammostore = ammostore - move

Finished Clean Up

--[[SETTINGS]]--

DAMAGE = 30 -- Set this to how much damage you want the gun to do
RPM = 600 -- How many rounds a minute can the gun fire?
FIRING_RATE = 60 / RPM
RELOAD_TIME = 2 -- Set this to how long you want to take to reload
MAX_AMMO = 10
--[[SETTINGS/]]--

ammo = MAX_AMMO -- How much ammo in a clip do you want?
ammostore = 100 -- How many stored rounds do you want?

-- <WELD STUFF>

local tool = script.Parent

reloading = false

function reloadGun(name, state, input)
	if state == Enum.UserInputState.Begin then
		print("reload key has been pressed")
		if ammostore > 0 then
			reloading = true
			 -- move the lesser of what you have and what you want
			local move = math.min(ammostore, MAX_AMMO)
			ammo = ammo + move
			ammostore = ammostore - move
			wait(RELOAD_TIME)
			reloading = false
		end
	end
end
-- Listen for reload action
local CAS = game:GetService("ContextActionService")
CAS:BindAction("ReloadGun", reloadGun, true, Enum.KeyCode.R, Enum.KeyCode.ButtonX)

local firing = false
tool.Equipped:connect(function(mouse)
	print("Equipped")
	local user = tool.Parent
	if not script.Parent:FindFirstChild("ammo") then
		print("There ain't no ammo vals! better make 'ne")
		local a = Instance.new("NumberValue", script.Parent)
		a.Name = "ammo"
		a.Value = ammo
		local b = Instance.new("NumberValue", script.Parent)
		b.Name = "storedammo"
		b.Value = ammostore
	end
	mouse.Button1Up:connect(function()
		print("Button is up")
		firing = false
	end)
	mouse.Button1Down:connect(function()
		if firing then
			return -- The mouse is already down, and a loop is still running
		end
		local me = {}
		firing = me
		while firing == me and not reloading and ammo > 0 do
			-- Stop if reloading or someone else is firing, or I run out of ammo
			ammo = ammo - 1
			print(ammo .. " shots left")

			local ray = Ray.new(tool.Handle.CFrame.p, (mouse.Hit.p -  tool.Handle.CFrame.p).unit*300)
			local hit, position = game.Workspace:FindPartOnRay(ray, user)
			print(position)

			print(hit) -- This is used for debugging
			local hitted = hit and hit.Parent and hit.Parent:FindFirstChild("Humanoid")
			if hitted then
				hitted:TakeDamage(DAMAGE)
			end
			wait(FIRING_RATE)
		end
	end)
end)
AnonymousEdited Permalink

2 comments

0

I had to abridge the script to remove the welding to stay under the SH character limit.

Anonymous#
0

Congradulations. You have been promoted from amateur mortal to God, how do you feel? But seriously, thanks

Anonymous#

you're problem begins on line 69 [;)]

You made a while loop loop without adding a wait, Roblox studio is so focused on running the script, It has no time to load your character, or anything for that matter.

Try adding a wait:

        while auto == true do
	wait(1/30)
        print("I can fire now")
AnonymousPermalink

2 comments

0

Alright, I patched that, I saw the wait that was meant to add the delay was not being done, and I also fixed the gun just repeat firing. Added a few prints, and have come to the conclusion the entire script is unstable...

Anonymous#
0

There's a lot more wrong with the script than this.

Anonymous#

This discussion is preserved as it was. New answers, comments, and votes are closed.