GetCore's PlayerFriendedEvent and PlayerUnfriendedEvent BindableEvents firing at inappropriate times

Zomebody · May 12, 2020, 12:33pm

The issue

Calling StarterGui:GetCore("PlayerFriendedEvent") and StarterGui:GetCore("PlayerUnfriendedEvent") return BindableEvent objects which should be called whenever the local player becomes friends with someone or unfriends someone as per the documentation:

And although this does indeed happen, these BindableEvent objects are in fact fired much more often, at inappropriate times.

Current behavior

The PlayerFriendedEvent BindableEvent is called whenever:

You become friends with someone during gameplay.
A friend of yours joins the game, but only the first time they join the game during your gameplay session.
You join a server your friends are in. The BindableEvent is called for every friend in the server.

The PlayerUnfriendedEvent BindableEvent is called whenever:

You unfriend someone during gameplay.
A player you are no friends with joins the game, but only the first time they join the server during your gameplay session.
You join a server where some players are no friends of yours. The BindableEvent is called for non-friend in the server.

Expected behavior

The PlayerFriendedEvent should only be called whenever:

You become friends with someone during gameplay.

The PlayerUnfriendedEvent should only be called whenever:

You unfriend someone during gameplay.

Reproducing the bug

I used this LocalScript to verify the unexpected behavior.

------------------------[[ = VARIABLES = ]]------------------------
--local Plr = game.Players.LocalPlayer
local StarterGui = game:GetService("StarterGui")

------------------------[[ = COREGUI LOGIC = ]]------------------------
local FriendedRemote = StarterGui:GetCore("PlayerFriendedEvent")
local UnfriendedRemote = StarterGui:GetCore("PlayerUnfriendedEvent")

FriendedRemote.Event:Connect(
	function(Plr, ...)
		print("Friended", Plr, ...)
		--game.ReplicatedStorage.ChangeFriendship:FireServer(Plr, "Friended")
	end
)

UnfriendedRemote.Event:Connect(
	function(Plr, ...)
		print("Unfriended", Plr, ...)
		--game.ReplicatedStorage.ChangeFriendship:FireServer(Plr, "Unfriended")
	end
)

I had a couple friends join me in a mostly empty game with that LocalScript. This is what my output looked like after these friends joined:

This is what the output looked like from XAXA’s perspective upon joining the game:

Notice how for both me and XAXA the output prints two names when in reality it should not print any names at all! We did not send friend requests nor unfriend each other during this time, so none of these prints should have been called.
Thank you @xaxa and @peteyk473 for helping me test this.

Why this should be fixed

Aside from obviously being unintended behavior which should always be fixed, this makes the logic for a piece of code I am working on significantly more complex. I am working on a Script which tracks for each player in the server with whom they are friends. The server has no way of knowing when friendship statuses between two players change except for constantly polling game.Players:GetFriendsAsync(Player.UserId), which of course takes up significantly more computing and networking resources than necessary. This is why I am using the PlayerFriendedEvent and PlayerUnfriendedEvent BindableEvents, but these signals are not reliable at all in their current state.

Iron_Legion · May 12, 2020, 5:06pm

We’ll look into this right away.

Club_Moo · May 13, 2020, 7:55am

This behavior seems correct, as to initialize the friend-status of players–After all–The status can change
externally to a particular server of yours–DataStore access has a similar initialization event idea.

Just keep flags for those initial event conditions, and not do whatever UI / friend-status-changed-in-game
things that you would normally do if the status changes in-game on those initial callbacks.

