feature/MAPG-235-basic-challenge-mode #48

Merged
balazs merged 43 commits from feature/MAPG-235-basic-challenge-mode into develop 2021-05-28 20:41:09 +02:00
4 changed files with 22 additions and 8 deletions
Showing only changes of commit 77ff175794 - Show all commits

View File

@ -84,7 +84,7 @@ div.mapItem>div.buttonContainer {
font-weight: 500; font-weight: 500;
} }
#restrictions input[type=checkbox] { #restrictions input {
height: auto; height: auto;
margin: 0.5em; margin: 0.5em;
} }
@ -94,6 +94,10 @@ div.mapItem>div.buttonContainer {
margin-left: 2em; margin-left: 2em;
} }
#timeLimitType {
margin-left: 2em;
}
@media screen and (min-width: 1504px) { @media screen and (min-width: 1504px) {
#mapContainer { #mapContainer {
grid-template-columns: auto auto auto auto; grid-template-columns: auto auto auto auto;

View File

@ -112,16 +112,19 @@ class GameController
$challenge->setToken($challengeToken); $challenge->setToken($challengeToken);
$challenge->setCreatedDate(new DateTime()); $challenge->setCreatedDate(new DateTime());
if($this->request->post('timerEnabled') !== null && $this->request->post('timeLimit') !== null) { if ($this->request->post('timerEnabled') !== null && $this->request->post('timeLimit') !== null) {
balazs marked this conversation as resolved Outdated
Outdated
Review

Maybe a do..while would be better here because the token calculation should not be repeated.

        do {
            // if a challenge with the same token already exists
            $challengeToken = rand();
        } while ($this->challengeRepository->getByToken($challengeToken));
Maybe a do..while would be better here because the token calculation should not be repeated. ```php do { // if a challenge with the same token already exists $challengeToken = rand(); } while ($this->challengeRepository->getByToken($challengeToken)); ```

My eye is twitching when variables are used outside of the scope of {}, but it's different in PHP, so it can be refactored.

My eye is twitching when variables are used outside of the scope of {}, but it's different in PHP, so it can be refactored.
$challenge->setTimeLimit($this->request->post('timeLimit')); $challenge->setTimeLimit($this->request->post('timeLimit'));
} }
if($this->request->post('noMove') !== null) { if ($this->request->post('timeLimitType') !== null) {
Outdated
Review

I think rand() should be called with explicit arguments, otherwise a number is returned between 0 and getrandmax() and getrandmax() is platform-dependent.

On the other hand maybe the token could be generated as the room ID for multiplayer. Then a fixed length string would be generated - I used bin2hex(random_bytes(3)) for room ID. I guess it was intentional to use integer indexes in the DB, it could be be more efficient but I already used string indexes for other purpose.

I think rand() should be called with explicit arguments, otherwise a number is returned between 0 and getrandmax() and getrandmax() is platform-dependent. On the other hand maybe the token could be generated as the room ID for multiplayer. Then a fixed length string would be generated - I used bin2hex(random_bytes(3)) for room ID. I guess it was intentional to use integer indexes in the DB, it could be be more efficient but I already used string indexes for other purpose.

Good point. However I don't think it would cause any problems. I thought it would be better to use 4 byte length integers instead of 3 bytes for more possible ids, because challenges are normally not getting deleted.

Good point. However I don't think it would cause any problems. I thought it would be better to use 4 byte length integers instead of 3 bytes for more possible ids, because challenges are normally not getting deleted.

I've read your comment again, and I see that bin2hex returns a string. Yes I thought it would perform better just to use an integer instead.
I can also see now, that on Windows the getrandmax() is only 32767, which is far too small. I am going to replace it with mt_rand() if that's fine for you.

I've read your comment again, and I see that bin2hex returns a string. Yes I thought it would perform better just to use an integer instead. I can also see now, that on Windows the getrandmax() is only 32767, which is far too small. I am going to replace it with mt_rand() if that's fine for you.
$challenge->setTimeLimitType($this->request->post('timeLimitType'));
}
if ($this->request->post('noMove') !== null) {
$challenge->setNoMove(true); $challenge->setNoMove(true);
} }
if($this->request->post('noPan') !== null) { if ($this->request->post('noPan') !== null) {
$challenge->setNoPan(true); $challenge->setNoPan(true);
} }
if($this->request->post('noZoom') !== null) { if ($this->request->post('noZoom') !== null) {
$challenge->setNoZoom(true); $challenge->setNoZoom(true);
} }

View File

@ -47,17 +47,17 @@ class Challenge extends Model
public function setNoMove(bool $noMove): void public function setNoMove(bool $noMove): void
{ {
$this->$noMove = $noMove; $this->noMove = $noMove;
} }
public function setNoPan(bool $noPan): void public function setNoPan(bool $noPan): void
{ {
$this->$noPan = $noPan; $this->noPan = $noPan;
} }
public function setNoZoom(bool $noZoom): void public function setNoZoom(bool $noZoom): void
{ {
$this->$noZoom = $noZoom; $this->noZoom = $noZoom;
} }
public function setCreatedDate(DateTime $created): void public function setCreatedDate(DateTime $created): void

View File

@ -53,6 +53,13 @@ TODO: condition!
<div> <div>
<input type="range" id="timeLimit" name="timeLimit" min="10" max="600" value="120" /> <input type="range" id="timeLimit" name="timeLimit" min="10" max="600" value="120" />
</div> </div>
<div id="timeLimitType">
Time limit
<input type="radio" id="timeLimitTypeGame" name="timeLimitType" value="game" checked />
<label for="timeLimitTypeGame">for the whole game</label>
<input type="radio" id="timeLimitTypeRound" name="timeLimitType" value="round" />
<label for="timeLimitTypeRound">per round</label>
</div>
</div> </div>
<div> <div>
<input type="checkbox" id="noMove" name="noMove" value="noMove" /> <input type="checkbox" id="noMove" name="noMove" value="noMove" />