Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.Players dictionary should be private #66

Closed
asadm opened this issue May 8, 2024 · 8 comments
Closed

.Players dictionary should be private #66

asadm opened this issue May 8, 2024 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@asadm
Copy link
Collaborator

asadm commented May 8, 2024

What happened?

People should just use OnPlayerJoin callback.

Version

0.0.20

What is your environment?

No response

Link to original discussion

No response

Relevant log output

No response

@asadm asadm added the bug Something isn't working label May 8, 2024
@asadm
Copy link
Collaborator Author

asadm commented May 8, 2024

Can we also add the snippet to keep players array in unity docs?

might be something like:

object[] players = Array.Empty<object>();
...
PlayroomKit.OnPlayerJoin((player) => {
  players.push(player)
  player.OnQuit(() => {
    Debug.Log($"{player.id} quit!");
    players.Where(p => p != player).ToArray();
  });
})

@SaadBazaz
Copy link
Collaborator

Will get this done. Is there a better way we could/should handle Players @asadm ? Just wondering, we're currently maintaining it in three places.

(JS side, then Unity side, then Game side)

@SaadBazaz
Copy link
Collaborator

I've made a PR for docs (asadm/playroom-docs#43).

@momintlh - Please make the method/field private, in a PR here.

@SaadBazaz SaadBazaz assigned momintlh and unassigned SaadBazaz May 9, 2024
@momintlh
Copy link
Collaborator

momintlh commented May 9, 2024

There was a discussion in discord to use a List / Array instead of the dictionary but still keeping it public. So should I do that or just make it private for now?

@SaadBazaz
Copy link
Collaborator

Just private for now. We're providing docs to guide users.

@SaadBazaz
Copy link
Collaborator

We will revisit the idea of List / Array in another release.

momintlh added a commit that referenced this issue May 9, 2024
- RPC events name sync between players.
- Made Players dictionary private #66
@SaadBazaz
Copy link
Collaborator

Completed in #67

@SaadBazaz
Copy link
Collaborator

@asadm Is GetPlayers() supposed to be private?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants