Optimizing Code (Chat GBT vs My code)

Help and Feedback Scripting Support
#1

Hello, I’ve written some code for a train system and I’ve copied it into Chat GBT and asked it to make the code more performant and optimized but, I’m wondering if anyone here on the Devforum would know what code is more performant and optimized, my code or Chat GBTs code.

If anyone could tell me which code is more performant and optimized then please let me know! If anyone would like to re write it into a better version then that’ll also be good! Any other tips and tricks are appreciated!

Does anyone have any suggestions to writing optimized code?

My Code 
local Trains = {}

function SetOrder(Train)
	local Cars = Train:GetChildren()

	table.sort(Cars, function(A, B)
		return A.PrimaryPart.Position.X > B.PrimaryPart.Position.X
	end)

	for Index, Car in Cars do
		Car.Name = Index
	end
end

return function(Model)
	if not Trains[Model] then
		local Train = {}

		Train.Unit = nil
		Train.CardinalDirection = nil
		Train.Connections = {}
		
		function Train:DeleteTrain()
			Model:Destroy()
			table.clear(Train)
		end

		function Train.Couple(North, South)
			if North:GetAttribute("Coupled") == false and South:GetAttribute("Coupled") == false then
				local NewTrain = Instance.new("Model", workspace.Trains)

				local Train1 = North.Parent.Parent.Parent.Parent:GetChildren()
				local Train2 = South.Parent.Parent.Parent.Parent:GetChildren()

				North:SetAttribute("Coupled", true)
				South:SetAttribute("Coupled", true)

				Trains[NewTrain] = Train
				table.clear(Trains[Train1])
				table.clear(Trains[Train2])

				for Index, Car in table.move(Train1, 1, #Train1, #Train2 + 1, Train2) do
					Car.Parent = NewTrain
				end
			end
		end
		
		function Train.CouplerTouched(Coupler)
			Train.Connections[Coupler] = Coupler.Touched:Connect(function(Hit)
				if Hit.Parent.Name == "Couplers" then
					if Hit.Name == "North" and Coupler.Name == "South" then
						Train.Couple(Hit, Coupler)
					elseif Hit.Name == "South" and Coupler.Name == "North" then
						Train.Couple(Coupler, Hit)
					end

					if Train.Connections[Coupler] then
						Train.Connections[Coupler]:Disconnect()
					end

					if Train.Connections[Hit] then
						Train.Connections[Hit]:Disconnect()
					end
				end
			end)
		end
		
		function Train.SetUpCouplers()
			for Index, Unit in Model:GetChildren() do
				local NorthCoupled = Unit.Miscellaneous.Couplers.North:GetAttribute("Coupled")
				local SouthCoupled = Unit.Miscellaneous.Couplers.South:GetAttribute("Coupled")

				if NorthCoupled == false and SouthCoupled == true then
					Train.CouplerTouched(Unit.Miscellaneous.Couplers.North)
				elseif NorthCoupled == true and SouthCoupled == false then
					Train.CouplerTouched(Unit.Miscellaneous.Couplers.South)
				elseif NorthCoupled == false and SouthCoupled == false then
					Train.CouplerTouched(Unit.Miscellaneous.Couplers.North)
					Train.CouplerTouched(Unit.Miscellaneous.Couplers.South)
				end
			end
		end
		
		function Train.DriveMode(Mode)
			Mode = (Mode == 1 and "Forward") or (Mode == 2 and "Off") or (Mode == 3 and "Reverse") or (Mode == 4 and "Isolate")

			Train.Unit:SetAttribute("DriveMode_" .. Train.CardinalDirection, Mode)
		end

		function Train.UpdateStuff(Unit, CardinalDirection)
			Train.Unit = Unit
			Train.CardinalDirection = CardinalDirection
		end

		Trains[Model] = Train

		return Train
	elseif Trains[Model] then
		return Trains[Model]
	end
end
Chat GBTs Code 
local Trains = {}

local function CoupleTrains(North, South)
	local NewTrain = Instance.new("Model", workspace.Trains)

	local Train1 = North.Parent.Parent.Parent.Parent:GetChildren()
	local Train2 = South.Parent.Parent.Parent.Parent:GetChildren()

	for Index, Car in ipairs(Train1) do
		Car.Parent = NewTrain
	end

	North:SetAttribute("Coupled", true)
	South:SetAttribute("Coupled", true)

	Trains[NewTrain] = {}
end

local function CouplerTouched(Coupler, Train)
	Train.Connections[Coupler] = Coupler.Touched:Connect(function(Hit)
		if Hit.Parent.Name == "Couplers" then
			if Hit.Name ~= Coupler.Name then
				CoupleTrains(Hit, Coupler)
			end

			if Train.Connections[Coupler] then
				Train.Connections[Coupler]:Disconnect()
			end

			if Train.Connections[Hit] then
				Train.Connections[Hit]:Disconnect()
			end
		end
	end)
end

local function SetUpCouplers(Train)
	for _, Unit in ipairs(Train:GetChildren()) do
		local NorthCoupled = Unit.Miscellaneous.Couplers.North:GetAttribute("Coupled")
		local SouthCoupled = Unit.Miscellaneous.Couplers.South:GetAttribute("Coupled")

		if not NorthCoupled and SouthCoupled then
			CouplerTouched(Unit.Miscellaneous.Couplers.North, Train)
		elseif NorthCoupled and not SouthCoupled then
			CouplerTouched(Unit.Miscellaneous.Couplers.South, Train)
		elseif not NorthCoupled and not SouthCoupled then
			CouplerTouched(Unit.Miscellaneous.Couplers.North, Train)
			CouplerTouched(Unit.Miscellaneous.Couplers.South, Train)
		end
	end
end

local function DriveMode(Train, Mode)
	local driveModes = { [1] = "Forward", [2] = "Off", [3] = "Reverse", [4] = "Isolate" }
	local modeStr = driveModes[Mode] or "Off"
	Train.Unit:SetAttribute("DriveMode_" .. Train.CardinalDirection, modeStr)
end

local function SetOrder(Train)
	local Cars = Train:GetChildren()
	table.sort(Cars, function(A, B)
		return A.PrimaryPart.Position.X > B.PrimaryPart.Position.X
	end)

	for Index, Car in ipairs(Cars) do
		Car.Name = tostring(Index)
	end
end

return function(Model)
	if not Trains[Model] then
		local Train = {
			Unit = nil,
			CardinalDirection = nil,
			Connections = {}
		}

		function Train:DeleteTrain()
			Model:Destroy()
			Trains[Model] = nil
		end

		Trains[Model] = Train
		SetOrder(Model)
		SetUpCouplers(Model)

		return Train
	else
		return Trains[Model]
	end
end

Thanks!

6 Likes
#2

Just from looking at it right away, chat GPTs code look more organized in my opinion

1 Like
#3

To me it just looks silly, lol.

Would you have any idea which one looks more optimized?

2 Likes
#4

Probably yours, considering that chat GPT used more local functions instead of global ones

1 Like
#5

Oh okay, thanks!

I’ll just see what others say plus I’m also reading a thousand things on optimizing code.

2 Likes
#6

In terms of memory, GPT has more efficient code. You create new functions for every train, while GPT creates most of the functions only once. IDK about the speed though.

1 Like
#7

Just a quick read through but ChatGPT’s code is more optimized because it puts the functions outside the main class and also changes them to local functions which will increase performance.

3 Likes
#8

I asked my brother just now, who does app development, and he said that yours looks easier to understand but chatGPTs is more efficient

1 Like
#9

Your original code had static methods attached to the train object which can be accessed via the returned table whereas ChatGPT seems to break it into local functions, which though giving a small performance boost doesn’t give access to these methods from the train object

1 Like
#10

What way would be better or what way would you use personally? My way or ChatGPTs way.

Is it a performance boost worth going for or not really?

What would the effect be if I had the functions outside of the train thing but put a reference to the functions inside of the trains table? Like Train.ThisThing = ThisThingFunction

Do you have any other ideas that would be helpful?

1 Like
#11

Oh cool, thanks!

I do think my look better and I suppose ChatGBT way is probably more efficient especially since its smarter than me lol. I also did think about the whole the more trains there are the more repeated functions and tables there are.

Do you or your brother have any other ideas that would be helpful?

1 Like
#12

Micro optimization isn’t really worth it, unless you are doing something performant heavy very frequently (like every frame). Just did some benchmarks in normal Lua (I don’t have access to Roblox Studio atm) and it provides a feeble 3e-06 (0.000003) seconds boost :skull: which won’t even be noticeable.

2 Likes
#13

GPT did do a good job at converting the methods of Train into global functions. Although it didn’t fully finish the job when it kept Train:DeleteTrain() as a locally created function.

What it should have done is made a dictionary with APIs which get populated into a new Train instance upon creation.

local trainMethods = {}

function trainMethods.DeleteTrain(self)
	...
end

function trainMethods.Couple(self, North South)
	...
end

function trainMethods.CouplerTouched(self, Coupler)
	...
end

function trainMethods.SetupCouplers(self)
	...
end

function trainMethods.DriveMode(self)
	...
end

function trainMethods.UpdateStuff(self)
	...
end

return function(Model)
	local Train = {
		Unit = nil,
		CardinalDirection = nil,
		Connections = {}
	}

	for i, v in trainMethods do
		Train[i] = v
	end

	return Train
end

In the above code, I took all of your functions and put them under a global table. When a new Train instance is created, a reference to each API will be added to the new Train.

Anywhere you referenced Train would now reference (for example) self. You can also reformat this like below and Lua will automatically set a global “self” variable in the function scope.

function trainMethods:Couple()
print(self.Name) 
end

This will not only keep memory low because it simply references shared API, but it will also help organize your code and help you develop a better understanding of how your system works.

1 Like
#14

None it would be the same, the extra performance boost is coming from that fact that you don’t have to index into a table to call a function but rather call it directly from the local scope

1 Like
#15

Lol I checked how long it took using the tick method and the number kept changing something it was like 7.00007598759855 (I just made the decimal numbers up but you get the point).

I don’t care too much about that mini performance boost by the time the games out a lot more terrible devices will probably not be compatible with Roblox meaning I don’t have to worry too much about bad devices, people will just have to deal with what they have if they’d like to play the game. I do suppose it could add up with the entire train system being more optimized though resulting in a big boost or a more noticeable boost.

#16

Your approach is even slower as now not only do you have to index into a table but you are copying over functions to another table.

#17

When the player exits the drivers seat should it delete the table completely and then create a new one when the player enters the seat again or should the table not be access unless a player is in the vehicle? If option 2 then what would be the best way to find the table later on?

I’m pretty sure with your code I wouldn’t be able to find that Created train object again unless I’ve stored it in another table somewhere.

Another thing, when you loop it couldn’t you just have instead another thing stored in the table that references to the train methods and then when I want to call it just go Train.TrainMethods:Couple() instead of Train:Couple()

#18

May I know what is the use case for the module?

#19

Unfortunately no, since him and I don’t really specialize in Roblox studios programming language

#20

For train stuff like horns, coupling, pantographs, and other things.