Making my 'Equip to holster' script more optimized/clean?

Help and Feedback Code Review
,
#1

This is a script that finds a weapon with a tag, duplicates its handle, and places it into the holster described in an attribute of that gun

I haven’t found a way to do an attribute for the offset though! Which means I have to manually check in the weld function what the guns name is to set the offset which is just extra work! But to get the orientation correct I must use the :FromOrientation() Function so I haven’t been able to find a better way, any help with this would be awesome :blush:

local collectionservice = game:GetService("CollectionService")

local function equiptoholster(tool, character, holster, offset)
	local model = tool.Handle:Clone()
	model.Name = tool.Name .." viewmodel"
	model.Parent = holster
	
	local cframe = CFrame.new(Vector3.new(0,2,0.75)) * CFrame.Angles(math.rad(-90), math.rad(0), math.rad(0))

	local weld = Instance.new("Weld")
	weld.Part0 = model
	weld.Part1 = holster
	if tool.Name == "1884 pin shot double barrel" then
		cframe =  CFrame.new(0.5,3.2,0) * CFrame.fromOrientation(math.rad(-90),math.rad(90),0)
	elseif tool.Name == "1884 missouri river hawkin" then
		cframe =  CFrame.new(0, -0.5, 3) * CFrame.fromOrientation(0,0,0)
	end
	weld.C1 = cframe
	weld.Parent = model
end

local function findHolster(v, character)
	local holsterr = character:WaitForChild("holsterr")
	local holsterl = character:WaitForChild("holsterl")
	local backholster = character:WaitForChild("backholster")
	
	if v:GetAttribute("holster") == "holsterr" then
		
		equiptoholster(v, character, holsterl)
		
	elseif v:GetAttribute("holster") == "holsterl" then
		
		equiptoholster(v, character, holsterr)
		
	elseif v:GetAttribute("holster") == "backholster" then
		
		equiptoholster(v, character, backholster)
		
	end
end

local function destroyViewmodel(v, character)
	if character:FindFirstChild(v.Name .." viewmodel", true) then
		character:FindFirstChild(v.Name .." viewmodel", true):Destroy()
	end
end

collectionservice:GetInstanceAddedSignal("equiptoholster"):Connect(function(v)
	local player = v.Parent.Parent
	local character = player.Character
	local humanoid = character:WaitForChild("Humanoid")
	local equipanimtrack = humanoid:LoadAnimation(v.equip)
	
	if not character:FindFirstChild(v.Name .." viewmodel") then
		
		findHolster(v, character)
		
	end
	
	player.Backpack.ChildRemoved:Connect(function(child)
		if child.Name == v.Name then
			equipanimtrack:Stop()
			destroyViewmodel(v, character)
		end
	end)
	
	v.Equipped:Connect(function()
		v.Handle.holster:Play()
		equipanimtrack:Play()
		destroyViewmodel(v, character)
	end)
	
	v.Unequipped:Connect(function()
		equipanimtrack:Stop()
		findHolster(v, character)
	end)
end)
#2

Is this script in every weapon? You could avoid repeating yourself by using a ModuleScript called by all weapons and save some space. I don’t know how to optimize the code itself (I’m not new, just bad at programming), but all two times I’ve made holsterable weapons I added & removed an accessory to the player’s character.

#3

This is currently in serverscriptservice
Editing this now this seems a little passive aggressive :sob: I appreciate you responding with some ideas

2 Likes
#4

Here’s a quick tip!

Instead of individually checking for weapon names, we can make it look cleaner, and make adding new weapons easier by using a table:

local WeaponHolsterCFrames = {
	["1884 pin shot double barrel"] = CFrame.new(0.5,3.2,0) * CFrame.fromOrientation(math.rad(-90),math.rad(90),0);
	["1884 missouri river hawkin"] = CFrame.new(0, -0.5, 3) * CFrame.fromOrientation(0,0,0);
}
local DefaultHolsterCFrame = CFrame.new(Vector3.new(0,2,0.75)) * CFrame.Angles(math.rad(-90), math.rad(0), math.rad(0))

-- instead of the if statements:
weld.C1 = WeaponHolsterCFrames[tool.Name] or DefaultHolsterCFrame
1 Like
#5

I feel like the if statements are redundant here. Instead, why not just do:

equiptoholster(v, character, v:GetAttribute("holster")
1 Like
#6

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.