Script isn't working properly

Help and Feedback Scripting Support
1

Hello! I’m working on a phone system, and currently I’m working on a messaging app.

I want to check every second for the players that are in the server, and make a button for every user in the server. Currently, it’s only making a button for yourself. The code might be messy, and it’s a serverscript, just because I couldn’t figure out how to get the rest of my scripts to work if I wouldn’t do it this way.

Anyways here’s the script:

while true do

	for i,v in pairs(Players:GetChildren()) do

		local convoList = v.PlayerGui:WaitForChild("PhoneGui").PhoneScreenFrame.DisplayFrame.Apps.Messages.ConvoList

		if not v.PlayerGui:WaitForChild("PhoneGui").PhoneScreenFrame.DisplayFrame.Apps.Messages.ConvoList:FindFirstChild(v.Name) then

			local sampleButton = remotes.GUISamples.SampleButton

			local newButton = sampleButton:Clone()

			print(v.Name, v.UserId)

			local thumbType = Enum.ThumbnailType.HeadShot
			local thumbSize = Enum.ThumbnailSize.Size420x420
			local content, isReady = Players:GetUserThumbnailAsync(v.UserId, thumbType, thumbSize)

			local newButton = sampleButton:Clone()

			newButton.Visible = true
			newButton.ConvoName.Text = v.Name
			newButton.Name = v.Name
			newButton.Parent = v.PlayerGui.PhoneGui.PhoneScreenFrame.DisplayFrame.Apps.Messages.ConvoList
			newButton.ProfilePicture.Image = content

			for _, button in pairs(convoList:GetChildren()) do

				if button:IsA("UICorner") then
					print("dont delete")
				elseif button.Name == "UIListLayout" then
					print("dont delete")
				else
					print("delete")
					button:Destroy()
				end

			end
		else
			return
		end
	end

	wait(1)

end

This is what it looked like when my friend tried it, and there was 2 people in the server.

image
image318×556 16.6 KB

On the other player’s end, they could only see themselves.

Is there a way to fix this easily?

1 Like
2

while true do loops like this are messy and very inefficient.

You should use events

3

That’s not what this is about. I don’t want people picking on the efficiency, I’m asking for a solution.

4

Is this a local script? If so, try moving it to a regular script.

5

The solution is to not extremely inefficient hard to maintain while loops.

Events are for this exact purpose

This is how it’s done

local function onPlayerAdded ()
     -- code that will run for each player 
     
end

-- if the code above yields this is necessary 
for i,player in ipairs(game.Players:GetPlayers())do
   task.spawn(onPlayerAdded)(player)
end

game.Players.PlayerAdded:Connect(onPlayerAdded)

You are constantly checking and updating something in a busy while loop (very inefficient)

1 Like
6

As I said, this is a server script.

1 Like
7

Oh yeah, I forgot about this. I’ll try implementing it to my script.

1 Like
8

What you want

  • to show an accurate list of players on phone gui

What you are doing

  • looping through all the players in the game and reapplying the phone gui for each player every second

What you should be doing

  • displaying the player on the phone once when they join the game, and removing them from the phone when they leave the game
2 Likes
9

OK, I see it now.

I recommend using GetPlayers (not GetChildren) at the beginning, as well as PlayerAdded events. I don’t think PlayerAdded fires for people who arrive before the script is called.

10

That’s what this script is for

The only time he should be looping is the initial gui draw when the player joins

11

To put all of the players on the frame, do this (not gonna write actual code because i am tired rn):

  1. When the phone gui is opened, get all players online and add a frame for each of them (game.Players:GetPlayers())
  2. Have playeradded and playerremoved events, and have them add and remove a frame respectively
  3. ???
  4. Profit

This is what i always do, and as far as i know is the most easiest & efficient method to do this.

1 Like
12

lol sorry about that, your post snuck in there while I was looking at the code. :slight_smile:

1 Like