Skip to content

Commit

Permalink
Partial fix to Elo recalc bug
Browse files Browse the repository at this point in the history
It works reliably 50% of the time
  • Loading branch information
allejo committed Jan 19, 2018
1 parent 7488052 commit ac67523
Show file tree
Hide file tree
Showing 5 changed files with 229 additions and 23 deletions.
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@
"jdorn/sql-formatter": "~1.2",
"phpunit/phpunit": "~5.7",
"sensio/distribution-bundle": "~3.0",
"sensiolabs/security-checker": "~4.1"
"sensiolabs/security-checker": "~4.1",
"fzaninotto/faker": "^1.7"
},

"scripts": {
Expand Down
52 changes: 51 additions & 1 deletion composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 12 additions & 21 deletions models/Player.php
Original file line number Diff line number Diff line change
Expand Up @@ -360,28 +360,11 @@ public function invalidateMatchFromCache(Match $match)
$seasonKey = $this->buildSeasonKeyFromTimestamp($match->getTimestamp());
$seasonElo = null;

// If we have an existing season history cached, save a reference to it for easy access. Don't create one if
// nothing is cached or else it'll cause for an empty cache to be created
if (isset($this->eloSeasonHistory[$seasonKey])) {
$seasonElo = &$this->eloSeasonHistory[$seasonKey];
}

// Unset the currently cached Elo for a player so next time Player::getElo() is called, it'll pull the latest
// available Elo
unset($this->eloSeason[$seasonKey]);
$eloChangelogIndex = array_search($match->getId(), array_keys($this->eloSeasonHistory[$seasonKey]));
$slicedChangeLog = array_slice($this->eloSeasonHistory[$seasonKey], 0, $eloChangelogIndex, true);

if ($seasonElo === null) {
return;
}

// This function is called when we recalculate, so assume that the match will be recent, therefore towards the
// end of the Elo history array. We splice the array to have all Elo data after this match to be removed.
foreach (array_reverse($seasonElo) as $key => $match) {
if ($match['match'] === $match) {
$seasonElo = array_splice($seasonElo, $key);
break;
}
}
$this->eloSeasonHistory[$seasonKey] = $slicedChangeLog;
$this->eloSeason[$seasonKey] = end($slicedChangeLog)['elo'];
}

/**
Expand Down Expand Up @@ -485,6 +468,14 @@ public function adjustElo($adjust, Match $match = null)
$this->eloSeason[$seasonKey] += $adjust;

if ($match !== null && $this->isValid()) {
$this->eloSeasonHistory[$seasonKey][$match->getId()] = [
'elo' => $this->eloSeason[$seasonKey],
'match' => $match->getId(),
'month' => $match->getTimestamp()->month,
'year' => $match->getTimestamp()->year,
'day' => null,
];

$this->db->execute('
INSERT INTO player_elo VALUES (?, ?, ?, ?, ?, ?)
', [ $this->getId(), $match->getId(), $seasonInfo['season'], $seasonInfo['year'], $elo, $this->eloSeason[$seasonKey] ]);
Expand Down
160 changes: 160 additions & 0 deletions tests/ModelTests/MatchEloTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
<?php

class MatchEloTest extends TestCase
{
/** @var \Faker\Generator */
private $faker;
private $createdModels = [];

protected function setUp()
{
$this->connectToDatabase();

$this->faker = Faker\Factory::create();
}

public function tearDown()
{
foreach ($this->createdModels as $model)
{
$this->wipe($model);
}

parent::tearDown();
}

// Yes, we need a test for our own helper function in our tests
public function testGenerateMatches()
{
$matchCount = 8;

$team = null;
$matches = [];

$this->generateMatches($matchCount, $team, $matches);

$this->assertArrayLengthEquals($matches, $matchCount);
$this->assertInstanceOf(Team::class, $team);
$this->assertInstanceOf(Match::class, $matches[0]);
}

