Qbox-project / qbx_core

http://qbox-project.github.io
Other
45 stars 109 forks source link

Gracefully handle slow model loading #495

Closed TransitNode closed 6 days ago

TransitNode commented 1 week ago

The problem

Instead of waiting for a timeout to occur before throwing an error, I tried to implement an improved system for handling slow model loading. Now, instead of abruptly timing out and throwing an error, the code will automatically extend the loading time for players with slower PCs. Here is my idea of how this would work out:

Doing it this way could help give players with slower computers more time to load models while also keeping players informed about what's happening during this. It also provides clear instructions if a critical loading failure should occur.

Ideal solution

At this very moment I don't have fivem nor a qbox server setup, so this code will be untested, but i wrote it to have it work as seamlessly as possible. If anyone has a better method or idea you are welcome to suggest it.

client/character.lua:

-- Function to request a model with notifications for slow loading or failures
local function requestModelWithNotification(model, timeout)
    timeout = timeout or config.loadingModelsTimeout
    local notificationId = 'slow_model_loading_' .. tostring(model)

    -- Attempt to load the model using ox_lib
    local success = lib.requestModel(model, timeout)
    if success then return model end

    -- Check if the model is valid before proceeding
    if not IsModelValid(model) then
        error(("Attempted to load invalid model '%s'"):format(model))
    end

    -- Manually request the model if ox_lib failed
    RequestModel(model)
    local startTime = GetGameTimer()
    local slowLoadingNotificationShown = false

    -- Loop until the model is loaded or we hit our extended timeout
    while not HasModelLoaded(model) do
        Wait(100)

        -- Show a notification if loading is taking longer than expected
        if not slowLoadingNotificationShown and GetGameTimer() - startTime > timeout then
            exports.qbx_core:Notify({
                text = 'The model is taking longer than usual to load. Please wait...',
                caption = 'Slow Model Loading'
            }, 'inform', 10000)
            slowLoadingNotificationShown = true
        end

        -- If loading takes too long, notify the player and potentially disconnect them
        if GetGameTimer() - startTime > timeout * 3 then
            exports.qbx_core:Notify({
                text = 'Unable to load the model. You may be disconnected. Please restart your game.',
                caption = 'Model Loading Failed'
            }, 'error', 10000)

            -- Check with the server if the player should be kicked
            if lib.callback.await('qbx_core:modelLoadFailure', false, model) then
                Wait(10000)
                return false
            end
        end
    end

    -- Notify the player if the model finally loaded after a slow start
    if slowLoadingNotificationShown then
        exports.qbx_core:Notify({
            text = 'The model has finished loading. Thank you for your patience.',
            caption = 'Model Loaded'
        }, 'success', 5000)
    end

    return model
end

