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

new micro control #36

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

new micro control #36

wants to merge 5 commits into from

Conversation

jerrioh
Copy link
Collaborator

@jerrioh jerrioh commented Dec 31, 2018

마이크로컨트롤을 위한 신규 CommbatManager 생성
기존 마이크로 관련 소스 oldmicro 패키지로 이동

geniusojw and others added 5 commits December 20, 2018 23:39
- tactics, squad, squadgenerator, tacticsmanager 등 추가
- 이전 micro소스코드 oldmicro 패키지로 이동
@RoySRose
Copy link
Owner

'merge pull request' 라는 커밋이 있는거로 봐서. 리베이스가 아닌 머지가 섞여 있는듯.
또한. first commit과 second commit이 특별한 부분이 없다면. 합치는 것이 어떨지.

@RoySRose RoySRose added the Architecture Monster bot 의 구조 변경 label Jan 2, 2019
@RoySRose
Copy link
Owner

RoySRose commented Jan 2, 2019

@jerrioh, 구조 자체에 대해서는 심도 있게 확인하지 않았습니다. 그 외 보이는 부분 필요사항입니다.

@snogada, 마이크로 쪽에 관심이 있으시면 해당 PR 의 구조 부분을 같이 리뷰해 보시는것도 좋을 것 같습니다. 보완할 부분이라던가.. 기능 추가 부분에 대한 논의도 좋을것 같습니다.

  1. rebase 요청
    image

  2. old micro 삭제 요청. (참고용으로 사용하는 부분은 git history를 통해서 참조 가능)
    새로운 micro 에 따라 유닛별 구현체들의 예시도 추가 필요하지 않을지?

  3. 모든 page 에서 author 정보 삭제(과거에는 그리고 현재에도 author 를 제일 위에 작성하는것을 유지하고 있는 프로젝트도 있지만. git 의 history 기능이 발전되면서. tracking info 를 코드위에 입력할 필요가 없게 됨에 따라 요즘은 잘 안 쓰는 추세임)

참고) effective java 의 item64 refer to objects by their interfaces 를 참고 해서 Control클래스를 interface 화 시키고. defaultControl 이라는 abstract 클래스를 끼는 모양으로 가는것도 향후에 고려해볼만할것 같습니다. (사실... 이건 저도 책에 나오는 효과가 그대로 나올런지 궁금하기도 하고 ㅋㅋㅋ)

@jerrioh
Copy link
Collaborator Author

jerrioh commented Jan 2, 2019 via email

@RoySRose
Copy link
Owner

RoySRose commented Jan 2, 2019

@jerrioh
2번 관련해서는 해당 부분 소스를 보게 될 사람들이 더 있을 것 같은데.
불필요한 소스가 많아질 수록 다른 사람들이 소스를 이해하기 위해서 더 많은 시간이 필요할 것 같습니다.

현재도 cleanup 진행하면서 주석 처리되었거나. 불필요한 소스는 최대한 지우고 있는 상황이므로
다시 한번 고려를 부탁드립니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture Monster bot 의 구조 변경
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants