Could someone help simplify this please?

Help and Feedback Code Review
,
1

Here are the functions:

image
image987×424 24.8 KB

Code 
local function temporarilyMoveOneintoTwo(instance, newParent)
	instanceParentTable[instance] = instance.Parent
	instance.Parent = newParent
end

local function temporarilyChangeInputColor(input, newColor)
	inputColorTable[input] = input.Color
	input.Color = newColor
end

local function changeCylinderAndConstraintColors(cylinder, color3:Color3, brickColor:BrickColor)
	
	temporarilyChangeInputColor(cylinder, color3)

	for _, constraint in ipairs(inputCylinderReturnArrayOfConstraints[cylinder]) do		
		temporarilyChangeInputColor(constraint, brickColor)
	end
	
end

local function turnCylindersAndConstraintsOrangeFromJoint(joint)

	local cylindersArray = inputJointReturnArrayOfCylinders[joint]

	if cylindersArray ~= nil then
		for _, cylinder in ipairs(cylindersArray) do
			changeCylinderAndConstraintColors(cylinder, Color3.new(0.812, 0.377, 0.142), BrickColor.new("Flame reddish orange"))
		end
	end

end

And here’s the actual code:

image
image1173×504 34.3 KB

Code 
if inputJointReturnJoint[castInstance] ~= nil then -- joint

	-- move into red
	temporarilyMoveOneintoTwo(castInstance, redModel) -- joint

	-- move into orange
	for _, displayPart in ipairs(blockReturnsArrayOfDisplayParts[castInstance]) do -- displayParts	
		temporarilyMoveOneintoTwo(displayPart, orangeModel)
	end
	temporarilyMoveOneintoTwo(inputJointReturnUnWelded[castInstance], orangeModel) -- unWelded
	turnCylindersAndConstraintsOrangeFromJoint(castInstance) -- cylinders and constraints

	-- move into yellow
	for _, part in ipairs(blockReturnsArrayOfBlocks[castInstance]) do -- every part the joint is welded to
		temporarilyMoveOneintoTwo(part, yellowModel)
	end

elseif inputCylinderReturnArrayOfConstraints[castInstance] ~= nil then -- cylinder

	print("cylinder")

	-- move into red
	changeCylinderAndConstraintColors(castInstance, Color3.new(1,0,0), BrickColor.Red())

else -- part

	print("part")

	-- move into red
	temporarilyMoveOneintoTwo(castInstance, redModel) -- yellow

	-- move into yellow
	for _, part in ipairs(blockReturnsArrayOfBlocks[castInstance]) do -- every part the part is welded to, also appears in joint, make into function
		temporarilyMoveOneintoTwo(part, yellowModel)
	end

end

Thanks in advance! :D

2

Make a module script and insert those functions into the said module

3

How would that cut down on duplicated code?

Also I’m already doing that :D

4

You’re already using a module script for those?

I recommend you to implement a module script for this.

5

yep, both are already in a module. the bottom code is part of a larger function that starts with a module.functionName :D

1 Like
6

In this case, you could try simplify the functions a little. ipairs and pairs are not needed anymore. You could rewrite the script to look this way:

local function temporarilyMoveOneintoTwo(instance, newParent)
	instanceParentTable[instance] = instance.Parent

	instance.Parent = newParent
end

local function temporarilyChangeInputColor(input, newColor)
	inputColorTable[input] = input.Color

	input.Color = newColor
end

local function changeCylinderAndConstraintColors(cylinder, color3 : Color3, brickColor : BrickColor)
	temporarilyChangeInputColor(cylinder, color3)

	for _, constraint in inputCylinderReturnArrayOfConstraints[cylinder] do		
		temporarilyChangeInputColor(constraint, brickColor)
	end
end

local function turnCylindersAndConstraintsOrangeFromJoint(joint)
	local cylindersArray = inputJointReturnArrayOfCylinders[joint]

	if cylindersArray then
		for _, cylinder in cylindersArray do
			changeCylinderAndConstraintColors(cylinder, Color3.new(0.812, 0.377, 0.142), BrickColor.new("Flame reddish orange"))
		end
	end
end
1 Like
7

Those are just small details though. I was thinking more of lines 192-194 and lines 211-213 being an exact copy, something could maybe be done about that. Or maybe something bigger can be done that I just don’t see that would simplify, optimize, or shorten the code.

Edit:
I can make something like this to replace the top two functions:

local function goofyFunction(inputTable, input, property, new)
	inputTable[input] = input[property]
	input[property] = new
end
1 Like
8

Yes, you can do this.

Those two here are really similar too.

9

There’s really not a way to simplify what you’re doing. Kindly ignore @Omnipotent_Corrupted as their suggestions are nothing short of redundant.

As far as I can see with the current code you’re showing, unless I get the full module to see the full scope of all the functions and how they interact; I cannot suggest a way to simplify your code.

2 Likes
10

Alright, thank you.

It’s basically all the code. Those functions in the top image are only used in the code in the lower image. If it’s as basic and as simple as it gets, that’s fine, I just had some inclining thought that something could be made shorter, and I just wasn’t seeing it. Thank you again :D

11

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