[Feat][#10] 플레이어 이동 구현 #14
Conversation
Stark-Industries0417
added
👀 s.needs-review
|
|
||
| @Bean | ||
| open fun objectMapper(): ObjectMapper { | ||
| return ObjectMapper().registerKotlinModule() |
There was a problem hiding this comment.
https://github.com/f-lab-edu/data-pipline/pull/15/files#diff-56793a65fcb619ba5d796fd1ad2dbfbd30ef330514899b7ec21764cb386f5c55L11-L12
여기 있는 라이브러리가 이 브랜치에 추가되어야할 것 같네요.
implementation("com.fasterxml.jackson.module:jackson-module-kotlin:2.15.2")
PR 요청시 프로그램 실행여부를 한번 확인해주시면 좋을것같아요 🙂
There was a problem hiding this comment.
넵 신경쓰겠습니다
| data class Position( | ||
| val x: Int, | ||
| val y: Int | ||
| ) |
There was a problem hiding this comment.
Position 은 내부 도메인에서도 사용하고있는 것 같은데요.
domain 패키지를 만들어 내부에서도 사용하면 좋겠네요. Pair(x, y) 대신 사용하면 좋을것같은데 어떠신가요?
There was a problem hiding this comment.
넵 좋습니다!
| @@ -0,0 +1,3 @@ | |||
| package game.server.dto | |||
|
|
|||
| interface Request | |||
There was a problem hiding this comment.
이 인터페이스는 무슨 역할을 의도하고 만드신 건지 궁금합니다. 🙂
There was a problem hiding this comment.
해당 인터페이스는 새로운 요청 및 핸들러를 쉽게 확장하기 위해 만들었습니다
| import org.springframework.stereotype.Component | ||
|
|
||
| @Component("move") | ||
| class PlayerMoveHandler( |
There was a problem hiding this comment.
Handler 라는 이름을 websocket 통신시 이를 다루는 컴포넌트에 대한 네이밍으로하셨다면,
여기는 구분해서 다른 이름을 만들어봤으면 좋겠네요.
혹은 여기를 Handler 로 유지하고, 상위 계층을 Router 등으로 표현하면 어떨까요.
계층을 구분하면 유지보수성이 좋아질 것 같다는 생각이 드네요!
There was a problem hiding this comment.
네 알겠습니다!
| objectMapper.writeValueAsString( | ||
| mapOf( | ||
| "type" to "move", | ||
| "success" to true, | ||
| "newPosition" to mapOf("x" to newX, "y" to newY) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Map → Json 대신, Object → Json 으로 바꾸어보면 어떨까요?
플레이어 움직임에 대한 속성들을 한 객체로 묶어 표현해보면 관리하기 용이할 것 같아요
| mapOf( | ||
| "type" to "move", | ||
| "success" to false, | ||
| "message" to "Move not allowed" | ||
| ) | ||
| ) |
There was a problem hiding this comment.
위 코멘트까지 적용하였습니다!
| "UP" -> Pair(x, y - speed) | ||
| "DOWN" -> Pair(x, y + speed) | ||
| "LEFT" -> Pair(x - speed, y) | ||
| "RIGHT" -> Pair(x + speed, y) | ||
| else -> Pair(x, y) |
There was a problem hiding this comment.
아까 Position 으로 대체하면좋겠다고 말씀드린 부분입니다. 🙂
There was a problem hiding this comment.
적용하였습니다
| private fun isMoveAllowed(x: Int, y: Int): Boolean { | ||
| val mapWidth = 800 | ||
| val mapHeight = 600 | ||
| return x in 0 until mapWidth && y in 0 until mapHeight | ||
| } |
There was a problem hiding this comment.
'사용자 움직임' 을 객체로 추상화시켰다면 PlayerMoveHandler 대신에 그 객체 안에서 이 '역할'을 담당할 수 있을 것 같아요.
There was a problem hiding this comment.
넵 해당 작업은 13번 이슈에서 진행하도록 하겠습니다
| when (type) { | ||
| "move" -> { | ||
| val request = objectMapper.convertValue(data, PlayerMoveRequest::class.java) | ||
| requestHandlerFactory.getHandler<PlayerMoveRequest>("move").handle(request) | ||
| } | ||
| else -> { | ||
| throw IllegalArgumentException("Unknown request type: $type") | ||
| } | ||
| } |
There was a problem hiding this comment.
RuquestHandlerFactory 가 이 역할을 해도 될것같네요.
There was a problem hiding this comment.
해당 작업 4번 브랜치에서 진행하였습니다
역/직렬화 작업을 GameRequestRouter 에서 맡는게 좋을 것 같아 개선하였습니다!
|
🔗관련 이슈
✨작업 내용
🛠️변경 사항
게임 데이터 생성을 목적으로 한 간단한 게임이라 큰 의미는 없지만,
게임 로직의 흐름을 공유드리기 위해 PR을 올립니다