-
Notifications
You must be signed in to change notification settings - Fork 3
[feat] 게임 종료 구현 #109
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] 게임 종료 구현 #109
Conversation
| player.initializeCorrectCount(); | ||
| player.toggleReady(); | ||
| } | ||
|
|
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.
disconnect 플레이어들은 continue;를 추가해서
아래initializeCorrectCount(); toglleReady(); 를 실행 안하게 만들어도 되지 않을까요 ?.?
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.
room 내부 로직으로 변경하면서, filter()로 disconnectedPlayers에 해당 플레이어가 속해있는지 판단하고 레디 여부 등을 실행하지 않게 구현할 수도 있었지만, contains()를 두고 비교하는 과정이 한 번 더 들어가야 한다는 점이 복잡도에서 큰 이점이 있다고 생각되지 않아 먼저 상태를 다 init 상태로 돌려두고 disconnectedPlayers들을 처리하는 방식으로 구현했습니다 !
| } | ||
|
|
||
| public void gameEnd(Room room) { | ||
| room.updateRoomState(RoomState.FINISHED); |
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.
finished가 있다면 현재 입장시에 waiting일때만 입장 가능하게 만들어야겠군요!
현재는 Playing이면 예외처리를 해놓은 상태입니다.
결과 보여주는 3초? 일때는 finished 유지해서 입장을 막고, waiting으로 바뀌었을때 입장 가능하게 만드는게
유저 경험을 고려했을때 더 깔끔하지 않을까 하는 생각입니다!
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.
해당 조건에 대한 예시를 생각해 봤을 때 카트라이더가 생각났습니다.
카트라이더는 게임이 끝나 순위가 나오는 동안은 다른 유저들이 방에 입장할 수 있는 것으로 알고 있습니다.
뇌이싱의 경우에도 결과 순위가 보이는 동안 유저가 입장하는 것은 논리상으로는 문제 되지 않을 것이라 생각되지만,
결과 화면에서 @@ 님이 입장했습니다와 함께 새로운 참여자의 채팅이 노출된다면 조금 당황스러울 것 같기는 합니다.
추가로, FINISHED 상태를 추가한다면 방 목록의 입장에서는 어떻게 보여줄 것인지에 대한 고민도 필요할 것 같습니다.
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.
-> waiting, playing 상태만 유지하는 것이 고민거리를 줄일 수 있지 않을까 생각됩니다.
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.
세희님, 경찬님의 의견을 듣고 고민을 해봤는데, 확실히 finished 상태까지 관리하게 되면 분기처리해줘야 할 사항들이 많다고 생각했습니다.
사실상 게임이 끝나고 waiting 상태로 변경해준다하더라도, 누군가 나가야 게임 방에 들어올 수 있게 되는 것이고, 게임이 끝나고 게임 결과 창을 닫는 타이밍은 플레이어 개별에게 달려있는 것이라 바로 waiting 상태로 변경 후, 기존 게임 대기 상태에서 이뤄질 수 있는 것들이 똑같이 이뤄져도 괜찮겠다 생각하여 상태를 두 가지로 두는 방향으로 변경했습니다 !
| if (room.isHost(player.getId())) { | ||
| changeHost(room, sessionId); | ||
| } | ||
|
|
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.
방 삭제와 방장 변경은 동시성이 필요해보입니다!
제가 이번 pr에서 방장 변경은 connect 상태인 유저만 대상이 될 수 있게 로직을 추가하긴했는데...
만약 방장이 중간에 나간다면 게임 종료 후에 exitRoomForDisconnectedPlayer 이 로직을 타야 방장이 바뀌겠군요 ,,,
그렇다면 이 로직도 동시성 적용이 되어야 할듯 합니다.
다른 유저랑 동시에 나가면 방 삭제, 방장 변경이 제대로 되지 않을 것 같습니다.
| room.removeSessionId(sessionId); | ||
| } | ||
|
|
||
| public void exitRoomForDisconnectedPlayer(Long roomId, Player player, String sessionId) { |
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.
[L4-변경제안]
exitRoom과 겹치는 코드는 공통 메서드로 따로 분리해서 사용해도 좋을것 같습니다!
| toQuestionStartResponse(room, START_DELAY)); | ||
| } | ||
|
|
||
| public void gameEnd(Room room) { |
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.
[L5-참고의견]
랭킹 정보 업데이트에 대한 TODO 주석을 달아두는 것도 좋을 것 같습니다.
|
|
||
| room.initializeRound(); | ||
| for (Player player : playerSessionMap.values()) { | ||
| if (player.getState().equals(ConnectionState.DISCONNECTED)) { |
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.
Player의 내부 메서드로 분리하면 좋을 것 같습니다 !
| } | ||
|
|
||
| for (Player player : disconnectedPlayers) { | ||
| String sessionId = room.getUserIdSessionMap().get(player.id); |
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.
이 부분도 Room 내부 메서드로 분리할 수 있을 것 같습니다 !
| int rank = 0; | ||
|
|
||
| List<GameResultResponse> gameResults = new ArrayList<>(); | ||
| for (int i = 0; i < totalPlayers; i++) { |
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.
해당 반복문을 메서드로 분리하면 GameResultListResponse로 변환하는 동작이 더 잘 드러날 것 같습니다 !
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.
Mapper에는 오로지 매핑 (A->B) 의 역할을 가진 메서드만 존재해야 한다고 생각하여, 구분해두지 않았는데, 가독성면에서 분리하는 것이 맞다고 생각해서 분리했습니다.
또, 해당 계산 메서드를 도메인에 빼는 것은 확실히 책임 분리가 되지 않는 것이라 생각되어 매퍼 클래스 안에서 private으로 두는 것으로 책임과 가독성을 균형을 최대한 맞추는 방향으로 분리했습니다 !
|
크게 있었던 이슈가 서비스 간의 순환 참조였는데, 이 부분이 게임을 시작하고, 끝내고, 타이머로 체크하고, 룸에서 연결 끊긴 플레이어를 나가기 처리해주는 로직이 너무 한 번에 이어지고 서로 이어져있어서 발생하는 이슈였습니다. 그래서 타임 아웃이 됐을 때나 정답을 맞혔을 때 이벤트를 발행하고, 그 이벤트를 처리하는 식으로 서비스 간의 참조를 줄였습니다. 그리고, 그 이벤트가 발행됐을 때는 게임 내에서의 로직 (정답 처리, 혹은 타임아웃 처리)이니까 GameService에서 핸들링해주도록 구현했습니다. 또한, 채팅이라는 기능은 RoomService에만 속한다기보단, 게임에도 밀접이 되어있다고 생각하여, ChatService로 따로 빼서 구현했습니다. |
| public enum RoomState { | ||
| WAITING, | ||
| PLAYING, | ||
| FINISHED |
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.
결국 FINISHED는 빠진건가요?.?
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.
아! 이전 리뷰에 답글 달았는데, 다시 달아드립니다!
세희님, 경찬님의 의견을 듣고 고민을 해봤는데, 확실히 finished 상태까지 관리하게 되면 분기처리해줘야 할 사항들이 많다고 생각했습니다.
사실상 게임이 끝나고 waiting 상태로 변경해준다하더라도, 누군가 나가야 게임 방에 들어올 수 있게 되는 것이고, 게임이 끝나고 게임 결과 창을 닫는 타이밍은 플레이어 개별에게 달려있는 것이라 바로 waiting 상태로 변경 후, 기존 게임 대기 상태에서 이뤄질 수 있는 것들이 똑같이 이뤄져도 괜찮겠다 생각하여 상태를 두 가지로 두는 방향으로 변경했습니다 !
|
|
||
| String answer = currentQuestion.getAnswer(); | ||
|
|
||
| if (answer.equals(chatMessage.message())) { |
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.
동시성 처리가 redis로 변환될 예정이니 chat을 서비스로 분리한 점 넘 좋네욥!
정답일 경우 구현해놓은 로직은 eventPublisher로 구현하신 이유가 있을까요?
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.
앗 위에 리뷰 확인했습니다 !
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.
서비스 간의 순환 참조가 발생하면서, 게임 진행의 과정이 다른 서비스와 너무 연결되어 사용된다고 생각했습니다.
기존 코드 기준 예를 들어, 채팅 (정답) (RoomService)-> 타이머 취소 (TimerService) -> 마지막 라운드 시 게임 종료 (GameService) -> Disconnected 된 플레이어 처리 (RoomService) 이런 식으로 서비스들이 서로 깊게 연결되고, 흐름 제어가 분산되어 있어서 유지보수와 테스트가 어렵다고 생각했습니다.
그래서 정답 맞힘/타임아웃과 같은 주요 게임 이벤트를 ApplicationEventPublisher를 통해 GameService에서 받아서 처리하도록 변경했습니다. 정답을 맞혔을 때, 타임아웃이 됐을 때, 두 개의 상황에서 GameService에서 서로 연결되게 호출하는 방식이 아닌 이벤트 핸들링을 통해 GameService에서 핸들링해줄 수 있도록 하기 위해 이벤트 퍼블리싱 방식으로 구현했습니다 !
* ✨ feat: 특정 닉네임이 포함된 유저 정보 조회 기능 * 🐛 fix: 닉네임 중복 확인 시 대소문자 구분하지 않음 * chore: Java 스타일 수정 --------- Co-authored-by: github-actions <>
* ✨ feat: 게임 설정 변경 인터페이스 및 구현체 추가 - GameSettingChanger 인터페이스 도입 * chore: Java 스타일 수정 * ✨ feat: 게임 설정 변경 기능 추가 * chore: Java 스타일 수정 * 🚚 move: 소켓 요청 DTO request 디렉토리로 이동 * 🚚 move: 소켓 요청 DTO 경로 변경 * chore: Java 스타일 수정 * ♻️ refactor: 게임 세팅 요청 후처리 분리 * chore: Java 스타일 수정 * ♻️ refactor: 코드 리뷰 반영 * chore: Java 스타일 수정 * 🗑️ remove: 불필요 코드 삭제 * chore: Java 스타일 수정 * ♻️ refactor: isHost 원상복구 * ♻️ refactor: QuizChangeRequest, RoundChangeRequest 타입 수정 * 🔧 chore: RoomUpdatedEventListener, RoomDeletedEventListener 빈 등록 * chore: Java 스타일 수정 * chore: Java 스타일 수정 * ♻️ refactor: player 조회 메서드 추가 --------- Co-authored-by: github-actions <>
* 🐛 fix: round 변경 시 question 조회에 fetch join 적용 * ♻️ refactor: questionCount만 찾는 쿼리로 변경 및 불필요한 퀴즈 아이디 조회 삭제 * chore: Java 스타일 수정 --------- Co-authored-by: github-actions <>
sehee123
left a comment
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.
수고하셨습니다!
엄청난 ,, 코드입니다 🚀
dlsrks1021
left a comment
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.
EventPublisher로 각 이벤트를 분리해서 구현하신 것이 순환 참조에서의 좋은 해결책이 된 것 같습니다!
jiwon1217
left a comment
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.
고생하셨습니다 !
LimKangHyun
left a comment
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.
고생하셨습니다!
🛰️ Issue Number
🪐 작업 내용
게임 종료 구현
TODO 🥲
📚 Reference
✅ Check List