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

Airline Baggage script not working. Anyway to fix this?

Asked 195 views1 answers

This script is not currently working, it has the Add Tag and the Weld 2.1 installed in it. Is this correct?

script.Parent.ClickDetector.MouseClick:connect(function(p)
	local tool = p.Backpack:findFirstChild("Case")
	if (tool ~= nil) then
		local tooleq = tool:findFirstChild("EquippedV")
		if (tooleq ~= nil) then
			if tooleq.Value == false then
				local model = Instance.new("Model")
				model.Name = "Case"
				local decs = tool:GetChildren()
				for _,v in pairs (decs) do
					if v:IsA("BasePart") then
						v.Parent = model
						v.Anchored = true
						v.CanCollide = true
					end	
				end
				tool:destroy()
				model.Parent = game.Workspace
				model:MoveTo(script.Parent.Position)
		
				function TransformModel(objects, center, new, recurse)
					for _,object in pairs(objects) do
						if object:IsA("BasePart") then
							object.CFrame = new:toWorldSpace(center:toObjectSpace(object.CFrame))
						end
						if recurse then
							TransformModel(object:GetChildren(), center, new, true)
						end
					end
				end

				local center = model:GetModelCFrame()
				TransformModel(
					model:GetChildren(),
					center,
					center * CFrame.Angles(0,math.rad(90),math.rad(90)),
					true
				)	
		
				local cd = Instance.new("ClickDetector")
				cd.Parent = model
				local tags = script.Parent.AddTag:Clone()
				tags.Parent = model
				tags.Disabled = false
				local welds = script["Weld 2.1"]:clone()
				welds.Parent = model
				welds.Disabled = false
				script.Parent.ClickDetector.MaxActivationDistance = 0
				script.Parent.Transparency = 1
				wait(15)
				script.Parent.ClickDetector.MaxActivationDistance = 32
				script.Parent.Transparency = 0.8
			else
				local casecolour = tool.Handle.BrickColor
				local newcase = game.ServerStorage.Case:Clone()
				local model = Instance.new("Model")
				model.Name = "Case"
				local decs = newcase:GetChildren()
				for _,v in pairs (decs) do
					if v:IsA("BasePart") then
						v.Parent = model
						v.Anchored = true
						v.CanCollide = true
						if v.Name == "Handle" then
							v.BrickColor = casecolour
						end
					end	
				end
				newcase:destroy()
				model.Parent = game.Workspace
				model:MoveTo(script.Parent.Position)
				local tool = p.Backpack:findFirstChild("Case")
				if (tool ~= nil) then
					tool:remove()
				end
		
				function TransformModel(objects, center, new, recurse)
					for _,object in pairs(objects) do
						if object:IsA("BasePart") then
							object.CFrame = new:toWorldSpace(center:toObjectSpace(object.CFrame))
						end
						if recurse then
							TransformModel(object:GetChildren(), center, new, true)
						end
					end
				end

				local center = model:GetModelCFrame()
				TransformModel(
					model:GetChildren(),
					center,
					center * CFrame.Angles(0,math.rad(90),math.rad(90)),
					true
				)	
		
				local cd = Instance.new("ClickDetector")
				cd.Parent = model
				local tags = script.Parent.AddTag:Clone()
				tags.Parent = model
				tags.Disabled = false
				local welds = script["Weld 2.1"]:clone()
				welds.Parent = model
				welds.Disabled = false
				script.Parent.ClickDetector.MaxActivationDistance = 0
				script.Parent.Transparency = 1
				wait(15)
				script.Parent.ClickDetector.MaxActivationDistance = 32
				script.Parent.Transparency = 0.8
			end
		end
	end
end)
Anonymous

2 comments

2

What errors do you get? What output, if any, do you get?

Anonymous#
1

It just doesn't work; the bag doesn't appear.

Anonymous#

1 answer

This script needs some massive cleanup. Short, easy-to-understand code is good code. It's also code that has fewer bugs.

Naming

ROBLOX now uses the convention that methods on objects should be :PascalCase(). That is, :FindFirstChild instead of :findFirstChild(), and :Destroy() instead of :destroy(), :Remove() instead of :remove(). This isn't mandatory (though it might be one day), but it's ROBLOX's preferred style.

Use meaningful, human-readable names! player instead of p!

Conditions

It's not idiomatic to use () around a condition in Lua. It's unnecessary messiness.

It's not idiomatic to use ~= nil. You can just use if tool then.

