Photon team based game, mantaining state across players

So I implemented my own teams solution, meaning that when a player joins it gets assigned a team by the master client and that's broadcasted to all the other players, that's done because if master client disconnects, the next player needs to know about the teams states.

Here is a little bit of code.
using System;
using System.Collections;
using System.Collections.Generic;

using UnityEngine;

using Photon.Pun;
using Photon.Pun.UtilityScripts;
using Photon.Realtime;
using System.Linq;
using RootMotion.Dynamics;

using PhotonHashTable = ExitGames.Client.Photon.Hashtable;

[Serializable]
public class Team : PhotonTeam
{
    public Color Color;
    public int Score;
    public List<Player> Players = new List<Player>();
}

public abstract class TeamBasedGameMode : GameMode
{
    public Team[] Teams;

    private IEnumerator ChangePlayerMeshColor(string playerId, Color color)
    {
        // Not relevant to this topic.
    }

    [PunRPC]
    public void SetPlayerTeam(string playerId, byte teamCode)
    {
        Debug.Log("SetPlayerTeam");

        var team = Teams.First(x => x.Code == teamCode);

        if (!team.Players.Any(x => x.UserId == playerId))
        {
            team.Players.Add(PhotonNetwork.PlayerList.FirstOrDefault(x => x.UserId == playerId));
        }

        StartCoroutine(ChangePlayerMeshColor(playerId, team.Color));
    }

    private void AssignTeamToNewPlayerLocally(Player newPlayer)
    {
        var teamWithLessPlayers = Teams.OrderBy(x => x.Players.Count).First();

        teamWithLessPlayers.Players.Add(newPlayer);
    }

    private void UpdateNewPlayerToOldPlayers(Player newPlayer)
    {
        Debug.Log("UpdateNewPlayerToOldPlayers");

        Debug.Log("RPC -> SetPlayerTeam to ALL");
        photonView.RPC("SetPlayerTeam", RpcTarget.All, newPlayer.UserId, Teams.FirstOrDefault(x => x.Players.Contains(newPlayer)).Code);
    }

    private void UpdateOldPlayersToNewPlayer(Player newPlayer)
    {
        Debug.Log("UpdateOldPlayersToNewPlayer");

        foreach (var team in Teams)
        {
            var oldPlayers = team.Players;

            foreach(var oldPlayer in oldPlayers)
            {
                Debug.Log("RPC -> SetPlayerTeam to " + newPlayer);
                photonView.RPC("SetPlayerTeam", newPlayer, oldPlayer.UserId, team.Code);
            }
        }
    }

    public override void OnPlayerEnteredRoom(Player newPlayer)
    {
        if (!PhotonNetwork.IsMasterClient)
            return;

        UpdateOldPlayersToNewPlayer(newPlayer);

        AssignTeamToNewPlayerLocally(newPlayer);
        UpdateNewPlayerToOldPlayers(newPlayer);
    }

    public override void OnJoinedRoom()
    {
        if (!PhotonNetwork.IsMasterClient)
            return;

        AssignTeamToNewPlayerLocally(PhotonNetwork.LocalPlayer);
        StartCoroutine(ChangePlayerMeshColor(PhotonNetwork.LocalPlayer.UserId, Teams.First(x => x.Players.Contains(PhotonNetwork.LocalPlayer)).Color));
        SetPlayerTeam(PhotonNetwork.LocalPlayer.UserId, Teams.First(x => x.Players.Contains(PhotonNetwork.LocalPlayer)).Code);
    }

    public override void OnPlayerLeftRoom(Player otherPlayer)
    {
        var team = Teams.FirstOrDefault(x => x.Players.Contains(otherPlayer));

        team.Players.Remove(otherPlayer);
    }
}

Now, my question is, is there a better way? For example using room properties. I know about PhotonTeamsManager, but it relies on setting properties remotely so there is a delay until they take effect, meaning two players could get added to the same team when they should be in separate teams(by adding each player to the team with less players)

