I'm afraid my code is inefficient

I got this anxiety, whenever I find a solution to a problem, I start thinking that there is a more obvious, and simpler solution.

For example, I have platforms in my game, and I check whether a player is on any of the platforms, and if they are, then I keep track of it in the object of the platform.

But I also needed to keep track of whether they left the platform, so I check if the player is on a platform, if the value I get is not nil, it means there is a platform there, so I add them to a table in that platform’s object. If the value is nil, then it means that there is no platform, and I iterate through the platform objects, and if that player is on that platform, I remove them from the table in that platform’s object.

It worked until now, but the code looks terrible.

Then I found a bug, when a player moves from one platform to another, the player won’t be removed from the previous platform’s object, since the value didn’t turn to nil, it turned from one object to another. I was thinking of a solution to this, and I thought of this.

I made another variable, where I stored the previous platform, now if the previous platform and the current platform are different, I remove the player from the previous platform’s object, and add them to the current platform’s object.

And the code became even more unreadable.

local bridgeMechanicsModule = {}

local Players = game:GetService("Players")
local bridgeObjects = require(game:GetService("ServerScriptService"):WaitForChild("bridgeObjects"))

local bridges = bridgeObjects.bridgeObjectsInfo.bridges
local bridgeType = bridgeObjects.bridgeObjectsInfo.bridgeType
local bridgeTypeColors = bridgeObjects.bridgeObjectsInfo.bridgeTypeColors
local playersInRoundList = require(game:GetService("ServerScriptService"):WaitForChild("playersInRoundList"))

local lastFoundPart = {} --Stores each player's last found part of raycasting. This is needed to check if the player is on the bridge, is not on the bridge and if the player moved from one bridge to another.

function bridgeMechanicsModule:checkForRayColission(player)
	local downcast = Ray.new(player.Character.HumanoidRootPart.CFrame.Position, Vector3.new(0,-10,0))
	local foundPart = workspace:FindPartOnRayWithWhitelist(downcast, bridges)
	if playersInRoundList[player.Name] == nil and lastFoundPart[player.Name] then --If the player isn't in round, but was on a bridge
		for index,val in ipairs(bridgeObjects.bridgeObjects) do
			if val.path == lastFoundPart[player.Name] then
				val:removePlayerFromColissions(player)
				local tempUpdateBridgeColor = coroutine.wrap(val.updateBridgeColor) --Returns that same function but in another thread
				tempUpdateBridgeColor(val) --Runs the function in another thread, with val as a parameter, because val.updateBridgeColor does not send self as a parameter automatically
			end
		end
		lastFoundPart[player.Name] = nil
		return
	elseif playersInRoundList[player.Name] ~= nil then
		if foundPart and foundPart ~= lastFoundPart[player.Name] then
			if lastFoundPart[player.Name] then
				for index,val in ipairs(bridgeObjects.bridgeObjects) do
					if val.path == lastFoundPart[player.Name] then
						val:removePlayerFromColissions(player)
						local tempUpdateBridgeColor = coroutine.wrap(val.updateBridgeColor)
						tempUpdateBridgeColor(val)
					end
				end
			end
			for index,val in ipairs(bridgeObjects.bridgeObjects) do
				if val.path == foundPart then
					val:addPlayerToColissions(player, foundPart)
					local tempUpdateBridgeColor = coroutine.wrap(val.updateBridgeColor)
					tempUpdateBridgeColor(val)
					lastFoundPart[player.Name] = foundPart
				end
			end
		elseif not foundPart and lastFoundPart[player.Name] then
			for index,val in ipairs(bridgeObjects.bridgeObjects) do
				if val.path == lastFoundPart[player.Name] then
					val:removePlayerFromColissions(player)
					local tempUpdateBridgeColor = coroutine.wrap(val.updateBridgeColor)
					tempUpdateBridgeColor(val)
				end
			end
			lastFoundPart[player.Name] = nil
		end
	end
end

function bridgeMechanicsModule:addToPlayerLastFoundPart(player) --Make sure that each player has their own lastFoundPart key, so if they leave the game without stepping on any bridge, the remove function will still find the key to remove.
	lastFoundPart[player.Name] = nil
end

function bridgeMechanicsModule:removeFromPlayerLastFoundPart(player) --Removing the lastFoundPart key of a player that left the game
	if lastFoundPart[player.Name] then
		for index, val in ipairs(bridgeObjects.bridgeObjects) do
			if val.path == lastFoundPart[player.Name] then
				val:removePlayerFromColissions(player) --If player left the game on a bridge, update the bridge
				local tempUpdateBridgeColor = coroutine.wrap(val.updateBridgeColor)
				tempUpdateBridgeColor(val)
				lastFoundPart[player.Name] = nil
			end
		end
	end
end

function bridgeMechanicsModule:run()
	print("a")
	Players.PlayerAdded:Connect(
		function(player)
			self:addToPlayerLastFoundPart(player)
		end
		
	)
	Players.PlayerRemoving:Connect(
		function(player)
			self:removeFromPlayerLastFoundPart(player)
		end)

	bridgeObjects:addBridgesToBridgeObjects()
	
	while true do
		for key,player in pairs(Players:GetPlayers()) do
			local playerCharacter = player.Character
			if playerCharacter then
				if playerCharacter:FindFirstChild("HumanoidRootPart") then
					self:checkForRayColission(player)
				end
			end
		end
		wait()
	end
end

return bridgeMechanicsModule

I wrote this 6 months ago, and then I took a long break, now I’m thinking, maybe I should start everything from scratch?

2 Likes

So yeah, I decided to rewrite the game from scratch.

1 Like

but whats the point tho,
your codde looks complex but if it works then why bother?

1 Like

Readability is important. What if he wants to take another 6 month break, comes back and is unable to read his own script? It would be difficult to make changes / add new features. And what if there’s a bug that he needs help on but no one is able to understand his code?

Also nice bump of a 2 year old thread lmao

1 Like

looks fine to me i dont see anything wrong with it

im curious how far you improved too lol what you been working on

a break from any game will be a struggle to get back to understanding how everything works but eventually you will catch onto understanding, unless your code is super bad or you greatly improved i don’t think it’d be a huge problem