Optimizing Check Points

I’m making an obby and saving check points using Profile Service. The problem is the code is messy and uses a lot of if statements.

I was wondering how this could be cleaned up. Also, I don’t know if I need to coroutine the second function, please let me know if I need to. By the way these two parts of the code are in a script in ServerScriptService.

image

Here is the first part:

for i, Part in pairs(Parts) do -- Iterates through all the check points in my game
	Part.Touched:Connect(function(Hit)
		local player = Players:GetPlayerFromCharacter(Hit.Parent) --Checks if the player actually exists
		if player then
			local PlayerProfile = ProfileCache[player] -- Checks to see if the player has data
			if PlayerProfile then
				if tonumber(Part.name) > PlayerProfile.Data.Stage then -- Makes sure you can't go back a stage/level
					PlayerProfile.Data.Stage = tonumber(Part.name) -- Updates your profile service data which in turn updates a leaderstat
				end
			end
		end
	end)
end

Second script (its in the same script (TELL ME IF I SHOULD USE A COROUNTINE)):

Players.PlayerAdded:Connect(function(player)
	player.CharacterAdded:Connect(function(char)
		local HRP = char:WaitForChild("HumanoidRootPart")
		local PlayerProfile = ProfileCache[player] -- Gets the players Profile Service Data
		if PlayerProfile then -- Makes sure it exists
			for i, Part in pairs(Parts) do -- Iterates through all of the checkpoints
				if tonumber(Part.Name) == PlayerProfile.Data.Stage then -- See's if the data matches the stage
					wait() -- If I don't add this it doesn't work even though I used WaitForChild()
					HRP.CFrame = Part.CFrame -- Spawns at check point
				end
			end
		end
	end)
end)
1 Like

There’s no real way for you to clean this up, already looks as good as it can get. Sometimes you do end up having code that has or needs a couple if statements to get around.

If you really hate looking at the if statements then you can use guard clauses. Guard clauses have more use in more complex code if you want to avoid scoping but developers tend to use them for small segments as well, I’ve seen and used a fair share.

My two cents are that they’re another alternate but annoying to write if your code isn’t complex. Like I said, your code is fine and changing over to guard clauses at this point is pointless but if you’re interested I’ll leave you a sample of what a conversion would look like in any case.

for _, checkpoint in ipairs(Parts) do
    checkpoint.Touched:Connect(function (hit)
        local player = Players:GetPlayerFromCharacter(hit.Parent)
        if not player then return end

        local playerProfile = ProfileCache[player]
        if not playerProfile or not (tonumber(checkpoint.Name) > playerProfile.Data.Stage) then return end

        playerProfile.Data.Stage = tonumber(checkpoint.Name)
    end)
end

In this case we’re strictly only using if statements as conditionals. If any conditional check fails then return is called which exits out of the scope, effectively stopping the function from continuing.


IMO for something like obby checkpoints I’d probably use CollectionService to handle the first sample with attributes or something other form of IDing to determine contiguity, then RespawnLocation to handle the second sample over CFraming HumanoidRootParts. RespawnLocation is pretty cool and supersedes most of my needs to explicitly write custom spawn positioning logic.

In regards to updating RespawnLocation, I’d just have an attribute on the player or a signal that I can connect to. When said signal is fired (either an AttributeChangedSignal or a custom signal written into the profile cache that fires when a value is updated), then I’d just read the current stage value and set RespawnLocation to the corresponding spawn. This is assuming there will only ever be one spawn per stage though. In all other cases, you’d need custom spawn positioning logic.

2 Likes

Thanks for the reply! I’ll be sure to check out everything you said and linked.

Sorry for the late reply but do you know if Profile Service has a function to check if the data has changed? My idea is to update both the leaderstat value and Respawn location in the ProfileCacher module. If not should I just use a while loop and coroutine it instead? Here is a concept of the code:

Stage.Changed:Connect(function() -- Would fire when profile data was updated
	local profile = CachedProfiles[player]
	if profile then
		for _, CheckPoint in pairs(Spawns:GetChildren())  do  -- Iterate through all the spawns
			if CheckPoint:GetAttribute("Stage") == profile.Data.Stage then -- Check if checkpoint attribute is equal to the profile data
				player.RespawnLocation = CheckPoint -- Sets respawn point
			end
			Stage.Value = profile.Data.Stage --Updates leaderstat value

		end
	end
end)

It doesn’t. In this case I would have a list of all the checkpoints that exist in the game stored in a dictionary somewhere. This way the loop only occurs once which is when the server is creating a list of the checkpoints. From there, most work is index lookups. Crude examples (not meant to be copied and pasted, can be branched out between ModuleScripts or adopted in other ways):

-- Getting a list
local checkpointSpawns = table.create(#spawns:GetChildren())

for _, checkpoint in ipairs(spawns:GetChildren()) do
    local stage = checkpoint:GetAttribute("Stage") -- or name if you use name
    checkpointSpawns[stage] = checkpoint
end

-- Setting a spawn
local function updateSpawnPoint(newValue)
    local respawnPoint = checkpointSpawns[newValue]
    player.RespawnLocation = respawnPoint
end)

stage.Changed:Connect(updateSpawnPoint)
updateSpawnPoint(stage.Value)

When needing to update data in ProfileService, you’d also subsequently change the stage value.

1 Like

Hi, sorry for another late reply. I finally got around to implementing a system where if the leaderstat is updated it also updates the profile in a module script. Now I only have one problem. When I run the game, the player can randomly spawn at any spawn point. Is their any way to fix this?

Do you also set the RespawnLocation when the data is first retrieved? I probably should’ve mentioned this before suggesting its use as a potential alternative but the catch about RespawnLocation is that if it isn’t set then the engine’s automatic spawning logic will take other existing spawns as valid and attempt to spawn the player at one of them.

You will need to set this immediately when they join or if that’s not fast enough then respawn them once so they end up at the proper location. Ultimately the latter case doesn’t sound too ideal though and in that case it may have been better to go with your original case of CFraming the HumanoidRootPart accordingly over what I suggested. I would still use attributes though! Instead of setting RespawnLocation it could help you determine which part to CFrame to.

1 Like

Sorry, I completely forgot about this but I fixed it by setting the spawn as soon as they join and I make sure to set their values as well. Thanks for the help!

Put the following code under the checkpoint part and create a Checkpoints folder under ServerStorage。If the player touched a checkpoint part, it will respawn at the last checkpoint part touched after death

local spawn = script.Parent
spawn.Touched:connect(function(hit)
	if hit and hit.Parent and hit.Parent:FindFirstChild("Humanoid") then
		local checkpoints = game.ServerStorage:WaitForChild("Checkpoints")
		local player = game.Players:GetPlayerFromCharacter(hit.Parent)
		local checkpointData = checkpoints:FindFirstChild("CheckpointData")
		if not checkpointData then
			checkpointData = Instance.new("Model", checkpoints)
			checkpointData.Name = "CheckpointData"
		end

		local checkpoint = checkpointData:FindFirstChild(tostring(player.userId))
		if not checkpoint then
			checkpoint = Instance.new("ObjectValue", checkpointData)
			checkpoint.Name = tostring(player.userId)

			player.CharacterAdded:connect(function(character)
				wait()
				character:WaitForChild("HumanoidRootPart").CFrame = checkpointData[tostring(player.userId)].Value.CFrame + Vector3.new(0, 4, 0)
			end)
		end

		checkpoint.Value = spawn
	end
end)
1 Like