Is my countdown good ?

[lapis hill]

Hello, it's to know if my countdown client script is good. 0 = the player leave (not the game), 10 = when the player is solo and 20 when the player play vs another player.

MenuCountdownEvent.OnClientEvent:Connect(function(Duration)
    CurrentCountdownID += 1
    local CountdownInstanceId = CurrentCountdownID

    PlayTextButton.Interactable = false

    if Duration == 0 then
        return
    end

    for i = Duration, 1, -1 do
        if CountdownInstanceId ~= CurrentCountdownID then
            return
        end

        if Duration == 10 then
            PlayTextButton.Text = "Play solo in " .. i .. "s"
        elseif Duration == 20 then
            PlayTextButton.Text = "Starting duo in " .. i .. "s"
        end

        task.wait(1)
    end

    if CountdownInstanceId == CurrentCountdownID then
        if Duration == 10 then
            PlayTextButton.Interactable = true
            PlayTextButton.Text = "Play solo"
        elseif Duration == 20 then
            PlayTextButton.Interactable = false
            PlayTextButton.Text = "Starting duo ..."
        end
    end
end)

Basically, to know if my code is 'perfect', if I can say, optimized and with nothing unnecessary

[knotty vine]

using duration to determine game mode is limiting yourself

[lapis hill]

Ik that why im asking, but it's a rythme game so it's 10 to show a song before clicking and 20 before it's start alone when 1v1

[knotty vine]

is your code meant to put the interactable to different values depending on gamemode

[lapis hill]

Yes, when the player leave the stage then it's reset so :
MenuCountdownEvent:FireClient(Player, 0)
Solo = MenuCountdownEvent:FireClient(Player, 10) to make him wait before starting solo
Duo = MenuCountdownEvent:FireClient(Player, 20) 20 second before the game auto start

[knotty vine]

local ModeInfo = {
    Solo = {
        Countdown = "Play solo in %s s";
        End = "Play solo";
        InteractableOnEnd = false
    };
    Duo = {
        Countdown = "Starting duo in %s s";
        End = "Starting duo ...";
        InteractableOnEnd = true
    }
}

MenuCountdownEvent.OnClientEvent:Connect(function(Duration, Type)
    CurrentCountdownID += 1
    local CountdownInstanceId = CurrentCountdownID


    if Duration == 0 then
        return
    end
    
    local ModeData = ModeInfo[Type]
    
    if not ModeData then
        warn(`No Mode data for countdown type: {Type}`)
        return
    end
    
    PlayTextButton.Interactable = false
    
    local CountdownText:string = ModeData.Countdown
    local EndText:string = ModeData.End
    
    for i = Duration, 1, -1 do
        if CountdownInstanceId ~= CurrentCountdownID then
            return
        end

        PlayTextButton.Text = CountdownText:format(i)

        task.wait(1)
    end

    if CountdownInstanceId == CurrentCountdownID then
        PlayTextButton.Interactable = ModeData.InteractableOnEnd
        PlayTextButton.Text = EndText
    end
end)```

I would do it a bit more like this but

thats just me

[lapis hill]

I see but why making a local when it's the same use amount ?

[knotty vine]

wdym

[left vine]

Your countdown does not account for time lost to variable framerate

The countdown instance ID isn't necessary either

[knotty vine]

I just modified his without changing it too much

also task.wait doesnt go off of frame rate

[lapis hill]

My reason for keeping the Countdown Instance ID is to handle edge cases where the server sends multiple countdown events quickly. For example, if a player leaves or switches from solo to duo, the old countdown loop could still be running and overwrite the UI of the new countdown.

[lapis hill]

Task.wait dosen't ?

[left vine]

Start and cancel a thread

[lapis hill]

It's worth to use thread on client ?

[left vine]

Zero impact

[lapis hill]

I see

[left vine]

No. It's not guaranteed to wait 1 second. If the client experiences a lag spike, your timer goes out-of-sync as it assumes 1 second has passed by statically subtracting 1 per yield

[lapis hill]

So i use tick + Duration ?

[knotty vine]

I would use a runservice connection

then you can display decimals pretty easy

[lapis hill]

Okay i will first look for the thread

[lapis hill]

Something like that ? :

MenuCountdownEvent.OnClientEvent:Connect(function(Duration)
    if CountdownThread then
        CountdownThread:Cancel()
    end
    
    PlayTextButton.Interactable = false
    
    if Duration <= 0 then
        return
    end
    
    CountdownThread = task.spawn(function()
        local EndTime = tick() + Duration
        
        while tick() < EndTime do
            local Remaining = math.ceil(EndTime - tick())
            
            if Duration == 10 then
                PlayTextButton.Text = "Play solo in " .. Remaining .. "s"
            elseif Duration == 20 then
                PlayTextButton.Text = "Starting duo in " .. Remaining .. "s"
            end
            
            task.wait(0.1)
        end
        
        if Duration == 10 then
            PlayTextButton.Interactable = true
            PlayTextButton.Text = "Play solo"
        elseif Duration == 20 then
            PlayTextButton.Text = "Starting duo ..."
        end
    end)
end)

[lapis hill]

I can use taskdelay with dutation but it's out of sync too ?

[left vine]

Yes

[left vine]

This is good, but threads aren't objects with methods. You have to use task.cancel or coroutine.close to cancel a thread

[lapis hill]

Oh yeah ofc mb

[left vine]

You shouldn't communicate modes via duration intervals. That's implicit state. Send an enum

tick is also considered deprecated. Use time

You can also check the result of EndTime - tick() instead of tick() < EndTime

[lapis hill]

Okay i will check all of that thanks

[lapis hill]

@left vine

Perfect ? :

local RunService = game:GetService("RunService")
local CountdownConnection

MenuCountdownEvent.OnClientEvent:Connect(function(Duration)
    if CountdownConnection then
        CountdownConnection:Disconnect()
    end
    
    PlayTextButton.Interactable = false
    
    if Duration <= 0 then
        return
    end
    
    local EndTime = time() + Duration
    local lastRemaining = nil
    
    CountdownConnection = RunService.Heartbeat:Connect(function()
        local RemainingTime = math.max(0, math.floor(EndTime - time()))
        
        if RemainingTime ~= lastRemaining then
            lastRemaining = RemainingTime
            
            if Duration == 10 then
                if RemainingTime > 0 then
                    PlayTextButton.Text = "Play Solo in " .. RemainingTime .. "s"
                else
                    PlayTextButton.Interactable = true
                    PlayTextButton.Text = "Play Solo"
                    CountdownConnection:Disconnect()
                end
            elseif Duration == 20 then
                if RemainingTime > 0 then
                    PlayTextButton.Text = "Play duo in " .. RemainingTime .. "s"
                else
                    PlayTextButton.Text = "Play duo"
                    CountdownConnection:Disconnect()
                end
            end
        end
    end)
end)