-
Notifications
You must be signed in to change notification settings - Fork 507
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
Multiplayer HITL #1939
Multiplayer HITL #1939
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments for reviewers.
for user_index, client in self._user_slots.items(): | ||
if client.connection_id == connection_id: | ||
client_state["userIndex"] = user_index | ||
# TODO: This can happen in some cases (immediately after disconnect?). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't investigated why this happens. This is rare.
for user_index in self._user_slots.keys(): | ||
if self.is_okay_to_send_keyframes(user_index): | ||
slot = self._user_slots[user_index] | ||
tasks[user_index] = slot.socket.send( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This cannot fail. Exceptions occur when awaiting task
.
raise ValueError( | ||
"isClientReady key not found in initial client message." | ||
) | ||
# Hack: connection record may be missing. Try to get it from client state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reliably occurs on AWS, but not on local network.
I haven't had the time to go to the bottom of this.
else networking_config.http_availability_server.code_available | ||
networking_config.http_availability_server.code_available | ||
if not network_mgr.has_connection() | ||
else networking_config.http_availability_server.code_unavailable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the code that determines whether a server is available. It is used by a load balancer in single-user and matchmaker in multi-user.
We only consider the server available when there is no connection.
In the multi-user case, the matchmaker dispatches every other user to the previous user's location.
# Gather all recent keyDown and keyUp events | ||
for client_state in client_states: | ||
for user_index in range(len(all_client_states)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The big deltas below are just inclusion into a loop. No other change here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file will be refactored into per-user states and a comms manager.
This will be done in a later PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class will be refactored in a later pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good. True test is whether it runs, so demo/video is good proof. 🙂
Docstrings are very informative, would love to see more where they are missing.
Thanks for the clarifying comments, overall this follows my expectations. Not enough deep context for meta/design comments. Looks functional and fine to me. 👍
# Draw a frustum (forward is flipped (z+)) | ||
self._gui_drawer.draw_transformed_line( | ||
mn.Vector3(0, 0, 0), | ||
mn.Vector3(size, size, size), | ||
color0, | ||
color1, | ||
) | ||
self._gui_drawer.draw_transformed_line( | ||
mn.Vector3(0, 0, 0), | ||
mn.Vector3(-size, size, size), | ||
color0, | ||
color1, | ||
destination_mask=server_only, | ||
) | ||
self._gui_drawer.draw_transformed_line( | ||
mn.Vector3(0, 0, 0), | ||
mn.Vector3(0, 0, pointer_len), | ||
mn.Vector3(size, -size, size), | ||
color0, | ||
color1, | ||
destination_mask=server_only, | ||
) | ||
self._gui_drawer.draw_transformed_line( | ||
mn.Vector3(0, 0, 0), | ||
mn.Vector3(-size, -size, size), | ||
color0, | ||
color1, | ||
destination_mask=server_only, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be refactored into a for
over the size vectors.
* Add max_client_count config. * Add multiplayer support to remote_client_state. * Add multiplayer support to networking process. * Make HitlDriver, ClientState and GuiDrawer fully networked. * Formatting pass. * Add missing include. * Make rearrange_v2 multiplayer. * Fixes for local mode. * Add UserData docstring.
* Add max_client_count config. * Add multiplayer support to remote_client_state. * Add multiplayer support to networking process. * Make HitlDriver, ClientState and GuiDrawer fully networked. * Formatting pass. * Add missing include. * Make rearrange_v2 multiplayer. * Fixes for local mode. * Add UserData docstring.
* Add max_client_count config. * Add multiplayer support to remote_client_state. * Add multiplayer support to networking process. * Make HitlDriver, ClientState and GuiDrawer fully networked. * Formatting pass. * Add missing include. * Make rearrange_v2 multiplayer. * Fixes for local mode. * Add UserData docstring.
Motivation and Context
This changeset introduces multiplayer HITL support.
phase2-2024-05-03_11.56.09.mp4
How It Works
Multiplayer is activated using a new
max_client_count
config value (up to 32).Overview
Server -> Client Communication
The server communicates to the clients via
gfx-replay
keyframes.Keyframes
are generated byhabitat-sim
(Recorder.cpp
).json
format.Messages
are extra info included to keyframes byhabitat-hitl
.Therefore,
keyframes
are global. They are consolidated and broadcasted.Prior to sending keyframes, the user-specific
messages
are added to them.Client -> Server Communication
The client communicates to the server via
ClientStates
andConnectionRecords
.ClientStates
are generated bysiro_hitl_unity_client
.ConnectionRecords
are sent once upon connecting.Connection
When a new connection occurs:
user_slot
(pre-allocated based onmax_client_count
).user_slot
is assigned to auser_index
, therefore a GUI-controllable agent.on_client_connected
event is emitted containing theConnectionRecord
anduser_index
.How Has This Been Tested
Types of changes
Checklist