Comments

  • PhotonUser
    edited July 2021
    EDIT: Simpler code.
    public Team[] Teams;
    
        private IEnumerator ChangePlayerMeshColor(string playerId, Color color)
        {
            //
        }
    
        [PunRPC]
        public void SetPlayerTeam(Player player, byte teamCode, string calledFrom)
        {
            var team = Teams.First(x => x.Code == teamCode);
    
            team.Players.Add(player);
    
            StartCoroutine(ChangePlayerMeshColor(player.UserId, team.Color));
    
            Debug.Log("Team " + teamCode + " has " + team.Players.Count);
        }
    
        private Team GetSmallestTeam()
        {
            return Teams.OrderBy(x => x.Players.Count).First();
        }
    
        public override void OnJoinedRoom()
        {
            base.OnJoinedRoom();
    
            if (!PhotonNetwork.IsMasterClient)
                return;
    
            SetPlayerTeam(PhotonNetwork.LocalPlayer, GetSmallestTeam().Code, "OnJoinedRoom()");
        }
    
        private void UpdateNewPlayerAboutOldPlayers(Player newPlayer)
        {
            foreach(var team in Teams)
            {
                foreach(var oldPlayer in team.Players)
                {
                    photonView.RPC("SetPlayerTeam", newPlayer, oldPlayer, team.Code, "UpdateNewPlayerAboutOldPlayers()");
                }
            }
        }
    
        public override void OnPlayerEnteredRoom(Player newPlayer)
        {
            base.OnPlayerEnteredRoom(newPlayer);
    
            if (!PhotonNetwork.IsMasterClient)
                return;
    
            UpdateNewPlayerAboutOldPlayers(newPlayer);
    
            photonView.RPC("SetPlayerTeam", RpcTarget.All, newPlayer, GetSmallestTeam().Code, "OnPlayerEnteredRoom()");
        }
    
        public override void OnPlayerLeftRoom(Player otherPlayer)
        {
            base.OnPlayerLeftRoom(otherPlayer);
    
            var team = Teams.First(x => x.Players.Contains(otherPlayer));
            team.Players.Remove(otherPlayer);
        }
    
  • Nobody? I mean, just some pointers like:

    - You should use room properties for that

    or

    - That approach is fine.
  • Tobias
    Tobias admin
    edited July 2021
    I thought I answered this or a very similar question recently. Maybe not here.

    In PUN, you should use properties, yes. Custom Player Properties will stick with each player, so you don't need to worry about cleaning up or sending again (when anyone joins).
    As you figured out, when you use RPCs, you need to send them whenever someone joins/leaves.
    I didn't read the code thoroughly but you send RPCs to each player individually. This is likely not needed. Send the RCP to all other players instead. This uses the effectiveness of the server.
    In the second piece of code, you call the RPC method locally (directly) instead of using photonView.RPC(method, ...) which means it's not called via the network.

    There is a teams implementation in PUN 2, which you should take a look at.
  • JohnTube
    JohnTube ✭✭✭✭✭
    edited July 2021
    Now, my question is, is there a better way? For example using room properties. I know about PhotonTeamsManager, but it relies on setting properties remotely so there is a delay until they take effect, meaning two players could get added to the same team when they should be in separate teams(by adding each player to the team with less players)

    Hey @PhotonUser,

    sorry for the delay on this one

    Did you read this (source):
    By default, setting properties for actor or room properties will not take effect on the sender/setter client (actor that sets the properties) immediately when joined to an online room unlike what it used to be in PUN Classic. Now, instead, the sender/setter client (actor that sets the properties) will wait for the server event PropertiesChanged to apply/set changes locally. So you need to wait until OnPlayerPropertiesUpdate or OnRoomPropertiesUpdate callbacks are triggered for the local client in order to access them. The new behaviour is due to the introduction of the new room option flag roomOptions.BroadcastPropsChangeToAll which is set to true by default. The reason behind this is that properties can easily go out of synchronization if we set them locally first and then send the request to do so on the server and for other actors in the room. The latter might fail and we may end up with properties of the sender/setter client (actor that sets the properties) different locally from what's on the server or on other clients. If you want to have the old behaviour (set properties locally before sending the request to the server to synchronize them) set roomOptions.BroadcastPropsChangeToAll to false before creating rooms. But we highly recommend against doing this.

    if roomOptions.BroadcastPropsChangeToAll = false is not a solution:

    Here is how I would do this using custom properties:
    - use ROOM properties and not actor properties unlike PhotonTeamsManager. Why? because we will be adding a team members counter property to use it for concurrency to avoid two players trying to join or leave a team at the same time. And SetProperties allows to set EITHER room properties XOR actor properties at once (in the same single request). and if we split this into two requests (one for team members count room property and another request for one for setting the actual team a player is joined to (or not) as actor property of that player) we will add more complexity and more delay.
    - so at the end you will be using X room properties:
    X = N + T
    N = number of joined actors (up to MaxPlayers if != 0); one property per joined actor, the property key should be the stringified actor number + prefix or suffix maybe to make it unique and not clash with other properties. Example: "a0". Value should be the team code (byte): "a0": 1.
    T = number of teams; one property per team, use team name or stringified team code + prefix or suffix to make it unique and not clash with other properties. Example: "t0". Value should be integer.

    Whenever a player joins or leaves a team, call Room.SetCustomProperties and set two new properties: the room property for the actor's team assignment + room property for the team members count. Use ExpectedProperties (CAS) at least with the OLD/CURRENT team members count.

    When a room is created you should initialize the room properties of the team members count to 0s.

    As a bonus you could make use of the team members' count properties for matchmaking (SQL).
  • PhotonUser
    edited July 2021
    Thanks for both of your answers.
    Ok so what happens when two players try to join the team with the least members at the same time, wouldn't they join the same team because of the delay from the server? From what I see that gets fixed by using expectedProperties parameter, I assume I would get some sort of failed properties when the properties on the server do not satisfy the sent expectedProperties?

    My approach only allows the master client to add players to teams, so it keeps a local Team[] array to keep track of instant changes, then It sends these changes to the other players and when this master client disconnects, then any other player can safely become the master client and take care of the team assignment.

    EDIT:

    Maybe I am misunderstanding a little bit and I think using CustomRoomProperties is advised as the propagation method instead of RPC's for this case, so I could still maintain a local copy of the current CustomRoomProperties for instant updates when the master client is assigning teams, so players don't run the risk of joining the same team.
  • JohnTube
    JohnTube ✭✭✭✭✭

    The solution I suggested which is the proper one to handle concurrent team members assignments using ExpectedUsers (CAS) will allow only one player to proceed and the other call will fail. But you need to send ALL teams' members count properties in the request as expected users I think as the two players could be trying to join two different teams, not sure if you want to allow that or not.

    Read more about CAS.

  • PhotonUser
    edited July 2021
    JohnTube wrote: »
    The solution I suggested which is the proper one to handle concurrent team members assignments using ExpectedUsers (CAS) will allow only one player to proceed and the other call will fail. But you need to send ALL teams' members count properties in the request as expected users I think as the two players could be trying to join two different teams, not sure if you want to allow that or not.

    Read more about CAS.

    Isn't it just easier to keep a local state of the teams and apply this logic locally on the master client and let him decide which team a player should be instead of relying on an error message from the server? Because if the master client takes care of the "sorting" part, it will know instantly which team has the least players.

    But of course maybe there are some cons to this approach and if there are you could tell me
  • Ok so I tried implementing this concept using room properties.

    I am not gonna bother posting code since I figured it's too much time consuming for a forum post, so I am gonna spec out what I did.

    If client is the master client, it will handle OnJoinedRoom and OnPlayerEnteredRoom callbacks, so it can add himself and joining players to teams. The master client assigns players to the correct team on the Teams[] array, so it is constantly maintaining Teams[] state locally, after modifying the correct Team with the new player it will propagate these changes as CustomRoomProperties, for example:

    Team_0_Players = userId1,userId2,userId3...

    I read that the least you send on room properties the better it is (if somebody could explain me to why is that would be good, because it is not as we are sending this info 10x a second), so I could abbreviate to t0p and use the Player's ActorNumber instead.

    The other clients handle OnRoomPropertiesUpdate to listen for changed properties, the they iterate over the changed properties, extract the team code with a string split and use that to find the correct team on its local Teams[] array, then it will extract the playerIds into a string array from property.Value, by doing that it is able to check if that team in the current iteration contains this player, if it does it will just skip, if it doesn't it will add the player to the team.

    By doing all this the master client and normal clients will maintain the state locally and will be ready to act as master clients too if the current one disconnects.

    What I noticed is that it is way easier to just use RPCs to propagate changes to other players, instead of doing all that string manipulation on OnRoomPropertiesUpdate for normal clients, to figure out the team for each player, with RPCs that would just be the master client sending a message to normal clients saying "hey just add this player to this team" or "hey just remove this player from this team", which is way simpler.

    What I guess is the ideal solution for me (and what makes most sense in my mind), is to use RPCs to propagate team changes to players AND use CustomRoomProperties to allow players in lobby searching for matches to filter rooms based on "free slots in a team" if they want to matchmake with friends and join an on progress match or whatever.
  • Thanks for the update.
    You can use the ActorNumber instead of userIDs and that means you could send the teams as (e.g.) 2 different int[]. Either in properties or in an RPC. As long as it works in your case, it will be fine...

    > I read that the least you send on room properties the better it is (if somebody could explain me to why is that would be good, because it is not as we are sending this info 10x a second)

    This is the usual optimization approach: The more frequent you have to send something, the more you should optimize it. Avoid huge amounts of data as much as you can and keep the frequent updates lean and you will be fine.
  • My team sorting script runs only on the master client listening for OnPlayerJoinedRoom, so What happens when a player joins at the same time as the Master Client disconnects?
  • Then someone else becomes Master Client and has to sort the teams.
  • PhotonUser
    edited July 2021
    @JohnTube
    JohnTube wrote: »
    The solution I suggested which is the proper one to handle concurrent team members assignments using ExpectedUsers (CAS) will allow only one player to proceed and the other call will fail. But you need to send ALL teams' members count properties in the request as expected users I think as the two players could be trying to join two different teams, not sure if you want to allow that or not.

    Read more about CAS.

    Any drawbacks I should consider if I plan on delegating concurrency issues to the master client instead of using CAS? The master would maintain state locally and update it to the other clients as well. But as you said CAS is the proper solution to handle such scenarios, so I wonder if my approach is equivalent.
  • JohnTube
    JohnTube ✭✭✭✭✭
    Hi @PhotonUser,

    Sorry for the delay.

    Using room or player properties makes team assignment persist.
    Using CAS feature makes team assignment "concurrency safe".

    Using Master Client to assign teams is optional.
    It can help reduce team assignment concurrency issues by allowing only one central node to handle this.

    The drawback of using Master Client for this is some edge cases where Master Client is not responsive and not disconnected/switched yet.

    You also need to handle Master Client switch anyway.
  • PhotonUser
    edited July 2021
    Thanks for your answer @JohnTube

    Now, one more question

    What are the drawbacks to only using RPC's approach and the MasterClient for this team sorting stuff (instead of CAS and customproperties)
    I know I gotta send the updates to all players to maintain state, and also send these updates also as soon as a new player joins, kinda working like room's CustomProperties.

    But one drawback that I see is that when a player joins and at the same time the master disconnects, will the new master send the updates fast enough or that could cause me some problems?

  • JohnTube
    JohnTube ✭✭✭✭✭
    Yes, edge case is when master client is disconnected, disconnecting, not responsive, etc.