Modules for a Donation Game

Resources Community Resources
7
  1. Your code inconsistently uses the up-to-date task library, to not using it.
  2. Your code does not use string interpolation.
  3. Your code misuses spawn and does not provide a significant benefit over iteration. Any case where spawn would be handy is also where HTTP Service would throttle, and then you’d be creating race conditions for retries. Speaking of which…
  4. There’s no protection on HttpService::GetAsync. Asynchronous functions should always be protected. There’s also no rate-limiting.
  5. Your code doesn’t give variables clear and concise names. This is a very opinionated thing, but most people would agree that at least your code is not clear or concise.
  6. Unnecessary waiting, especially for code that could yield and has no protection against that happening! This is a way bigger problem than you realize.
  7. Your code does not provide typings. This is a preference but come on, they’ve been out since 2020. There’s no reason to not just adapt.
  8. This is a released module yet isn’t release-ready. You’ve left a lot of debug logging in here for no reason. This should’ve been removed before publishing or at least make a toggleable variable for debug outputs.

Anyway, enough criticism, what should this resource maybe look like as a open-source-ready resource? It’s easy to be told what you did wrong but not be shown what you were supposed to do. When it comes to programming, there’s always going to be multiple different ways of getting your task finished and everyone will always argue about better methods till the end of time. Of course, these are mostly opinionated and it usually doesn’t matter. The reason your module falls under the circumstance of actually mattering is because it goes against the standards of what is expected.

I’m not saying my implementation is better in optimization, or, quite frankly, if it even works. This is untested and I wrote it inside of the Devforum. Presentation, being up-to-date, readability, type-safety, limit safety (if applicable, which it is in your case) are all extremely necessary when it comes to community resources. Even if you don’t need them or use them for yourself, when you publish something here it’s no longer for yourself and not everyone is you. So, here’s a more legible, type-safe, throttle-safe*, up-to-date with standards, and spawnless version of getting Gamepasses (Products) and Catalog Assets of a specified Player which also allows offline players to be checked given a username or userId.

* The HTTPS request throttle is not fool proof if you’re making a lot of requests outside of this module.

local players = game:GetService("Players")
local https = game:GetService("HttpService")

local placeURL = "https://games.roproxy.com/v2/users/%d/games?accessFilter=2&limit=50&sortOrder=Desc"
local gamepassURL = "https://games.roproxy.com/v1/games/%d/game-passes?limit=100&sortORder=Asc"
local catalogURL = "https://catalog.roproxy.com/v1/search/items/details?Category=3&Subcategory=%d&CreatorName=%s&salesTypeFilter=1&SortType=3"

local bindable = Instance.new("BindableEvent")

local totalRequests = 0 
local requestLimit = 60 -- to be safe in case there's other requests in the game
local retryLimit = 3
local function get(url: string, retries: number?): any
    retries = retries or 0
    totalRequests += 1

    if totalRequests >= requestLimit then
        bindable.Event:Wait()
    end

    local success, response = pcall(https.GetAsync, https, url)
    task.delay(60, function()
        totalRequests -= 1
        bindable:Fire()
    end)

    local function retry()
        return get(url)
    end

    if not success then
        if retries < retryLimit then
            return https:JSONDecode(get(url, retries + 1)), retry
        end

        return nil
    end

    return https:JSONDecode(response), retry
end

export type Asset = {
    id: number,
    price: number,
    isGamepass: boolean
}

return function(player: Player | string | number): {Asset}?
    local playerName
    local playerUserId

    if type(player) == "string" then -- assume username, you could do a tonumber check too!
        playerName = player

        local success, userId = pcall(players.GetUserIdFromNameAsync, players, playerName)
        if not success then
            return nil
        end

        playerUserId = userId
    elseif type(player) == "number" then -- assume user ID
        playerUserId = player

        local success, username = pcall(players.GetNameFromUserIdAsync, players, playerUserId)
        if not success then
            return nil
        end

        playerName = username
    elseif typeof(player) == "Player" then
        playerName = player.Name
        playerUserId = player.UserId
    else
        return nil
    end

    local tshirtURL = catalogURL:format(55, playerName)
    local shirtURL = catalogURL:format(56, playerName)
    local pantsURL = catalogURL:format(57, playerName)
    local placesURL = placeURL:format(playerUserId)

    local all = {}

    for _, url in { tshirtURL, shirtURL, pantsURL } do
        local response, retry

        repeat response, retry = get(url)
        until response and response.data or not retry

        if response then
            for _, item in response.data do
                if item.price and item.price > 0 then
                    table.insert(all, {
                        id = item.id,
                        price = item.price,
                        isGamepass = false
                    })
                end
            end
        end
    end

    local places = {}
    local cursor = ""
    local retries = 0

    while retries < retryLimit do
        local response, retry

        repeat
            response, retry = get(`{placesURL}&cursor={cursor}`)
            retries += 1
        until response and response.data or not retry

        if response then
            cursor = response.nextPageCursor
            retries = 0

            for _, place in response.data do
                table.insert(places, place.id)
            end
        end
    end

    for _, placeId in places do
        local gamepassesURL = gamepassURL:format(placeId)
        local response, retry

        repeat response, retry = get(gamepassesURL)
        until response and response.data or not retry

        if response then
            for _, gamepass in response.data do
                if gamepass.price then
                    table.insert(all, {
                        id = gamepass.id,
                        price = gamepass.price,
                        isGamepass = true
                    })
                end
            end
        end
    end

    return all
end
4 Likes