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.
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)
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.
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.
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.
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)
