Avoid dark and duplicate nametag colors #708
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,26 +1,59 @@ | ||
| local lowerBound, upperBound = unpack(get("color_range")) | ||
| local usedColors = {} | ||
|
|
||
| local function generateColor() | ||
| local r, g, b | ||
| repeat | ||
| r = math.random(lowerBound, upperBound) | ||
| g = math.random(lowerBound, upperBound) | ||
| b = math.random(lowerBound, upperBound) | ||
| until (r + g + b) > 200 | ||
| return r, g, b | ||
| end | ||
|
|
||
| local function colorToKey(r, g, b) | ||
| return r .. "," .. g .. "," .. b | ||
| end | ||
|
|
||
| local function randomizePlayerColor(player) | ||
| player = player or source | ||
| if not isElement(player) then return end | ||
|
|
||
| local r, g, b, key | ||
| for i = 1, 10 do | ||
| r, g, b = generateColor() | ||
| key = colorToKey(r, g, b) | ||
| if not usedColors[key] then | ||
| break | ||
|
Member
There was a problem hiding this comment. This doesn't seem like the right approach, assuming the goal of the PR is to ensure all players have a unique color, and that colors are different from each other. Technically there are 206³ (8,741,816) possible keys (RGB color combinations) given 206 possible configurations (50-255 range) per value. However we're only attempting to generate a color 10 times before giving up if they've already been used. Whilst technically not an issue given the huge amount of possible combinations/keys, it's still a logically flawed approach given the previously mentioned factor of luck; and the simple fact this isn't optimal/efficient. A better approach would be to generate a limited size pool of random colors on resource start (equal to the max player count) and then select a random one to assign to a player. Once assigned, pop from the active pool, place into an "inactive" list/array (indexed by player) and return back to the pool once the player disconnects (so the color can be used again). This ensures players are all assigned unique colors, without needing to iterate at all to find an unused value (since we generate one at random first try from the pre-allocated pool). You may also wish to implement additional logic to ensure the active pool colors aren't too similar when initially pre-allocating them. |
||
| end | ||
| end | ||
|
|
||
| local oldR, oldG, oldB = getPlayerNametagColor(player) | ||
| if oldR then | ||
| usedColors[colorToKey(oldR, oldG, oldB)] = nil | ||
| end | ||
|
|
||
| setPlayerNametagColor(player, r, g, b) | ||
| usedColors[key] = true | ||
| end | ||
|
|
||
| addEventHandler("onPlayerJoin", root, randomizePlayerColor) | ||
|
|
||
| local function setAllPlayerColors() | ||
| usedColors = {} | ||
| for _, player in ipairs(getElementsByType("player")) do | ||
| randomizePlayerColor(player) | ||
| end | ||
| end | ||
|
|
||
| addEventHandler("onGamemodeMapStart", root, setAllPlayerColors) | ||
| addEventHandler("onResourceStart", resourceRoot, setAllPlayerColors) | ||
|
|
||
| addEventHandler("onPlayerQuit", root, | ||
| function() | ||
| local r, g, b = getPlayerNametagColor(source) | ||
| if r then | ||
| usedColors[colorToKey(r, g, b)] = nil | ||
| end | ||
| end | ||
| ) | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the most unlucky player in the history of MTA, this could return a value under 200 even if they waited for an infinite amount of time
The lower bound would need increasing to ensure the three values always have a sum larger than 200 (in which case you don't need to check if it's larger than 200 anymore, colors are always bright enough), or you would need to lower the check itself; i.e
(r + g + b) > 149given current lower bounds.It also seems like this is configurable for a reason via meta.xml (so server owners can choose the range of colors) - yet you're now introducing a hard constraint within the code itself (limiting the range of these configuration values).