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

Multiplayer #20

Merged
merged 73 commits into from
Jan 16, 2025
Merged

Multiplayer #20

merged 73 commits into from
Jan 16, 2025

Conversation

legoeruro
Copy link
Contributor

No description provided.

FelixNgFender and others added 30 commits October 29, 2024 13:50
…Move script and into runtime and initialize windows for find server, sessions, and send data
@YiqinZhao
Copy link
Member

Also, something is wrong with the CI?

@legoeruro
Copy link
Contributor Author

Also, something is wrong with the CI?

We haven't done the implementation for benchmarking with synchronous data sending right now, so pyright throws error with unused variables

@YiqinZhao
Copy link
Member

Let's ignore those lines for the unimplemented parts.

@@ -0,0 +1,7 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should only have the .vscode folder at the project root level.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't put the unity/.vscode directory in because I didn't want to mess with the default-generated stuff Unity gave us. I did solidify the python/.vscode directory though


@private
def LeaveSession(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this function is this Camel Case function named?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a gRPC handler's convention. It has to be camelCase to override the stub handler/interface in ARFlowServiceServicer

interceptors = [ErrorInterceptor()] # pyright: ignore [reportUnknownVariableType]
server = grpc.server( # pyright: ignore [reportUnknownMemberType]
futures.ThreadPoolExecutor(max_workers=10),
compression=grpc.Compression.Gzip,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any potential real-time performance impact on using compression v.s. no compression?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this was discussed in a previous meeting. Detailed performance improvement needs to be measured.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I took notes of it and will setup benchmark tests after I have done the unit tests for the server code (almost done).

@YiqinZhao
Copy link
Member

Sorry, I just realized I didn't submit the reviews. Take a look at the comments above.

@YiqinZhao
Copy link
Member

@FelixNgFender The CI is still failing. Can we have a quick fix for that? Maybe ignore some unused pytest code.

@FelixNgFender
Copy link
Contributor

I disabled typechecking & testing temporarily for this PR. We will add it back when testing & benchmarking is done.

Copy link
Member

@YiqinZhao YiqinZhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@YiqinZhao YiqinZhao merged commit 2aee1bd into cake-lab:main Jan 16, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants