[RFR] Setup the multiplayer game #15

Luwangel · 2017-11-15T12:49:07Z

Create, join and play multiplayer games
See the list of open multiplayer games

Luwangel · 2017-11-15T13:37:12Z

[RFR] Expose an api to play the 15 puzzle game

jdemangeon · 2017-11-23T16:05:03Z

app/src/Controller/ApiController.php

+
+  public function new(Request $request) {
+    $mode = $request->get('mode');
+    $apiResponse = $this->api->new(self::DEFAULT_SIZE, $mode && $mode == 'multi');


Prefer strict equality check ===

$mode is not used elsewhere. You can directly do $request->get('mode') === 'multi'

jdemangeon · 2017-11-23T16:07:24Z

app/src/Controller/ApiController.php

+    $game = $context->getGame();
+    $player = $context->getPlayer();
+
+    if ($context->getIsPlayer() && $game->getWinner() == null) {


Same for ===

jdemangeon · 2017-11-23T16:08:19Z

app/src/Authentication/TokenAuthManager.php

+  public static function isPlayer(Request $request, Game $game, string $token) : bool {
+    $player1 = $game->getPlayer1();
+    $player2 = $game->getPlayer2();
+    return $player1->getToken() == $token || !$game->isFull() && $player2->getToken() == $token;


Same for === (checks type too)

jdemangeon · 2017-11-23T16:08:27Z

app/src/Authentication/TokenAuthManager.php

+  }
+
+  public static function getPlayer(Request $request, Game $game, string $token) {
+    if ($game->getIsMultiplayer() && $game->getPlayer2() && $game->getPlayer2()->getToken() == $token) {


jdemangeon · 2017-11-23T16:12:24Z

app/src/Controller/ApiController.php

+  public function new(Request $request) {
+    $mode = $request->get('mode');
+    $apiResponse = $this->api->new(self::DEFAULT_SIZE, $mode && $mode == 'multi');
+    $this->playerRepository->save($apiResponse['player']);


As already said at presentation, you use our repositories in a "active record" way.
You don't benefit from Doctrine ORM DataMapper that let you save all our "persisted" entities in the same time.

You do:
->persist(player)
->flush() => Save to database
->persist(game)
->flush() => Save to database

You must preferably do:
->persist(player)
->persist(game)
->flush() => Save to database in only one transaction

jdemangeon · 2017-11-23T16:13:56Z

app/src/Controller/ApiController.php

+  public function games() {
+    $gameIds = $this->gameRepository->findOpenMultiplayerGames();
+
+    $response = new JsonResponse();


You can put data in JsonResponse constructor => 1 line instead of 5

return new JsonResponse(['game_ids' => $gameIds]);

jdemangeon · 2017-11-23T16:14:14Z

app/src/Controller/ApiController.php

+      $this->gameRepository->save($apiResponse['game']);
+    }
+
+    $response = new JsonResponse();


You can put data in JsonResponse constructor

return new JsonResponse([......]);

jdemangeon · 2017-11-23T16:14:42Z

app/src/Controller/GameController.php

+  public function move(GameContext $context, int $tile) {
+    $game = $context->getGame();
+    $player = $context->getPlayer();
+    if ($context->getIsPlayer() && $game->getWinner() == null) {


jdemangeon · 2017-11-23T16:15:31Z

app/src/Controller/ApiController.php

+    $this->playerRepository->save($apiResponse['player']);
+    $this->gameRepository->save($apiResponse['game']);
+
+    $response = new JsonResponse();


You can put data in JsonResponse constructor

return new JsonResponse([......]);

jdemangeon · 2017-11-23T16:15:38Z

app/src/Controller/ApiController.php

+    $game->setPlayer2($newPlayer);
+    $this->gameRepository->save($game);
+
+    $response = new JsonResponse();


You can put data in JsonResponse constructor

return new JsonResponse([......]);

jdemangeon · 2017-12-05T10:22:05Z

app/src/Controller/ApiController.php

+    $currentPlayer = $context->getPlayer();
+    $player = $currentPlayer ?: $game->getPlayer1();
+
+    if ($game->getIsMultiplayer()) {


You can simplify this part of code, only otherPlayer changes

jdemangeon · 2017-12-05T10:23:59Z

app/src/Controller/GameController.php

+    $game = $context->getGame();
+    $currentPlayer = $context->getPlayer();
+    $player = $currentPlayer ?: $game->getPlayer1();
+    if ($game->getIsMultiplayer()) {


You can simplify that part of code, only otherPlayer changes

jdemangeon · 2017-12-05T10:32:52Z

app/src/Controller/MainController.php

@@ -4,15 +4,21 @@

 use Symfony\Bundle\FrameworkBundle\Controller\Controller;
 use Symfony\Component\HttpFoundation\Response;
+use App\Repository\GameRepository;

 class MainController extends Controller {


As you extends Symfony Controller (https://api.symfony.com/2.7/Symfony/Bundle/FrameworkBundle/Controller/Controller.html)

You can benefit from ControllerTrait wich is used by it (https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerTrait.php)

You can do the following.

return $this->renderView(...) instead of return new Response($this->twig->render...)

return $this->json(...) instead of return new JsonResponse($this->serializer->serialize....)

...

When you extends Controller, it's useless to inject services into the constructor because the Symfony Controller already has all the container (which is heavy).

So either you extends controller (and use Trait or $this->container->...), or you don't extends it an inject services in your constructor.

[RFR] Fix the api

Luwangel added 2 commits November 15, 2017 13:47

Setup multiplayer

a054b2d

Display the list of open multiplayer games in the main page

27d2464

Luwangel added 2 commits November 16, 2017 17:14

Write the tests

1fd93a4

Fix the tests

8478f5e

Luwangel changed the title ~~[WIP] Setup the multiplayer game~~ [RFR] Setup the multiplayer game Nov 16, 2017

Luwangel and others added 6 commits November 17, 2017 13:26

Define new routes for the api

081b6f4

Create a new controller for api routes

67ce6cc

Change route methods according to the review

2dc038f

Make player and game entities serializable

ec49226

Change the context resolvers to accept the authorization's header

82ae317

Merge pull request #16 from marmelab/expose_api

3755991

[RFR] Expose an api to play the 15 puzzle game

jdemangeon reviewed Nov 23, 2017

View reviewed changes

Luwangel added 5 commits December 4, 2017 09:26

Apply the reviews

c84b3ec

Fix the api controller

b43fee9

Fix the api

404e630

Add a cors module

51711db

Allow preflight requests

7d9859f

jdemangeon reviewed Dec 5, 2017

View reviewed changes

Luwangel and others added 4 commits December 5, 2017 11:49

Let the cors service manage the preflight requests

b299f5c

Apply the reviews

9559be5

Fix join a game

ea751d8

Merge pull request #17 from marmelab/fix_api

71f3380

[RFR] Fix the api

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFR] Setup the multiplayer game #15

[RFR] Setup the multiplayer game #15

Luwangel commented Nov 15, 2017 •

edited

Loading

Luwangel commented Nov 15, 2017

jdemangeon Nov 23, 2017

jdemangeon Nov 23, 2017

jdemangeon Nov 23, 2017

jdemangeon Nov 23, 2017

jdemangeon Nov 23, 2017

jdemangeon Nov 23, 2017

jdemangeon Nov 23, 2017

jdemangeon Nov 23, 2017

jdemangeon Nov 23, 2017

jdemangeon Nov 23, 2017

jdemangeon Nov 23, 2017

jdemangeon Dec 5, 2017

jdemangeon Dec 5, 2017

jdemangeon Dec 5, 2017

[RFR] Setup the multiplayer game #15

Are you sure you want to change the base?

[RFR] Setup the multiplayer game #15

Conversation

Luwangel commented Nov 15, 2017 • edited Loading

Luwangel commented Nov 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Luwangel commented Nov 15, 2017 •

edited

Loading