Using solely these events will obviate the need to pummel ```Player:IsFriendsWith()``–Which in effect
makes all servers involved less bogged-down. (I realize this is your intent BTW.)

EDIT: Your 2nd and 3rd bullet points in each category are the same thing just from different perspectives, as you likely know.

Zomebody · May 13, 2020, 12:48pm

From what I have seen in the CoreScripts, I am almost 100% certain that the behavior is in fact not correct. Let me explain to you why.

This is how the logic behind firing the BindableEvents is defined within the CoreScripts, found in PlayerDropDown.lua line 685:

LocalPlayer.FriendStatusChanged:connect(function(player, friendStatus)
	if friendStatus == Enum.FriendStatus.Friend then
		PlayerFriendedEvent:Fire(player)
	elseif friendStatus == Enum.FriendStatus.NotFriend then
		PlayerUnFriendedEvent:Fire(player)
	end
end)

This uses the FriendStatusChanged event, an event only CoreScripts are allowed to use. However, note the friendStatus argument. This enumeration can have multiple values:

The code above only checks whether this value is set to Friend or NotFriend. This asking for trouble however! Presumably, the reason why these BindableEvents are fired for each player when you join, or whenever someone joins your server (but only the first time they join your server) is because the friendship status between you and the other players need to be initialized first. This means the status is initially unknown.

After the status is loaded, FriendStatusChanged fires once again with the up-to-date information. Though, the code only checks this updated status, when in reality it should check the change. Becoming friends with someone means to transition from not being friends to being friends. Unfriending someone means to transition from being friends to not being friends.

This behavior would also explain why, when someone rejoins, the PlayerFriendedEvent and PlayerUnfriendedEvent BindableEvents are not triggered once again. That’s because at that point the client is already aware of the friendship status between you and that person as it had already been initialized prior.

If I had to try to fix this code, I would probably try something along the lines of:

local RelationWith = {}

LocalPlayer.FriendStatusChanged:Connect(function(player, updatedStatus)
	if RelationWith[player] == Enum.FriendStatus.NotFriend and updatedStatus == Enum.FriendStatus.Friend then
		PlayerFriendedEvent:Fire(player)
	elseif RelationWith[player] == Enum.FriendStatus.Friend and updatedStatus == Enum.FriendStatus.NotFriend then
		PlayerUnFriendedEvent:Fire(player)
	end
	if updatedStatus == Enum.FriendStatus.Friend or updatedStatus == Enum.FriendStatus.NotFriend then
		RelationWith[player] = updatedStatus
	end
end)

This way the change between being friends and not being friends is made a lot more explicit.

Club_Moo · May 13, 2020, 9:50pm

Regardless of what you’ve shown, to demonstrate a problem, you’ll have to show that when Alice and Bob
are friends and Alice is in the game, then Bob joins, then Bob leaves, then unfriends Alice, then Bob
rejoins–If nothing is fired at this point, then there is a problem–Otherwise you haven’t demonstrated one.

TheFirepc_TRofical · June 28, 2022, 9:17am

Bumping this as the issue is still present despite being reported 2 years ago.

B_arocena · July 28, 2022, 4:09am

in-depth explanation and solution

I had this same issue! As Zomebody mentioned it’s to do with the core GUI events firing too often; I solved it by writing a sort of debounce wrapper around the events. This bug also mean the events mistakenly detect friends joining, I used PlayerAdded/Removed to properly get the friendship state when friends first join.

This snippet is untested, but it conveys how this should be implemented. You’d use the BindableEvents as outputs:

-- The following snippet observes the local player's friends currently in the server.
local Players = game:GetService("Players")
local StarterGui = game:GetService("StarterGui")
 
-- Custom events! This is the output of the snippet, where you'd want to connect your code.
-- 'friendAdded' will fire when a friend joins the server, or a player is friended in-game.
-- 'friendRemoved' will fire when a friend is un-friended in-game.
local friendAdded = Instance.new("BindableEvent")
local friendRemoved = Instance.new("BindableEvent")
 
-- Logic.
-- Note that 'PlayerFriendedEvent' and 'PlayerUnfriendedEvent' are currently unreliable.
-- See: https://devforum.roblox.com/t/getcores-playerfriendedevent-and-playerunfriendedevent-bindableevents-firing-at-inappropriate-times/570403/4
-- Due to players initially starting with an 'unknown' friend value, they fire for all players in the game at launch, and on the first time another player that joins the server.
-- This is unexpected, they should really be firing when the state changes between Friended <-> Unfriended!
-- Therefore, we must also use Player:IsFriendsWith() initially, and then CoreGui events just for when the state changes.
 
-- This also means we need a cache to store who's already a friend, so that we don't double-fire the events!
local friends = {}
 
-- Handle players leaving / joining.
local function handleFriendshipState(player: Player, isFriends)
    if player == Players.LocalPlayer then
        return
    end
 
    if isFriends then
        if not friends[player] then
            friends[player] = true
            friendAdded:Fire(player)
        end
    else
        if friends[player] then
            friends[player] = nil
            friendRemoved:Fire(player)
        end
    end
end
 
local function playerAdded(player: Player)
    if player ~= Players.LocalPlayer then
        -- You can't be friends with yourself!
        -- Check as :IsFriendsWith() yields.
        -- We need this as the CoreGui events aren't supposed to track friends joining, mistakenly firing the first time they enter the session.
        -- Therefore we get the initial friendship state here, then update it as needed with the CoreGui events.
        local isFriendsWith = Players.LocalPlayer:IsFriendsWith(player.UserId)
        handleFriendshipState(player, isFriendsWith)
    end
end
Players.PlayerAdded:Connect(playerAdded)
for _, player in Players:GetPlayers() do
    task.defer(playerAdded, player)
end
Players.PlayerRemoving:Connect(function(player: Player)
    handleFriendshipState(player, false)
end)
 
-- Handle changes for players already in this server.
StarterGui:GetCore("PlayerFriendedEvent").Event:Connect(function(player: Player)
    handleFriendshipState(player, true)
end)
StarterGui:GetCore("PlayerUnfriendedEvent").Event:Connect(function(player: Player)
    handleFriendshipState(player, false)
end)

I’ve implemented a working version of this for Quenty’s Nevermore library; there’s a lot of proprietary stuff to read through but it has more detailed comments on the caveats of this bug. You can read it here:

github.com

Quenty/NevermoreEngine/blob/b55f30f819115a7cf1d3e011282d86fb7ead2b16/src/friendutils/src/Client/RxFriendUtils.lua

--[[
	Utilities for observing the local player's friends.
	@class RxFriendUtils
]]

local require = require(script.Parent.loader).load(script)

local Players = game:GetService("Players")
local StarterGui = game:GetService("StarterGui")

local Brio = require("Brio")
local Maid = require("Maid")
local Observable = require("Observable")

local RxFriendUtils = {}

--[=[
	Observe friends in the current server (not including the LocalPlayer!), useful for social GUIs.

	The lifetimes exist for the whole duration another player is a friend and in your server.

This file has been truncated. show original

(posting on behalf of @OttoHatt )