public function testGenerateTeamWithPlayers()
{
$playerCount = 5;

$team = null;
$players = [];

$this->generateTeamWithPlayers($playerCount, $team, $players);

$this->assertArrayLengthEquals($players, $playerCount);
$this->assertInstanceOf(Team::class, $team);
$this->assertInstanceOf(Player::class, $players[0]);
}

// Onward to our real tests for Elo recalculations!
public function testEloRecalculation()
{
/** @var Team $team */
$team = null;
/** @var Match[] $matches */
$matches = [];

$this->generateMatches(10, $team, $matches);

$preRecalcTeamElo = $team->getElo();
$preRecalcLeaderElo = $team->getLeader()->getElo();

$matchToEdit = $matches[3];

$matchResult = $matchToEdit->getMatchDescription($team);
$scoreTeamA = $matchToEdit->getScore($matchToEdit->getTeamA());
$scoreTeamB = $matchToEdit->getScore($matchToEdit->getTeamB());

$matchToEdit->setTeamPoints($scoreTeamB, $scoreTeamA);

Match::recalculateMatchesSince($matchToEdit);

$newLeaderElo = Player::get($team->getLeader())->getElo();
$newTeamElo = Team::get($team->getId())->getElo();

if ($matchResult === 'win') {
$this->assertLessThan($preRecalcLeaderElo, $newLeaderElo);
$this->assertLessThan($preRecalcTeamElo, $newTeamElo);
} elseif ($matchResult === 'loss') {
$this->assertGreaterThan($preRecalcLeaderElo, $newLeaderElo);
$this->assertGreaterThan($preRecalcTeamElo, $newTeamElo);
} else {
$this->assertEquals($preRecalcLeaderElo, $newLeaderElo);
$this->assertEquals($preRecalcTeamElo, $newTeamElo);
}
}

/**
* Generate random matches with a single team always being in the match.
*
* @param int $numberOfMatches
* @param Team $team_a
* @param Match[] $matches
*
* @throws Exception
*/
private function generateMatches($numberOfMatches, &$team_a, array &$matches)
{
$participantCount = $this->faker->numberBetween(2, 10);

$team_a = $team_b = null;
$team_a_players = $team_b_players = [];

$this->generateTeamWithPlayers($participantCount, $team_a, $team_a_players);
$this->generateTeamWithPlayers($participantCount, $team_b, $team_b_players);

$team_a_roster = __::map($team_a_players, function ($n) { return $n->getId(); });
$team_b_roster = __::map($team_b_players, function ($n) { return $n->getId(); });

for ($i = 0; $i < $numberOfMatches; $i++) {
$matches[$i] = Match::enterMatch(
$team_a->getId(),
($this->faker->boolean) ? $team_b : null,
$this->faker->numberBetween(0, 6),
$this->faker->numberBetween(0, 6),
$this->faker->randomElement([15, 20, 30]),
null,
sprintf('-%d days', $numberOfMatches - $i),
$team_a_roster,
$team_b_roster,
null,
null,
null,
'official',
'red',
'purple'
);

array_unshift($this->createdModels, $matches[$i]);
}
}

/**
* Generate a real team with multiple players assigned to the team.
*
* @param int $playerCount
* @param Team $team
* @param Player[] $players
*
* @throws Exception
*/
private function generateTeamWithPlayers($playerCount, &$team, array &$players)
{
$players[0] = $this->getNewPlayer();

$team = Team::createTeam($this->faker->text($maxNbChars = 42), $players[0]->getId(), '', '');

array_unshift($this->createdModels, $team);

for ($i = 1; $i < $playerCount; $i++) {
$players[$i] = $this->getNewPlayer();
$team->addMember($players[$i]->getId());
}
}
}
4 changes: 4 additions & 0 deletions tests/bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,7 @@

$kernel = new AppKernel('test', true);
$kernel->boot();

// @todo Extract clearDatabase() function to separate class
// @body The test database should be cleared every time the bootstrap is run, therefore this functionality should be in a more generic context
FeatureContext::clearDatabase();

0 comments on commit ac67523

Please sign in to comment.