Is there a way i can shorten this? (pt2)

Help and Feedback Scripting Support
#1

Basically had a situation similar to this in

but ya know, the post died so im checking if i can shorten a different part of the code thats similarly structured like the other one

Anyways any clue how to shorten this

local function reload()
	if reloading then return end
	reloading = true
	if StoredAmmo.Value <= 0 then 
		print("Out Of Ammo")
		reloading = false
		tool.Folder.Magazine.ak47_magin:Play()
		return
	end
	if game.ReplicatedStorage.Tals:FindFirstChild(gt.Value).ReloadTals:FindFirstChild(tool.strings.T1.Value) then
		local TalFolder = game:GetService("ReplicatedStorage").Tals
		local TalentMod = require(TalFolder:FindFirstChild(gt.Value).ReloadTals:WaitForChild(tool.strings.T1.Value))
		local strings = tool.strings		
		TalentMod.Yes(strings, "T1")
	elseif game.ReplicatedStorage.Tals:FindFirstChild(gt.Value).ReloadTals:FindFirstChild(tool.strings.T2.Value) then
		local TalFolder = game:GetService("ReplicatedStorage").Tals
		local TalentMod = require(TalFolder:FindFirstChild(gt.Value).ReloadTals:WaitForChild(tool.strings.T2.Value))
		local strings = tool.strings
		TalentMod.Yes(strings, "T2")
	elseif game.ReplicatedStorage.Tals:FindFirstChild(gt.Value).ReloadTals:FindFirstChild(tool.strings.T3.Value) then
		local TalFolder = game:GetService("ReplicatedStorage").Tals
		local TalentMod = require(TalFolder:FindFirstChild(gt.Value).ReloadTals:WaitForChild(tool.strings.T3.Value))
		local strings = tool.strings
		TalentMod.Yes(strings, "T3")
	elseif game.ReplicatedStorage.Tals:FindFirstChild(gt.Value).ReloadTals:FindFirstChild(tool.strings.T4.Value) then
		local TalFolder = game:GetService("ReplicatedStorage").Tals
		local TalentMod = require(TalFolder:FindFirstChild(gt.Value).ReloadTals:WaitForChild(tool.strings.T4.Value))
		local strings = tool.strings
		TalentMod.Yes(strings, "T4")
	else
		local reloadsound = tool.Sounds["AK47-RELOAD"]
		local rsound = reloadsound:Clone()
		rsound.Parent = tool.Folder.Magazine
		rsound:Play()
		wait(2.664)
		if StoredAmmo.Value >= 30 then
			local NeededAmmo = 30 - AmmoLeft.Value
			StoredAmmo.Value -= NeededAmmo
			AmmoLeft.Value += NeededAmmo
			reloading = false
		else
			local NeededAmmo = 30 - AmmoLeft.Value
			if NeededAmmo > StoredAmmo.Value then
				AmmoLeft.Value += StoredAmmo.Value
				StoredAmmo.Value = 0
				reloading = false
			end
		end
		rsound:Destroy()
	end
	reloading = false
end

(mainly the elseif statements)

#2

To be honest if the code works there is little reason to go back to it. Shorter code isn’t inherently better. Longer code can be easier to read and run faster than shorter code. Readability is generally more important than code length

But if you want shortened code here is some

local function reload()
	if reloading then return end
	reloading = StoredAmmo > 0
	
	if not reloading then 
		tool.Folder.Magazine.ak47_magin:Play()
		return
	end
    
	for i = 1, 4 do
		local str = "T" .. i
		if game.ReplicatedStorage.Tals:FindFirstChild(gt.Value).ReloadTals:FindFirstChild(tool.strings["T" .. i].Value) then
			local TalentMod = require(game.ReplicatedStorage.Tals.TalFolder:FindFirstChild(gt.Value).ReloadTals:WaitForChild(tool.strings[str].Value))	
			TalentMod.Yes(tool.strings, str)
			reloading = false
			return
		end
	end
	
	local reloadsound = tool.Sounds["AK47-RELOAD"]
	local rsound = reloadsound:Clone()
	rsound.Parent = tool.Folder.Magazine
	rsound:Play()
	task.wait(2.664)
	
	local subv = math.min(30 - AmmoLeft, StoredAmmo.Value)
	StoredAmmo.Value -= subv
	AmmoLeft.Value += subv
	
	reloading = false
	rsound:Destroy()
end
2 Likes
#3

I agree with this, although if i needed to change something i dont wanna change it 4 times, and even then the solution you came up with is way better to read than what i was doing

Just me being dumb here, how does this work?

#4

In trying to explain I realized a mistake I made in the code. Here is the fixed version.

local subv = math.min(30 - AmmoLeft.Value, StoredAmmo.Value)
StoredAmmo.Value -= subv
AmmoLeft.Value += subv

This line is really where the magic happens. It’s a bit to unpack though.

local subv = math.min(30 - AmmoLeft.Value, StoredAmmo.Value)

So subv is going to be equal to the amount of ammo that will be added to the clip and removed from the ammo storage.

First we need to calculate how many more bullets will be needed to fill the clip. That’s the 30 - AmmoLeft.Value. It’s the same as NeededAmmo in your code. So if we only did the NeededAmmo calculation the code would look like this:

local subv = 30 - AmmoLeft.Value
StoredAmmo.Value -= subv
AmmoLeft.Value += subv

That will work and will work great. Except when you don’t have enough ammo in your ammostore. Because as this is written it will just give you a full clip regardless. To fix that we need a way to make it so that if the number is bigger than what’s in the ammostore, then use ammostore instead.

There would be two ways to do this. You could use an if statement to set the subv variable to the same number as stored ammo when stored ammo is smaller like this:

local subv = 30 - AmmoLeft.Value
if subv > StoredAmmo.Value then
    subv = StoredAmmo.Value
end
StoredAmmo.Value -= subv
AmmoLeft.Value += subv

That would work. But there is another way to accomplish that. Math.min returns the smallest of the passed numbers. So if it wants to put in 30 bullets but you only have 20 in storage, 20 would be smaller so it would use that.

That brings us to the final code.

local subv = math.min(30 - AmmoLeft.Value, StoredAmmo.Value)
StoredAmmo.Value -= subv
AmmoLeft.Value += subv

Hopefully my explanation is understandable.

#5

So something like
if Ammoleft was 25
and stored ammo was 100
it would be something like

local subv math.min(5, 100)
100 -= subv
AmmoLeft.Value += subv

And if ammoleft is 5 And stored ammo is 20

local subv = math.min(25, 20)

it would choose the 20 over the 25
And add 20 to the mag instead of 25

1 Like
#6

Yup. That’s exactly what it does.

1 Like