-
Notifications
You must be signed in to change notification settings - Fork 0
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
Multiplayer #38
Multiplayer #38
Conversation
|
|
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.
Very nice 👍 🥇
Ich glaube, ich habe mal wieder mehr in deinem PR gelernt als ich zurückgeben kann.
} | ||
|
||
public Lobby joinLobby(String id, @RequestBody Player player) { | ||
Lobby lobby = lobbyRepo.findById(id).orElseThrow(RuntimeException::new); |
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.
Hier könntest du die getLobbyById Methode des Service nutzen.
public Lobby setWinner(String id, ObjectNode payload) throws JsonProcessingException { | ||
Lobby lobby = lobbyRepo.findById(id).orElseThrow(RuntimeException::new); | ||
|
||
Player player = new ObjectMapper().treeToValue(payload.get("player"), Player.class); |
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.
Wieso so kompliziert?
Kann sich aber vielleicht später noch für mich kläre ;)
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.
Aye... I see. Du hast wohl den Spieler und die Zeit im Body.
} | ||
} | ||
|
||
return lobbyRepo.save(lobby.withWinner(player).withTimeToBeat(time)); |
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.
Ich glaube ich fände es leserlicher, wenn du erst den Fall winner == null
behandelst. Weniger Einrückungen und weniger Negierung.
import org.springframework.web.bind.annotation.*; | ||
|
||
@RestController | ||
@RequestMapping("/api/lobby") |
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.
Vielleicht "lobbies" statt "lobby". Deine anderen APIs sind auch im Plural.
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.
Bin auch pro Vereinheitlichung. Vielleicht benenne ich die anderen dann eher so um, dass sie auch im Singular stehen.
Without tests for now