From 6e1fe8ccd8c17d1678e720e92386f762e319ef57 Mon Sep 17 00:00:00 2001 From: Dan Hemberger Date: Sat, 9 Sep 2023 15:40:03 -0700 Subject: [PATCH] SmrAlliance: disallow alliance ID of 0 Previously, we would allow creating an `SmrAlliance` instance when the input `$allianceID` was 0. This was problematic because most of the class attributes would be unset, and most methods would be unusable. Now, we will raise an AllianceNotFoundError if the ID is 0, and we remove the `isNone` method. This will ensure that all instances of the class are valid and can be used without restriction. We replace the existing `isNone` calls with checks if the alliance ID is 0. --- src/lib/Smr/Alliance.php | 61 ++++++++----------- .../Player/Rankings/AllianceVsAlliance.php | 33 ++++++---- test/SmrTest/lib/AllianceIntegrationTest.php | 10 --- 3 files changed, 47 insertions(+), 57 deletions(-) diff --git a/src/lib/Smr/Alliance.php b/src/lib/Smr/Alliance.php index 62b4caab3..eba9aebb1 100644 --- a/src/lib/Smr/Alliance.php +++ b/src/lib/Smr/Alliance.php @@ -94,37 +94,35 @@ protected function __construct( protected readonly int $gameID, DatabaseRecord $dbRecord = null, ) { - if ($allianceID !== 0) { - $db = Database::getInstance(); - $this->SQLID = [ - 'alliance_id' => $db->escapeNumber($allianceID), - 'game_id' => $db->escapeNumber($gameID), - ]; + $db = Database::getInstance(); + $this->SQLID = [ + 'alliance_id' => $db->escapeNumber($allianceID), + 'game_id' => $db->escapeNumber($gameID), + ]; - if ($dbRecord === null) { - $dbResult = $db->read('SELECT * FROM alliance WHERE ' . self::SQL, $this->SQLID); - if ($dbResult->hasRecord()) { - $dbRecord = $dbResult->record(); - } - } - if ($dbRecord === null) { - throw new AllianceNotFound('Invalid allianceID: ' . $allianceID . ' OR gameID: ' . $gameID); + if ($dbRecord === null) { + $dbResult = $db->read('SELECT * FROM alliance WHERE ' . self::SQL, $this->SQLID); + if ($dbResult->hasRecord()) { + $dbRecord = $dbResult->record(); } - - $this->allianceName = $dbRecord->getString('alliance_name'); - $this->password = $dbRecord->getString('alliance_password'); - $this->recruiting = $dbRecord->getBoolean('recruiting'); - $this->description = $dbRecord->getNullableString('alliance_description'); - $this->leaderID = $dbRecord->getInt('leader_id'); - $this->bank = $dbRecord->getInt('alliance_account'); - $this->kills = $dbRecord->getInt('alliance_kills'); - $this->deaths = $dbRecord->getInt('alliance_deaths'); - $this->motd = $dbRecord->getString('mod'); - $this->imgSrc = $dbRecord->getString('img_src'); - $this->discordServer = $dbRecord->getNullableString('discord_server'); - $this->discordChannel = $dbRecord->getNullableString('discord_channel'); - $this->flagshipID = $dbRecord->getInt('flagship_id'); } + if ($dbRecord === null) { + throw new AllianceNotFound('Invalid allianceID: ' . $allianceID . ' OR gameID: ' . $gameID); + } + + $this->allianceName = $dbRecord->getString('alliance_name'); + $this->password = $dbRecord->getString('alliance_password'); + $this->recruiting = $dbRecord->getBoolean('recruiting'); + $this->description = $dbRecord->getNullableString('alliance_description'); + $this->leaderID = $dbRecord->getInt('leader_id'); + $this->bank = $dbRecord->getInt('alliance_account'); + $this->kills = $dbRecord->getInt('alliance_kills'); + $this->deaths = $dbRecord->getInt('alliance_deaths'); + $this->motd = $dbRecord->getString('mod'); + $this->imgSrc = $dbRecord->getString('img_src'); + $this->discordServer = $dbRecord->getNullableString('discord_server'); + $this->discordChannel = $dbRecord->getNullableString('discord_channel'); + $this->flagshipID = $dbRecord->getInt('flagship_id'); } /** @@ -169,13 +167,6 @@ public static function createAlliance(int $gameID, string $name, bool $allowNHA return self::getAlliance($allianceID, $gameID); } - /** - * Returns true if the alliance ID is associated with allianceless players. - */ - public function isNone(): bool { - return $this->allianceID === 0; - } - /** * Returns true if the alliance is the Newbie Help Alliance. */ diff --git a/src/pages/Player/Rankings/AllianceVsAlliance.php b/src/pages/Player/Rankings/AllianceVsAlliance.php index 560ef094b..24e509b26 100644 --- a/src/pages/Player/Rankings/AllianceVsAlliance.php +++ b/src/pages/Player/Rankings/AllianceVsAlliance.php @@ -59,19 +59,24 @@ public function build(AbstractPlayer $player, Template $template): void { $alliance_vs = []; foreach ($alliance_vs_ids as $curr_id) { - $curr_alliance = Alliance::getAlliance($curr_id, $player->getGameID()); $container = new self($curr_id, $this->versusAllianceIDs); $style = ''; - if (!$curr_alliance->isNone() && $curr_alliance->hasDisbanded()) { - $style = 'class="red"'; - } if ($player->getAllianceID() === $curr_id) { $style = 'class="bold"'; } + if ($curr_id === 0) { + $alliance_name = 'No Alliance'; + } else { + $curr_alliance = Alliance::getAlliance($curr_id, $player->getGameID()); + if ($curr_alliance->hasDisbanded()) { + $style = 'class="red"'; + } + $alliance_name = $curr_alliance->getAllianceDisplayName(); + } $alliance_vs[] = [ 'ID' => $curr_id, 'DetailsHREF' => $container->href(), - 'Name' => $curr_alliance->isNone() ? 'No Alliance' : $curr_alliance->getAllianceDisplayName(), + 'Name' => $alliance_name, 'Style' => $style, ]; } @@ -79,14 +84,14 @@ public function build(AbstractPlayer $player, Template $template): void { $alliance_vs_table = []; foreach ($alliance_vs_ids as $curr_id) { - $curr_alliance = Alliance::getAlliance($curr_id, $player->getGameID()); foreach ($alliance_vs_ids as $id) { - $row_alliance = Alliance::getAlliance($id, $player->getGameID()); - $showRed = (!$curr_alliance->isNone() && $curr_alliance->hasDisbanded()) || - (!$row_alliance->isNone() && $row_alliance->hasDisbanded()); + $showRed = ( + ($curr_id !== 0 && Alliance::getAlliance($curr_id, $player->getGameID())->hasDisbanded()) || + ($id !== 0 && Alliance::getAlliance($id, $player->getGameID())->hasDisbanded()) + ); $showBold = $curr_id === $player->getAllianceID() || $id === $player->getAllianceID(); $style = ''; - if ($curr_id === $id && !$row_alliance->isNone()) { + if ($curr_id === $id && $id !== 0) { $value = '--'; if ($showRed) { $style = 'class="red"'; @@ -120,8 +125,12 @@ public function build(AbstractPlayer $player, Template $template): void { $template->assign('AllianceVsTable', $alliance_vs_table); // Show details for a specific alliance - $main_alliance = Alliance::getAlliance($this->detailsAllianceID, $player->getGameID()); - $mainName = $main_alliance->isNone() ? 'No Alliance' : $main_alliance->getAllianceDisplayName(); + if ($this->detailsAllianceID === 0) { + $mainName = 'No alliance'; + } else { + $main_alliance = Alliance::getAlliance($this->detailsAllianceID, $player->getGameID()); + $mainName = $main_alliance->getAllianceDisplayName(); + } $template->assign('DetailsName', $mainName); $kills = []; diff --git a/test/SmrTest/lib/AllianceIntegrationTest.php b/test/SmrTest/lib/AllianceIntegrationTest.php index 9d2e3e8f1..6ad522ff1 100644 --- a/test/SmrTest/lib/AllianceIntegrationTest.php +++ b/test/SmrTest/lib/AllianceIntegrationTest.php @@ -102,14 +102,4 @@ public function test_isNHA(): void { self::assertTrue($alliance->isNHA()); } - public function test_isNone(): void { - // Create an alliance that is not "none" - $alliance = Alliance::createAlliance(1, 'Some alliance'); - self::assertFalse($alliance->isNone()); - - // Create an alliance that is "none" - $alliance = Alliance::getAlliance(0, 1); - self::assertTrue($alliance->isNone()); - } - }