Similarly, instead of if tooleq.Value == false then, it's much easier to read and interpet if not tooleq.Value then. You should probably use a better name than tooleq so it actually reads like a human would write it, e.g., if not equipped.Value then.

You can make it even easier to understand by flipping the then and else blocks, so that the conditions reads if equipped.Value then -- negatives are harder to understand.

Functions

The function TransformModel doesn't belong in the middle of three ifs and an event connection. You want to define functions separately as much as possible.

A good illustration of why you want that is in this script. You've repeated TransformModel twice.

Move it out.

Spacing

pairs is a function. Standard spacing rules suggest you should use pairs(decs) not pairs (decs).

You should have spaces after commas. _, v instead of _,v.

Don't Repeat Yourself!

Repeating yourself is bad. It means you have more code that you have to understand.

It also means if you have a bug in one place, you have a bug everywhere you copy-pasted.

Don't copy-paste code. Don't repeat yourself.

Lines 7 to 52 are repeated -- they are almost exactly the same. Only the different parts should be "repeated" -- lines 18+ are exactly the same.

It could instead look like this:

			local model = Instance.new("Model")
			model.Name = "Case"
			local newcase
			if equipped.Value then
				newcase = game.ServerStorage.Case:Clone()
				newcase.Handle.BrickColor = tool.Handle.BrickColor
				local tool = player.Backpack:FindFirstChild("Case")
				if tool then
					tool:remove()
				end
			else
				newcase = tool
			end
			local decs = newcase:GetChildren()
			for _, v in pairs(decs) do
				if v:IsA("BasePart") then
					v.Parent = model
					v.Anchored = true
					v.CanCollide = true
				end 
			end
			newcase:Destroy()

However this code is not right: newcase:Destroy() would nullify all of the work done to create newcase and set its children's properties.

I'm guessing that is a typo.

You're shadowing local tool in this code. It's not even necessary, since it's the same as the old tool. Again, don't repeat yourself.

The :Destroy() is similarly redundant, since you'll handle it with newcase:Destroy() in this rewrite.

Rewritten

function TransformModel(objects, center, new, recurse)
	for _,object in pairs(objects) do
		if object:IsA("BasePart") then
			object.CFrame = new:toWorldSpace(center:toObjectSpace(object.CFrame))
		end
		if recurse then
			TransformModel(object:GetChildren(), center, new, true)
		end
	end
end

script.Parent.ClickDetector.MouseClick:connect(function(player)
	local tool = player.Backpack:FindFirstChild("Case")
	if tool then
		local equipped = tool:FindFirstChild("EquippedV")
		if equipped then
			local model = Instance.new("Model")
			model.Name = "Case"
			local newcase
			if equipped.Value then
				newcase = game.ServerStorage.Case:Clone()
				newcase.Handle.BrickColor = tool.Handle.BrickColor
			else
				newcase = tool
			end
			local decs = newcase:GetChildren()
			for _, v in pairs(decs) do
				if v:IsA("BasePart") then
					v.Parent = model
					v.Anchored = true
					v.CanCollide = true
				end 
			end
-----------------------------------------------------------------------------
			newcase:Destroy() -- Something's not right here
-----------------------------------------------------------------------------
			model.Parent = game.Workspace
			model:MoveTo(script.Parent.Position)

			local center = model:GetModelCFrame()
			TransformModel(
				model:GetChildren(),
				center,
				center * CFrame.Angles(0, math.rad(90), math.rad(90)),
				true
			)

			local cd = Instance.new("ClickDetector")
			cd.Parent = model
			local tags = script.Parent.AddTag:Clone()
			tags.Parent = model
			tags.Disabled = false
			local welds = script["Weld 2.1"]:Clone()
			welds.Parent = model
			welds.Disabled = false
			script.Parent.ClickDetector.MaxActivationDistance = 0
			script.Parent.Transparency = 1
			wait(15)
			script.Parent.ClickDetector.MaxActivationDistance = 32
			script.Parent.Transparency = 0.8
		end
	end
end)
AnonymousEdited Permalink

5 comments

0

What do you think I should do with the newcase:Destroy()?

Anonymous#
0

What did you want this script to do? Compare that to what it's actually doing.

Anonymous#
0

I want the baggage to appear on this script's parent: BagPosition but it's not appearing at all.

Anonymous#
0

I'm assuming that bag you want to appear is "newcase". If you want it to appear, do you have any reason to Destroy it?

Anonymous#
0

Yes, I believe that was a mistake. The script is quite old; I gave up hope with it and now I'd like to try and make it work.

Anonymous#

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