-- Function to load a random ped model
local function randomPed()
    local ped = randomPeds[math.random(1, #randomPeds)]
    local modelLoaded = requestModelWithNotification(ped.model, config.loadingModelsTimeout)

    -- If the random model failed to load, try a default model
    if not modelLoaded then
        exports.qbx_core:Notify('Failed to load random character model. Using default.', 'error')
        modelLoaded = requestModelWithNotification(`a_m_y_skater_01`, config.loadingModelsTimeout)
    end

    if modelLoaded then
        -- Set the player model and appearance
        SetPlayerModel(cache.playerId, modelLoaded)
        pcall(function() exports['illenium-appearance']:setPedAppearance(PlayerPedId(), ped) end)
        SetModelAsNoLongerNeeded(modelLoaded)
    else
        -- Notify the player if all attempts to load a model have failed
        exports.qbx_core:Notify('Critical error: Unable to load any character model. Please restart your game.', 'error')
    end
end

-- Function to preview a specific ped model
local function previewPed(citizenId)
    if not citizenId then randomPed() return end

    -- Get the ped data from the server
    local clothing, model = lib.callback.await('qbx_core:server:getPreviewPedData', false, citizenId)
    if model and clothing then
        local modelLoaded = requestModelWithNotification(model, config.loadingModelsTimeout)

        -- If the specific model failed to load, try a default model
        if not modelLoaded then
            exports.qbx_core:Notify('Failed to load character model. Using default.', 'error')
            modelLoaded = requestModelWithNotification(`a_m_y_skater_01`, config.loadingModelsTimeout)
        end

        if modelLoaded then
            -- Set the player model and appearance
            SetPlayerModel(cache.playerId, modelLoaded)
            pcall(function() exports['illenium-appearance']:setPedAppearance(PlayerPedId(), json.decode(clothing)) end)
            SetModelAsNoLongerNeeded(modelLoaded)
        else
            -- If all attempts fail, notify the player and fall back to a random ped
            exports.qbx_core:Notify('Critical error: Unable to load any character model. Please restart your game.', 'error')
            randomPed()
        end
    else
        -- If no specific ped data is found, use a random ped
        randomPed()
    end
end

server/character.lua:

-- Table to keep track of model loading failures for each player
local modelLoadFailures = {}

-- Callback to handle model loading failures
lib.callback.register('qbx_core:modelLoadFailure', function(source, model)
    local src = source
    -- Increment the failure count for this player
    modelLoadFailures[src] = (modelLoadFailures[src] or 0) + 1

    -- If the player has had too many failures, kick them
    if modelLoadFailures[src] >= 3 then
        SetTimeout(10000, function()
            DropPlayer(src, 'Multiple model loading failures. Please restart your game.')
            -- Clean up the failure count for this player
            modelLoadFailures[src] = nil
        end)
        return true -- Indicate that the player should be kicked
    end

    return false -- Indicate that the player should not be kicked yet
end)

Alternative solutions

No response

Additional context

No response

Manason commented 1 week ago

If I understand correctly, this is effectively an increased timeout, but with player notifications along the way, so the player has an indication it's still working. That is, unless there is an issue with the request model native itself, and calling it multiple different times makes it more likely to succeed.

Do you think adding this much code to the code base is worth the player notifications? Another way to implement this would be a single long timeout load, with a separate thread for doing player notifications every x seconds.

TransitNode commented 1 week ago

I'm not really sure I understand what you mean by "adding this much code to the code base is worth it", in my opinion anything that could improve QoL or player experience is always worth it. But we should of course look at a way of implementing this without adding bloat. If its possible to implement this idea without the "amount of code", I'm all for it. Your idea of a separate thread for playing notifications seems like a better way to do this, only problem I can see from doing that is, if the loading completes or fails before the notifications catch up. For example if the model loads, but the notification tells the player its taking a while to load and he decides to quit/restart even though it has loaded.

Manason commented 1 week ago

I'm not really sure I understand what you mean by "adding this much code to the code base is worth it", in my opinion anything that could improve QoL or player experience is always worth it. But we should of course look at a way of implementing this without adding bloat. If its possible to implement this idea without the "amount of code", I'm all for it. Your idea of a separate thread for playing notifications seems like a better way to do this, only problem I can see from doing that is, if the loading completes or fails before the notifications catch up. For example if the model loads, but the notification tells the player its taking a while to load and he decides to quit/restart even though it has loaded.

You could have a break condition using a Boolean that would be set when the model loads to prevent the kind of issue you are describing. I think doing it this way would be a small implementation.

TransitNode commented 1 week ago

I'm not really sure I understand what you mean by "adding this much code to the code base is worth it", in my opinion anything that could improve QoL or player experience is always worth it. But we should of course look at a way of implementing this without adding bloat. If its possible to implement this idea without the "amount of code", I'm all for it. Your idea of a separate thread for playing notifications seems like a better way to do this, only problem I can see from doing that is, if the loading completes or fails before the notifications catch up. For example if the model loads, but the notification tells the player its taking a while to load and he decides to quit/restart even though it has loaded.

You could have a break condition using a Boolean that would be set when the model loads to prevent the kind of issue you are describing. I think doing it this way would be a small implementation.

What do you think about this implementation? With this wrapper we maintain original functionality of oxlib lib.requestmodel while adding our own functionality to it. It uses a single thread for both notifications, no server-side code, we scratch the idea of having the client kicked, even though I think that's desirable. And we have a config option for the extended timeout.

local function requestModelWithNotification(model, timeout)
    timeout = timeout or config.loadingModelsTimeout
    local success = lib.requestModel(model, timeout)
    if success then return true end

    local startTime = GetGameTimer()
    local notified = false
    CreateThread(function()
        while not HasModelLoaded(model) do
            local elapsed = GetGameTimer() - startTime
            if elapsed > timeout and not notified then
                exports.qbx_core:Notify('Model loading slowly. Please wait...', 'inform')
                notified = true
            elseif elapsed > timeout * config.extendedTimeoutMultiplier then
                exports.qbx_core:Notify('Loading failed. Please restart your game.', 'error')
                break
            end
            Wait(5000) -- Check every 5 seconds
        end
    end)

    return lib.requestModel(model, timeout * config.extendedTimeoutMultiplier)
end

in config.lua:

config.extendedTimeoutMultiplier = 2 

Then replace lib.requestmodel with requestModelWithNotification(model, timeout) where desired in character.lua.

mafewtm commented 1 week ago

This all seems so unnecessary. Just edit the timeout config option and be done with it. We shouldn't need to do all this, let alone drop players for failing to load models.

TransitNode commented 1 week ago

This all seems so unnecessary. Just edit the timeout config option and be done with it. We shouldn't need to do all this, let alone drop players for failing to load models.

I don't understand where you are coming from, you would rather have players be confused about why they're being thrown an error for having a slower computer, how would the server owner know what an appropriate timeout amount would be? Would you rather have players just sitting there waiting with no indication on what they are waiting on? I don't think you have the right game design perspective on this. In a game framework, enhancing player experience isn't just a nice-to-have—it's a core responsibility. Every opportunity to improve feedback and reduce frustration directly contributes to the framework's value and usability.

Manason commented 1 week ago

I'm not really sure I understand what you mean by "adding this much code to the code base is worth it", in my opinion anything that could improve QoL or player experience is always worth it. But we should of course look at a way of implementing this without adding bloat. If its possible to implement this idea without the "amount of code", I'm all for it. Your idea of a separate thread for playing notifications seems like a better way to do this, only problem I can see from doing that is, if the loading completes or fails before the notifications catch up. For example if the model loads, but the notification tells the player its taking a while to load and he decides to quit/restart even though it has loaded.

You could have a break condition using a Boolean that would be set when the model loads to prevent the kind of issue you are describing. I think doing it this way would be a small implementation.

What do you think about this implementation? With this wrapper we maintain original functionality of oxlib lib.requestmodel while adding our own functionality to it. It uses a single thread for both notifications, no server-side code, we scratch the idea of having the client kicked, even though I think that's desirable. And we have a config option for the extended timeout.

local function requestModelWithNotification(model, timeout)
    timeout = timeout or config.loadingModelsTimeout
    local success = lib.requestModel(model, timeout)
    if success then return true end

    local startTime = GetGameTimer()
    local notified = false
    CreateThread(function()
        while not HasModelLoaded(model) do
            local elapsed = GetGameTimer() - startTime
            if elapsed > timeout and not notified then
                exports.qbx_core:Notify('Model loading slowly. Please wait...', 'inform')
                notified = true
            elseif elapsed > timeout * config.extendedTimeoutMultiplier then
                exports.qbx_core:Notify('Loading failed. Please restart your game.', 'error')
                break
            end
            Wait(5000) -- Check every 5 seconds
        end
    end)

    return lib.requestModel(model, timeout * config.extendedTimeoutMultiplier)
end

in config.lua:

config.extendedTimeoutMultiplier = 2 

Then replace lib.requestmodel with requestModelWithNotification(model, timeout) where desired in character.lua.

I guess this is better, but I still think an even better solution would just be to have the second thread maintain a print interval, and only call lib.requestModel once. That has the benefit of printing at whatever interval is desired, instead of just once. I also think it would be simpler code.

As a side note, we're doing some research to try to figure out why ox_lib times out so often. Maybe we can solve the problem at the source.

TransitNode commented 1 week ago

I guess this is better, but I still think an even better solution would just be to have the second thread maintain a print interval, and only call lib.requestModel once. That has the benefit of printing at whatever interval is desired, instead of just once. I also think it would be simpler code.

I tried to make the code more readable and also let lib.requestmodel handle the loading process itself. The code waits until the initial timeout period has passed before showing any notification and the notification thread stops either when the model successfully loads or when the extendedTimeout is exceeded. I also decided to hardcode the config value, I'm thinking if we are able to solve the issue at the source, this would be a temporary relief, in the sense it allows players with slower hardware know that they are taking some extra time loading these models.

local function requestModelWithNotification(model, timeout)
    timeout = timeout or config.loadingModelsTimeout
    local extendedTimeout = timeout * 2  -- (2x the original timeout)

    CreateThread(function()
        local startTime = GetGameTimer()
        while not HasModelLoaded(model) do
            local elapsed = GetGameTimer() - startTime
            if elapsed > timeout then
                local isExtendedTimeoutExceeded = elapsed > extendedTimeout
                exports.qbx_core:Notify(
                    isExtendedTimeoutExceeded and 'Loading failed. Please restart your game.' or 'Model loading slowly. Please wait...',
                    isExtendedTimeoutExceeded and 'error' or 'inform'
                )
                if isExtendedTimeoutExceeded then return end
            end
            Wait(5000) -- notification interval
        end
    end)

    return lib.requestModel(model, extendedTimeout)
end