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

2주차 스프린트 #54

Merged
merged 89 commits into from
Nov 26, 2020
Merged

2주차 스프린트 #54

merged 89 commits into from
Nov 26, 2020

Conversation

sbyeol3
Copy link
Collaborator

@sbyeol3 sbyeol3 commented Nov 26, 2020

💁 설명

2주차 스프린트 배포를 위한 마스터 머지

📑 체크리스트

구현한 목록 체크리스트

🚧 주의 사항

PR을 읽을 때 살펴볼 사항

ramram1048 and others added 30 commits November 23, 2020 10:04
- src/backend 폴더에 .env파일을 만들어서 process.env를 참조하면 됨
master에 merge해버렸던거 develop에넣어요
- build시 game.html으로 새 페이지 번들링
- 개발서버 실행 시 /game.html 으로 접근 가능
- 기존 game.html -> game/index.html 변경
- 이미지 파일 로드를 위한 웹팩 로더 추가
- 백그라운드 이미지 추가
- 폰트 추가
- 메인페이지와 관련된 소스를 /main 폴더로 분리
- babel과 함께 fetch 사용시 regeneratorruntime 에러 발생하는것 대응
- 명세: param에 { method, uri, ...기타 설정값 } 넣으면 json을 알아서 가져옴
- 버튼, form에 id 부여, eventListener 연결
- TODO: 에러 컨트롤
- json에 추가되지 않아 설치가 안되어 오류나는 문제 있었음
- devdepedency로 설치
- 방 코드에 대한 request 결과가 ok가 아니면 에러문구 렌더링
- 글씨가 잘 안보여서 백그라운드 이미지 투명도 조절
- input text align 수정
- requesthandler 변수 네이밍 변경
- 함수 이름 구체적으로 수정
[Feat/index page] 메인 페이지 구현 #10
- 너무 구버전이었음~~
- 이전에 fetch관련 이슈로 있었던 babel 플러그인도 uninstall함
- 앞으로 자주 접근하게 될 game 폴더에 대한 alias 추가
- nodemon 설치 및 script 추가
- setting.json 설정
- 쓸데없는 에러를 방지하기 위한 rules 추가
- 하나의 게임을 관리하기 위한 객체 생성
- roomID를 생성하기 위한 함수 존재
- 게임방 생성 및 입장을 위한 route 설정 및 로직 구현
- 변수 이름 및 메소드 이름 변경
- topic default 값 변경
- 변수 이름 및 메소드 이름 변경
- topic default 값 변경
- 필요없는 주석 제거
[BE] 게임방 생성/입장 로직 구현

close #13 #15
jinhyukoo and others added 27 commits November 26, 2020 13:22
- info, error level에 관련해서만 설정했음
- 폴더 네이밍 변경
메인페이지의 HTTP Request가 대응되도록 코드 일부 수정
- cors 오리진 넣어두는 키 dotenv에 추가
- js 이용하지 않고 html/scss 이용해서 배치/스타일링
- game.js 코드들 함수로 구조화
- socket 최초 설정 코드를 util 폴더로 이전
- 채팅 메시지 레이아웃/스타일링
- 기존 코드들의 import 경로들도 대응
- 랜덤닉네임, 랜덤컬러 생성하는 유틸함수 (임시로 대충만듦)
- rooms에서 게임 룸 아이디 찾는 유틸함수 구현
- 대기실에서 join 하는 이벤트 핸들러 추가
- 채팅 전송 시 해당 룸에 있는 소켓에게 메세지 전달하는 핸들러 추가
- 게임 클래스의 adduser 함수 추가
- Game state를 파일로 분리
- routes 수정
- dotenv 템플릿 수정
- dotenv 템플릿 수정하고 하드코딩된 URL 수정
[Feat/chat#45] 서버 플레이어 참여 및 채팅 관련 소켓핸들링, dotenv 템플릿 수정
우측 채팅창 레이아웃/스타일링, 이벤트 대응, FE webpack alias
- 유저 정보를 관리하기 위한 User 클래스 추가
- 유저정보를 찾거나 roomID를 찾을 수 있는 함수들 구현
- 클래스 수정한 대로 소켓 핸들러 코드 수정
- 추후 리팩토링 또 필요함
- 빈 문자열에 이벤트가 발생하던 문제
- 매우 길고 띄어쓰기가 없는 문자열에 채팅창 레이아웃 터지던 문제
- 스크롤이 아래로 향하지 않던 문제
- 콘솔 로그 삭제..^^
[Feat/chat#45] 유저 클래스 추가 및 채팅 핸들러에서 닉네임 보내주게 수정
@sbyeol3 sbyeol3 merged commit 97750dd into master Nov 26, 2020
Copy link

@crongro crongro left a comment

Choose a reason for hiding this comment

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

전체적으로 잘 구현했네요.
여러가지 신경쓰는 모습이 코드에서 나타납니다.
리뷰 참고하세요~

@@ -8,6 +9,11 @@ const createApplication = () => {

app.use(logger('dev'));
Copy link

Choose a reason for hiding this comment

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

참고로, 대부분의 서버에서는 로그를 수집하는 활동을 지속적으로 해서 문제가 언제 생기는지 모니터링하곤해요.

@@ -8,6 +9,11 @@ const createApplication = () => {

app.use(logger('dev'));
app.use(express.json());
app.use(
cors({
origin: process.env.FRONTEND_ORIGIN,
Copy link

Choose a reason for hiding this comment

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

env 설정으로 분리한거 좋네요.

@@ -0,0 +1,36 @@
import User from './User';

export default class Game {
Copy link

Choose a reason for hiding this comment

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

역할이 명확한 클래스네요. 좋아요.

}

findUserInfoAll(socketID) {
return this.users.get(socketID).getUserInfo() || null;
Copy link

Choose a reason for hiding this comment

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

위에서는 유저가 없다면 false, 여기서는 null 인데
의미적으로 어떤게 더 나을까요?
통일도 해야겠고요.

}

addCard(cardID) {
this.cards = [...this.cards, cardID];
Copy link

Choose a reason for hiding this comment

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

제 생각에도 OOP에서의 상태변경도, immutable 방식이 더 좋은 방법이라고 생각.

}

.chat-send-button {
padding: 0.5em !important;
Copy link

Choose a reason for hiding this comment

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

특별한 경우가 아니면 important 는 잘 안써요.
원인을 한번 찾아보세요~

margin: 0.5rem 2rem 0.5rem 1rem;

.chat-message {
background-color: $white-color;
Copy link

Choose a reason for hiding this comment

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

css variables 사용은 좋죠.

const config = { method: 'GET', uri: `/rooms/${roomCode}` };
const { success } = await requestHandler(config);
if (success) redirectToGameRoom(roomCode);
else {
Copy link

Choose a reason for hiding this comment

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

에러처리를 하려면 try/catch 로 비동기 로직을 감싸거나, Promise.catch 를 사용해서 해보시죠.
그러면 더 코드가 명시적으로 에러를 어떻게 만드는지 구분해서 표현하는 것이라 좋기도하고요.

Copy link
Collaborator

Choose a reason for hiding this comment

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

#74

ActionWrapper.toggleClass('waiting-action-wrapper');
ActionWrapper.attachToRoot();

const ButtonReturnToLobby = new ButtonObject();
Copy link

Choose a reason for hiding this comment

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

초기화할 객체가 한 두개가 아니네요.
메서드까지 많아서 좀더 복잡해보이는거 같기도하고요.

이렇게 객체로 옵션을 지정해서 추가하는 방법도 있고요.
취향적인 거라 참고~

new ButtonObject().attach({
   content : "로비로 돌아가기", 
   className : "button-cancel", 
   depObject:ActionWrapper,  
    handler:redirecToLobby
})

Copy link
Collaborator

Choose a reason for hiding this comment

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

다음에 constructor로 묶어줄 생각을 하고있습니다 ~~

const { method, uri } = config;
const url = `${process.env.API_BASE_URL}${uri}`;
if (method.toLowerCase() === 'get') return getHandler({ ...config, url });
if (method.toLowerCase() === 'post') return postHandler({ ...config, url });
Copy link

Choose a reason for hiding this comment

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

method.toLowerCase() 를 변수에 저장해서 한번만 수행하는 걸로

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.

None yet

5 participants