--[[
Alright I dunno what's going in this code but there is a lot that rough here
and likely your biggest issue is your use of while why do people do this
--]]
-- DO IT
local RunService = game:GetService("RunService")
-- Suggest if your going to require or get service stuff you do it at the top
local MarketplaceService = game:GetService("MarketplaceService")
-- Why are you requiring this within the function? No reason and slower
local ChatService = require(game:GetService("ServerScriptService"):WaitForChild("ChatServiceRunner").ChatService)
-------------------------------------------------------------------------------------------------
-- What is this Id? Put some context in the name there is nothing to imply
-- what this actually is
local SomethingId = 855140482
--Is this a generic color table, is does it have context to character face
-- again be more specific with your variables this is over genaric
-- something liek this would be in an ENUM if its that universal
-- PERSONAL PREFERENCE: Outside of some rare cases, anything with more than
-- 2 arguments i change the format to drop down, easier to read and edit
local ColorTable = {
Color3.fromRGB (
255,
0,
0
),
Color3.fromRGB (
255,
120,
0
),
Color3.fromRGB (
255,
200,
0
),
Color3.fromRGB (
0,
255,
0
),
Color3.fromRGB (
0,
166,
255
),
Color3.fromRGB (
175,
0,
255
),
Color3.fromRGB (
255,
0,
255
)
}
-- If you never change this then this should just be hard referenced
-- If you can change it make a function that returns the table
local DefaultTag = {
{
TagText = "VIP",
TagColor = Color3.fromRGB (
255,
200,
0
),
}
}
--[[
This function almost comments itself but it takes this API call and makes it more
readable in context to waht your using it for plus now its encapsilated
and make it easier to maintain
]]
local function doesUserOwnGamePass(player)
return MarketplaceService:UserOwnsGamePassAsync(player.UserId, SomethingId)
end
--[[
If we do not have the speaker from the first call we are going to hold here
till we do
We do a flat check, to see if we catch it first time, if we do we skip the repeat
and return the speaker we got.
Otherwise we now go into our repeat that will automatically complete
when the speaker is not nill and return it.
HaZaaa so much easier
]]
local function getOrWaitForSpeaker(player)
local speaker = ChatService:GetSpeaker(player.Name)
-- Predicts are awesome
if speaker ~= nil then return speaker end
-- 0.01 is may as well just do it this way irrelevent
-- of time at this point
repeat
task.wait()
speaker = ChatService:GetSpeaker(player.Name)
until speaker ~= nil
return speaker
end
game.Players.PlayerAdded:Connect(function(player)
-- Predicate this is going to exit the fastest from this and makes code more readable
-- less if blocks
if not doesUserOwnGamePass(player) then return end
local speaker = getOrWaitForSpeaker(player)
-- This is a very very pattern to use
--local speaker = nil
--while speaker == nil do
-- speaker = ChatService:GetSpeaker(player.Name)
-- if speaker ~= nil then break end
-- task.wait(0.01)
--end
-- this is likely what is causing you lag but this would go on forever you have ZERO way of
-- stopping this or rolling over it, you j ust gonna do it FOREVER there are better
-- event drivine ways to do these kind of updates isntead of trying to passive scan them
--while true do
-- task.wait(1)
-- speaker:SetExtraData("Tags", Tag)
-- speaker:SetExtraData("ChatColor", ColorTable[math.random(1, #ColorTable)])
--end
-- Since I do not have any idea what or why your doing this this way
-- This is WAY better and more stable then the wait use. It happens once a frame
-- IF you want to slow it down, just add a timer. AKA creat a variable
-- add the delta time to the value every frame till it hits a time, have it perform a function
-- in that function flip the update back off repeat. EZ peezy
-- I suggest using a real random object instead of math.random.
RunService.Heartbeat:Connect(
function(deltaTime)
speaker:SetExtraData("Tags", DefaultTag)
-- Performance note
-- since color table is not dynamic and never edited you should store this value
-- above like under it call it ColorTableSize or whatever
-- Then use that instead "#" every time is a function call and will eat at performance
-- It doesnt matter so much NOW and if you were doing this as a more event driven thing
-- it would matter far less, but if you put this in a active loop that its hit frequently YOU SHHOULD
-- reduce junk function calls when you can.
speaker:SetExtraData("ChatColor", ColorTable[math.random(1, #ColorTable)])
end
)
end)
Sorry its late and im not in a good mood but here some feedback
ok so im quite confused because this code should work but it doesnt and nothing in the output is this supposed to be a server script in serverscriptservice?
If five people who bought the gamepass join your game, you will have at least five while loops running forever. This will create memory that never gets cleaned up because you’re running code infinitely and code needs memory to run.
You should remove the while true do loop and replace it with a connection to the Chatted event. You don’t need to be constantly changing the chat colour, only when they send a new message.
Sorry, it’s been a rough few days. Just personal stuff nothing that’s your fault =).
Yea, though again, this guys answer is solid. If there is a reliable event that you can hook your set up logic too or process logic when it happens, that’s your generalized best bet and more efficient scenario.
But for a future note, while loops are great, but don’t use them in the way you have been. Your treating them like heartbeat or stepped but not putting them there.
