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

feat: Structure actors and its packet handling #1

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

RanolP
Copy link
Member

@RanolP RanolP commented Apr 5, 2020

No description provided.

Copy link
Contributor

@kiwiyou kiwiyou left a comment

Choose a reason for hiding this comment

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

Actix를 십분 활용하기 위해서 좀 더 기능을 나누고 모델인 kiwitu를 넘어서 보다 프로덕션 환경을 생각해야 할 것 같습니다.

src/core/room.rs Outdated Show resolved Hide resolved
use std::collections::HashMap;

#[derive(Clone, Default)]
pub struct Server {
Copy link
Contributor

Choose a reason for hiding this comment

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

Actor 디자인 상 전체를 관리하는 개념이 아니라 각각의 역할 분담하는 구조라, Server를 만들기보다 User를 가지고 있는 Lobby와 Room 정보를 가지고 있는 RoomManager로 분리하면 좋을 것 같아요.

Copy link
Member Author

Choose a reason for hiding this comment

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

611fe11 에서 Lobby와 RoomManager로 분리했습니다. 더 개선이 필요할까요?

src/core/session.rs Outdated Show resolved Hide resolved
src/core/typealias.rs Outdated Show resolved Hide resolved
src/websocket.rs Outdated Show resolved Hide resolved
src/websocket.rs Outdated
*/
trace!("{}", &text);
if let Ok(message) = serde_json::from_str::<PacketClient>(&text) {
self.server.do_send(message);
Copy link
Contributor

Choose a reason for hiding this comment

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

전송 실패할 경우 (actix mailbox full 등의 이유로) remote에 알려줄 필요가 있습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

611fe11 에서 구조를 변경해, WsSession의 또 다른 Handler 구현이 처리하도록 넘겨줍니다. 더 개선이 필요할까요?

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.

2 participants