Sample/group up flow#31
Conversation
orcist
left a comment
orcist
left a comment
There was a problem hiding this comment.
Leaving a few refactoring suggestions and some requests for changing the sample flow, let me know when ready for another review! Well done on the first submission. 💪
| public bool DeleteTicketOnPause = false; | ||
| public bool DeleteTicketOnQuit = true; |
There was a problem hiding this comment.
| public bool DeleteTicketOnPause = false; | |
| public bool DeleteTicketOnQuit = true; | |
| public bool DeleteGroupOnPause = false; | |
| public bool DeleteGroupOnQuit = true; |
| [Header("Logging")] | ||
| public bool LogTicketUpdates = true; | ||
| public bool LogAssignmentUpdates = true; | ||
| public bool LogPollingUpdates = false; |
There was a problem hiding this comment.
| public bool LogPollingUpdates = false; | |
| public bool LogGroupUpdates = true; |
probably some leftover from earlier versions?
| && message.Contains("created") | ||
| ) | ||
| { | ||
| StatusDisplay.text = $"Group created, awaiting teammates.\nID: {group.Current.GroupID}"; |
There was a problem hiding this comment.
| StatusDisplay.text = $"Group created, awaiting teammates.\nID: {group.Current.GroupID}"; | |
| StatusDisplay.text = $"Group created, awaiting members.\nID: {group.Current.GroupID}"; |
| && message.Contains("joined") | ||
| ) | ||
| { | ||
| StatusDisplay.text = "Group joined, awaiting other ready teammates."; |
There was a problem hiding this comment.
| StatusDisplay.text = "Group joined, awaiting other ready teammates."; | |
| StatusDisplay.text = "Group joined, waiting for group owner to set ready."; |
|
|
||
| if ( | ||
| action == ObservableActionType.Update | ||
| && message.Contains("created") |
There was a problem hiding this comment.
Shouldn't we disable the join input and button as well? When in a group, players are expected to either continue to matchmaking or leave the group before joining another group.
There was a problem hiding this comment.
Actually, maybe you can merge this and the group join blocks together and include even the UI related code from StartMatchmaking. After all the changes I suggested, the only difference between create and join is the the displayed text and the log, could put that bit in an if statement...
There was a problem hiding this comment.
the buttons/input field actually get disabled in StartMatchmaking; the SetActive() calls are to switch between which buttons are displayed depending on if you're using the owner or member flow (so: no need to switch Create Group to Delete Group if you're not an owner)
| JoinGroupButton.gameObject.SetActive(false); | ||
| IdInputField.gameObject.SetActive(false); | ||
|
|
||
| LeaveGroupButton.gameObject.SetActive(true); |
There was a problem hiding this comment.
probably need to disable delete group button too, no?
| SetReadyButton.gameObject.SetActive(true); | ||
| SetReadyButton.interactable = true; | ||
|
|
||
| DeleteGroupButton.gameObject.SetActive(true); |
There was a problem hiding this comment.
probably need to disable leave group button also, no?
| public void StartMatchmaking(Dictionary<string, float> pings, bool isReady) | ||
| { | ||
| CreateGroupButton.interactable = false; | ||
| JoinGroupButton.interactable = false; | ||
| IdInputField.interactable = false; | ||
|
|
||
| MatchmakingClient.CreateGroup(new MyGroupUpRequestDTO(pings, isReady)); | ||
| } | ||
|
|
||
| public void StartMatchmaking(Dictionary<string, float> pings, bool isReady, string groupID) | ||
| { | ||
| CreateGroupButton.interactable = false; | ||
| JoinGroupButton.interactable = false; | ||
| IdInputField.interactable = false; | ||
|
|
||
| MatchmakingClient.JoinGroup(new MyGroupUpRequestDTO(pings, isReady), groupID); | ||
| } |
There was a problem hiding this comment.
| public void StartMatchmaking(Dictionary<string, float> pings, bool isReady) | |
| { | |
| CreateGroupButton.interactable = false; | |
| JoinGroupButton.interactable = false; | |
| IdInputField.interactable = false; | |
| MatchmakingClient.CreateGroup(new MyGroupUpRequestDTO(pings, isReady)); | |
| } | |
| public void StartMatchmaking(Dictionary<string, float> pings, bool isReady, string groupID) | |
| { | |
| CreateGroupButton.interactable = false; | |
| JoinGroupButton.interactable = false; | |
| IdInputField.interactable = false; | |
| MatchmakingClient.JoinGroup(new MyGroupUpRequestDTO(pings, isReady), groupID); | |
| } | |
| public void StartMatchmaking(Dictionary<string, float> pings, bool isReady, string groupID = null) | |
| { | |
| if (groupID is null) { | |
| MatchmakingClient.CreateGroup(new MyGroupUpRequestDTO(pings, isReady)); | |
| } else { | |
| MatchmakingClient.JoinGroup(new MyGroupUpRequestDTO(pings, isReady), groupID); | |
| } | |
| } |
removes some code duplication, assuming the UI related code will move to the observable callback
| public void BecomeReady() | ||
| { | ||
| SetReadyButton.interactable = false; | ||
| MatchmakingClient.SetReady(true); | ||
| } |
There was a problem hiding this comment.
You can also un-ready yourself, could we add support for that? Basically just flip the boolean to the opposite value without disabling the button. Maybe we could change the button text to indicate whether you're ready or not (e.g. "waiting" when the player is ready, and "set ready" when the player is not ready).
2 participants
No description provided.