-
Notifications
You must be signed in to change notification settings - Fork 46
[ FE ][Team 15: Yeon, Beemo, Jenny] 야구게임 구현 #35
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
Conversation
소켓 테스트
GamePage로 넘어온 왼쪽 헤더부분까지 데이터 뿌려주기
소켓 테스트
GamePage로 넘어온 왼쪽 헤더부분까지 데이터 뿌려주기
Feature/socket beemo4
* Update README.md * Update README.md * Update README.md * docs: FE 초기 폴더구조 * feat: 리액트 기본 환경 설정 * feat:[#3] teamSelect 폴더구성 및 Component 제작 중 * chore * feat : [#5] teamScore 폴더 제작 및 TeamScore Component 제작 완성 * feat: [#3] TeamSelect 초기 버전 제작 완성 및 추가 설정 - Styled-components의 ThemeProvider 적용 - 디렉토리 구조 일부 변경 * chore: .DS_Store 제거 * feat: [#11] GamePlay 컴포넌트 구조 설계 완료 - GamePlay 컴포넌트 CSS 정의 (프로토타입) - GamePlay 컴포넌트를 구성하는 모든 파일 생성 - 기능은 없음 (파일만 생성함) - App.js의 GlobalStyle 수정 - 전역으로 수직 / 수평 가운데 정렬 * feat: [#12] GamePlay - GameScore 컴포넌트 완성 (프로토타입) * feat: [#14] GamePlay - MatchScreen 컴포넌트 완성 (프로토타입) * feat: SituationScreen 프로토타입 완성 및 router 적용 - react router dom 활용 - 존재하지 않는 페이지 임시 생성 * chore: 일부 폴더 구조 변경 (TeamScore, TeamSelect) * refactor: [#24] 전체적인 CSS 보정 및 GamePlay 부분 전반적으로 수정 * feat: [#26] GamePlay - BattleGround 프로토타입 완성 * feat: [#30] Proxy 설정 및 TeamSelect API 적용 작업 중.. * feat: [#28] PlayerListPopup 프로토타입 완성 및 일부 네이밍 변경 - 일부 네이밍 변경 - TeamScore -> TeamScorePopup * style: [#28] PlayerListPopup의 Style 변경 & theme.js 일부 속성 제거 - PlayerListPopup의 StyledPlayerListPopup의 CSS 변경 * feat:[#30] API 적용 * chore * feat: [#33] 재사용 되는 PopupFrame 생성 (초기버전 & 활용) - 이 PopupFrame 컴포넌트를 활용하여 PlayerListPopup & TeamScorePopup 컴포넌트 디자인 변경 - TeamScorePopup는 테이블과 전체적 스타일 변경 필요 * feat: [#35] GamePlay BattleGround Animation 추가 * feat:[#35] 애니메이션 수정 * feat: TeamScorePopup 디자인 전체적 변경 및 임시 팝업 기능 * feat: [#40] GamePlay Component API 조작중 * feat: [#41] Popup 컴포넌트들 애니메이션 적용 및 Context 생성 & 일부 구조 변경 * feat: ([#40] 과는 거리가 먼) OAuth를 위한 Login 관련 컴포넌트 생성+ - 일부 Context에 useReducer로 적용 * feat: [#45] GameScore & MatchScreen 데이터 활용하여 표시 - 라이언 아이콘의 State도 GamePlayProvider로 이동. * feat: Oauth Login 구현 * feat: [#45] Ball, Strike, Out Count 컨트롤 (Pitch 버튼 클릭 시) - 초기 타입 * feat: [#45] BattleGround 컴포넌트 데이터 핸들링 (1) - BattleGround - GameScore: 득점 시 점수 변화 - Round: Round 상태 변화 - MatchScreen: 상황에 따라 선수 데이터 변경 - 대체적으로 GamePlayProvider와 BattleGround, GamePlay가 주로 수정되었습니다 * feat: table API 적용 * feat: 안타 or 아웃에 따라 PUT 요청, 점수판 관련 API 요청은 진행중 * chore * chore: Add background Co-authored-by: dudn1933 <dudn1933@naver.com> Co-authored-by: leehangeul <75162982+dudn1933@users.noreply.github.com>
hyeyoon
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.
안녕하세요. 리뷰를 맡은 윤입니다. 😀
리뷰가 늦었네요 😅. 죄송합니다!
야구게임 재미있게 봤습니다. 선수들이 실제로 뛰어다니는 것처럼 현실성 있는 ㅎㅎ 게임을 재밌있고, 완성도있게 잘 구현하셨어요. 👏👏 2주동안 진행하는 것이 쉽지 않았을 것 같은데 고생 많으셨습니다!!
전체적으로 회고에도 언급하신 것처럼, 데드라인이 있다보니 코드 정리 및 리팩토링을 해야할 부분이 많이 보입니다. 이후에 다른 프로젝트를 들어가더라도, 시간이 날 때 꼭 리팩토링해보시는 것을 추천합니다. 그리고 useReducer의 활용은 안보이는데, 상태관리에 장점이 많으니 꼭 GamePage 쪽 리팩토링하실 때 사용해보시는 것을 추천합니다.
프로젝트 후반부에 오다보면 그동안 지나친 매직넘버, 코드 통일성 등이 점점 늘어나다보니 리팩토링이 막막해질 때도 있는데요. 다음 프로젝트 시작하실 때는 prettier, eslint 설정도 꼭 추가해서 시작해보시면 좋을 것 같아요.
고생 많으셨습니다!
| }); | ||
|
|
||
|
|
||
| socket.on('plusMemberScore', ({ teamId, memberId, postData }) => { |
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.
[p5] 사실 eslint, prettier는 프로젝트 시작 단계에서 설정하는 것이 좋지만, 위에 기록을 보니 singlequote를 doublequote로 수정하는 등 코드 통일성을 높이려는 부분이 보여서 여기도 맞춰주면 좋을 것 같아요. 프로젝트 마무리 단계라면 전체적으로 prettier를 돌려보시는 것도 추천합니다. 😀
| ); | ||
| baseball.users = baseball.users.filter( | ||
| ({ playerId }) => playerId !== connectPlayerId | ||
| ); |
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.
[p4] 동일한 로직반복되는 부분이라 하나의 함수로 분리하고 처리해도 좋을 것 같네요.
| ...matchTeams.find((game) => { | ||
| return user.playerId === game.playerId; | ||
| }), | ||
| ...user, |
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.
[p4] 함수 로직이 복잡한데 함수로 분리해서 처리하거나, 좀 더 로직을 효율적으로 개선시켜보면 좋을 것 같아요.
| "Content-Type": "application/json;charset=utf-8", | ||
| }, | ||
| }) | ||
| } |
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.
[p5] api요청을 이렇게 분리해두고 사용하는 부분 좋네요. 👍
| }`} | ||
| description={`#${attackState === "away" | ||
| ? away.find((player) => player.position === "투수").id | ||
| : home.find((player) => player.position === "투수").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.
[p4] 여기도 동일한 로직을 사용하고 있어서 함수로 분리해서 처리하면 더 가독성을 높일 수 있어요.
| {action === 'ball' && <><NumberCircle>{(recordLength - index)}</NumberCircle>볼</>} | ||
| {action === 'hit' && <><FinishItem>안타 !</FinishItem></>} | ||
|
|
||
| <StrikeBallStatus>{action !== 'hit' && `S${strike} B${ball}`}</StrikeBallStatus> |
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.
[p4] 여기 컴포넌트들은 switch 문을 사용해서 처리해도 좋을 것 같아요. 😀
| acc.hit += cur.hit; | ||
| acc.out += cur.out; | ||
| return acc; | ||
| }, { atBat: 0, hit: 0, out: 0 }); |
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.
[p5] 전체적으로 값을 reduce를 사용해서 잘 구현하셨네요. 👍
| { ...firstRecord, records: [currentSBData, ...firstRecord.records] }, | ||
| ...remainRecords, | ||
| ]; | ||
| }); |
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.
[p4] 이 부분이 ㅎㅎ 언급하신 것처럼 시간 상 리팩토링이 안된 부분 같은데 ㅎㅎ. 알고 계시겠지만 함수 안에 로직이 너무 많습니다. 전체적으로 여기 파일에서 데이터들이 있어서 그런것 같은데, 꼭.. 함수로 분리하고, 비슷한 데이터는 묶어서 따로 파일로 분리하는 등의 과정을 해보셨으면 합니다.
| const [currentScore, setCurrentScore] = useState(0); // 현재 게임의 스코어 | ||
| const [outState, setOutState] = useState(0); | ||
| const [teamScore, setTeamScore] = useState({ home: 0, away: 0 }); | ||
|
|
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.
[p4] 전체적으로 useState를 활용해서 데이터들을 관리하고 있는데요. 비슷한 데이터를 묶어서 useReducer를 활용해서 코드를 개선해보시면 좋을 것 같아요.
| const fetchData = await (await fetch(...param)).json(); | ||
| if (fetchData.statusCode >= 400 || fetchData.body === null) throw { status: 'fail' }; | ||
| if (fetchData.statusCode >= 400 || fetchData.body === null) | ||
| throw { status: "fail" }; |
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.
[p4] api 요청이 들어가는 곳은 꼭 try...catch를 활용해서 에러처리해보시는 것을 추천합니다.
진행상황
회고
소켓통신이 둘 다 처음이라 반복적인 코드가 늘어나고 로직이 복잡해지는 것 같습니다.
원래 react에서의 데이터 처리가 쉽지 않다고 느꼈었는데 이번 미션은 더더욱 그랬던것같습니다.
제가 모르는 로직을 상대는 알고 있을 수 있다는 것을 인지하고 빠르게 질문이나 소통을 했어야 했는데, 그러지 못한 것 같아 아쉽습니다
시간 조절을 잘 했어야 했는데 잘 하지 못한듯 하여 아쉽습니다. 마지막 10분까지도 열심히 코드를 짜내는 팀원의 모습이 아주 멋있었습니다.
현재 GamePage 에서 게임 진행에 관한 대부분의 상태가 담겨있는데 시간상 리팩토링을 하지 못한점 죄송합니다 ㅠ
구현화면
_2021_05_14_22_35_35_594.mp4
전체화면
_2021_05_14_22_40_54_870.mp4