-
Notifications
You must be signed in to change notification settings - Fork 15
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
Racial images update #1
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shows editing of entire file, error... |
Page-
pushed a commit
that referenced
this pull request
Jul 26, 2012
… make picking members more secure.
hemberger
pushed a commit
that referenced
this pull request
Jun 1, 2019
This commit consists of patches automatically generated for this project on https://scrutinizer-ci.com
hemberger
added a commit
that referenced
this pull request
Apr 25, 2021
Fixes the phpstan warning: > Method DummyPlayer::getShip() overrides method AbstractSmrPlayer::getShip() but misses parameter #1 $forceUpdate.
hemberger
added a commit
that referenced
this pull request
Jun 14, 2021
We need to be able to get the session var before we process any pages, but it is during page processing where we set the session var. Thus, we need to initialize the session var to an empty Page (as a kind of bootstrapping strategy). This fixes the following error: > Error Message: ErrorException: Undefined array key "" in /smr/src/lib/Smr/Session.php:259 > Stack trace: > #0 /smr/src/lib/Smr/Session.php(259): exception_error_handler(2, 'Undefined array...', '/smr/src/lib/Sm...', 259) > #1 /smr/src/lib/Default/AbstractSmrPlayer.class.php(2169): Smr\Session->getCurrentVar() > #2 /smr/src/tools/npc/npc.php(140): AbstractSmrPlayer->removeUnderAttack() > #3 /smr/src/tools/npc/npc.php(91): NPCStuff() > #4 {main}
hemberger
added a commit
that referenced
this pull request
Jun 14, 2021
The shop_hardware_processing.php script forwards the current LocationID in the session var to the next page. This was not being set as part of the preparation in npc.php (because the LocationID is not needed by anything the NPC does, i.e. no subsequent display page). However, it is no significant burden to provide this, so we pass along the sector ID to the `doUNO` function. Fixes the following error: > Error Message: Exception: Could not find "LocationID" in var! in /smr/src/lib/Default/Page.class.php:166 > Stack trace: > #0 /smr/src/engine/Default/shop_hardware_processing.php(64): Page->addVar('LocationID') > #1 /smr/src/lib/Default/Page.class.php(231): require('/smr/src/engine...') > #2 /smr/src/lib/Default/smr.inc.php(372): Page->process() > #3 /smr/src/tools/npc/npc.php(352): do_voodoo() > #4 /smr/src/tools/npc/npc.php(176): processContainer(Object(Page)) > #5 /smr/src/tools/npc/npc.php(91): NPCStuff() > #6 {main}
hemberger
added a commit
that referenced
this pull request
Aug 20, 2021
In the `displayGrouped` and `displayMessage` functions, we now return the constructed message (an array) instead of passing in the message box by reference and appending to it inside the functions. This fixes an issue with scout messages where the "GroupedMessages" element did not yet exist, but we were passing it by reference to the `displayMessage` function as if it were an array. This caused the following error: > TypeError: displayMessage(): Argument #1 ($messageBox) must be of type array, null given
hemberger
added a commit
that referenced
this pull request
Aug 21, 2021
* Return `$player` instead of returning it by reference. This change is needed because the value is `null` when it is passed in, which breaks the type requirement `AbstractSmrPlayer`. Fixes the error: > Unncaught TypeError: check_for_registration(): Argument #1 > ($player) must be of type AbstractSmrPlayer, null given * If the registration check fails, we return `false` instead of `true` so that we can use the union return type `AbstractSmrPlayer|false`. * Use a `callable` for the `$callback` argument instead of a string representation that must be run with `eval`.
hemberger
added a commit
that referenced
this pull request
Apr 11, 2022
Setting NPC_SCRIPT as a PHP constant made it difficult to test classes that depended on it. This is because PHPUnit has no mechanism to change the value of a PHP constant, apart from `@runInSeparateProcess`, and then setting the constant to a different value in that process. Unfortunately, `@runInSeparateProcess` interacted poorly with PHP-DI in a way that I don't completely understand. When running tests using this decorator, the following error would be triggered: PHPUnit\Framework\Exception: PHP Fatal error: Uncaught Error: Using $this when not in object context in /smr/vendor/php-di/php-di/src/Compiler/Template.php:4 Stack trace: #0 Standard input code(1154): require_once() #1 {main} thrown in /smr/vendor/php-di/php-di/src/Compiler/Template.php on line 4 Fatal error: Uncaught Error: Using $this when not in object context in /smr/vendor/php-di/php-di/src/Compiler/Template.php:4 Stack trace: #0 Standard input code(1154): require_once() #1 {main} thrown in /smr/vendor/php-di/php-di/src/Compiler/Template.php on line 4 This error always occurred when running the associated test files directly, but in an upcoming class `SectorLock`, it also occurred when running the entire test suite, which is obviously a blocking issue. To fix this, we convert the PHP constant `NPC_SCRIPT` into a PHP-DI variable with the same name. This allows us to change its value more easily, and particularly at runtime instead of compile time. In some sense this is an abuse of the DI container, since we're not actually using it for DI -- rather just for global variable storage. Oh well.
hemberger
added a commit
that referenced
this pull request
Jul 5, 2022
We would like to be able to use an enum to narrow the type of sector link directions, but cannot because they are used as array keys. Restricting their possible values via a special PHPStan docstring type turns out to be unhelpful, because the docstring would need to propagate to many more function interfaces, and yet we still wouldn't actually be able to enforce this restriction anywhere (since we would ultimately need to assume that some input variable was of this type). Therefore, we fall back to simply throwing an Exception if an invalid value is used. Fixes PHPStan warnings of the type: > Parameter #1 $dir of method SmrSector::setLinkSector() expects 'Down'|'Left'|'Right'|'Up', string given.
hemberger
added a commit
that referenced
this pull request
Jul 5, 2022
> Parameter #1 $callback of function spl_autoload_register expects > (callable(string): void)|null, Closure(string): bool given. Two changes were needed to match the parameter type expected by PHPStan: 1. Promote the 'get_class_loc' string to first-class callable syntax (see #1183). 2. Remove the boolean return value from `get_class_loc`. This was a "speculative" feature added in 3f6854f, but it doesn't conform to the expected interface nor is it needed for the autoloader to throw an Error if the class is not found.
hemberger
added a commit
that referenced
this pull request
Jul 5, 2022
Fixes PHPStan warning (and subsequent warnings): > Parameter #1 $forces of method SmrCombatDrones::shootPlayerAsPort() expects SmrPort, $this(AbstractSmrPort) given. This was the result of passing `$this` from within AbstractSmrPort.
hemberger
added a commit
that referenced
this pull request
Jul 5, 2022
These were added to prevent PHPStan unmatched type warnings, which was helpful for Level 4, but at Level 5 these type requirements propagate to the callers, which cannot always easily enforce these requirements. Therefore, we switch to throwing in the default match arm instead. Fixes the following PHPStan warnings: > Parameter #1 $pieceID of static method Smr\Chess\ChessPiece::getLetterForPiece() expects 1|2|3|4|5|6, int given. > Parameter #1 $letter of static method Smr\Chess\ChessPiece::getPieceForLetter() expects 'B'|'b'|'K'|'k'|'N'|'n'|'P'|'p'|'Q'|'q'|'R'|'r', string given.
hemberger
added a commit
that referenced
this pull request
Aug 8, 2022
Since we are setting these var values in PHP code (rather than getting them from user input), we don't need to validate their values. Fixes PHPStan warnings: > Parameter #1 $orderID of method AbstractSmrShip::moveWeaponUp() expects int, float|int|string given. > Parameter #1 $orderID of method AbstractSmrShip::moveWeaponDown() expects int, float|int|string given.
hemberger
added a commit
that referenced
this pull request
Aug 8, 2022
Fixes PHPStan warning: > Parameter #1 $num of function number_format expects float, > float|int|string given. By properly defining the Deposit/Withdrawal display value in the engine file, we can unconditionally display its value in the template.
hemberger
added a commit
that referenced
this pull request
Aug 8, 2022
Fixes PHPStan warning: > Parameter #1 $str of method Smr\Template::convertHtmlToAjaxXml() > expects string, string|false given. If output buffering is not turned on, then the template will not be able to display anything.
hemberger
added a commit
that referenced
this pull request
Aug 8, 2022
We have a constraint on the return type that it must be the same as the type of the input array key. We use the `@template` syntax to achieve this. Fixes PHPStan warnings of the type: > Parameter #1 $buildingTypeID of method SmrPlanet::destroyBuilding() > expects int, int|string given.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
The return value of `fgets` can be false if an error occurred. Fixes PHPStan warning: > Parameter #1 $string of function trim expects string, string|false given.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
The return value of `gzcompress` can be false if an error occurred. Fixes PHPStan warning: > Parameter #1 $binary of method Smr\Database::escapeBinary() expects string, string|false given.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
The PHP functions `file_get_contents` and `parse_ini_string` can return false if they fail. If the former does, it's an internal error, so we throw an Exception. If the latter does, it indicates the SMR file may be malformed, so we throw a UserError. Fixes PHPStan warnings: > Parameter #1 $haystack of function strstr expects string, string|false given. > Cannot access offset 'Metadata' on array|false. > Cannot access offset 'Galaxies' on array|false. > Argument of an invalid type array|false supplied for foreach, only iterables are supported.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
Array shapes provide a more precise constraint for an array return, and so should be used whenever possible. Fixes PHPStan warning: > Parameter #1 $rank of static method Smr\HallOfFame::displayHOFRow() expects int, float|int given.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
PHP functions often return false if they fail. It's important to check for this return value, both so that we can throw a helpful Exception, and so that we don't use a false value improperly down the line. Fixes PHPStan warnings: > Method Smr\Template::addJavascriptForAjax() should return string but returns string|false. > Argument of an invalid type DOMNodeList<DOMNode>|false supplied for foreach, only iterables are supported. > Parameter #1 $value of function count expects array|Countable, DOMNodeList<DOMNode>|false given.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
Since all we want to do is split a string on a delimiter, and then trim the results, we can do that much more explicitly with the `explode` and `trim` methods. With `preg_split`, we would have needed to validate that the result did not return false (which also would have prevented us from using array destructuring as well). Fixes the following PHPStan warnings: > 158 Cannot use array destructuring on array<int, string>|false. > 187 Parameter #1 $array of method Smr\Database::escapeArray() expects array, array<int, string>|false given. > 195 Parameter #1 $array of method Smr\Database::escapeArray() expects array, array<int, string>|false given. Unrelatedly, fixed an issue with player names not being escaped properly for display.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
We cannot pass an int value of `$X` to the `str2int` method, because it only accepts strings. This regression was introduced in 7330d7a, and was causing an error when accepting missions (since that's the only place we currently pass an int `$X`). We may want to consider changing `str2int` to allow other types, e.g. int or float or even mixed, as the implementation works for any type (but then obviously we'd need to rename the function). Fixes the PHPStan warning: > Parameter #1 $val of function str2int expects string, int|string given.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
Use the FILTER_VALIDATE_IP flag of `filter_var` on the IP address to avoid warnings from calling `gethostbyaddr` on invalid IP addresses. We still fall back to "unknown" for the host if `gethostbyaddr` fails (though I'm not sure if that's possible for validated IPs). Fixes PHPStan warnings: > Cannot use array destructuring on array<int, string>|false. > Parameter #1 $string of method Smr\Database::escapeString() expects string|null, string|false given.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
While we don't expect the database field `player_has_notes.note` to be uncompressed, it is safer to make sure that it uncompresses properly. Fixes PHPStan warning: > Parameter #1 $message of function bbifyMessage expects string, string|false given.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
Use an explicit array shape instead of `mixed` for the return type of the SmrAccount::getIndividualScores method. Fixes PHPStan warning: > Parameter #1 $val of function IRound expects float, array|int given.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
We don't need to include the bounty that the killer gained in the array that we return in `killPlayerByPlayer`. This allows us to simplify the logic a bit by using local variables. Fixes PHPStan warning: > Parameter #1 $type of method AbstractSmrPlayer::increaseCurrentBountyAmount() expects Smr\BountyType, Smr\BountyType::HQ|Smr\BountyType::UG|string given.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
Narrow the typehint for "weapon data" arrays by using an explicit array shape, since each field can only ever be a single type. This fixes the following PHPStan warnings: > Parameter #1 $damage of method AbstractSmrPort::takeDamageToShields() expects int, bool|int given. > Parameter #1 $damage of method AbstractSmrShip::takeDamageToShields() expects int, bool|int given. > Parameter #1 $damage of method SmrForce::takeDamageToMines() expects int, bool|int given. > Parameter #1 $damage of method SmrPlanet::takeDamageToShields() expects int, bool|int given. Ideally, we would convert this array to a data class, but the combat templates are sufficiently fragile that I wanted to make the smallest changes possible to fix the warnings, which meant only changing the docstrings. To avoid repeating a complex array shape in many places, we use the PHPStan `typeAliases` configuration option, which allows us to specify the type once, and then use it in docstrings as if it were a real type.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
Fixes PHPStan warning: > Parameter #1 $length of function random_bytes expects int<1, max>, (float|int) given.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
The return value of `gzcompress` can be false if an error occurs. Fixes PHPStan warning: > Parameter #1 $binary of method Smr\Database::escapeBinary() expects string, string|false given.
hemberger
added a commit
that referenced
this pull request
Jan 8, 2023
We added validation of the result for the `write` method during the major Database refactor in 7054e3d, but the `read` method was not similarly modified. For the `read` method, we expect a `mysqli_result`. If it returns bool instead, then it failed (false) or was the wrong query type (true). Fixes PHPStan warning: > Parameter #1 $dbResult of class Smr\DatabaseResult constructor expects mysqli_result, bool|mysqli_result given.
hemberger
added a commit
that referenced
this pull request
Jan 23, 2023
Now that we are using strict types in templates, we cannot pass null to `htmlspecialchars`, as it only takes strings. This was causing an error on the page for any alliance with a null Discord server/channel. Fixes PHPStan warnings of the type: > Parameter #1 $string of function htmlspecialchars expects string, string|null given.
hemberger
added a commit
that referenced
this pull request
Feb 9, 2023
There are often large portions of a page that are displayed as a result of a condition being met (in this case, if we've selected a ship to compare against). This usually results in defining a whole collection of template variables together, or not. However, the template doesn't know about these dependencies. It only knows that we've checked one condition (`isset($CompareShip)` in this case), and then left a bunch of variables potentially undefined. Until we have a more comprehensive solution for the static analysis of templates, the best we can probably do is assert that _all_ variables in the collection are defined. This may be a bit redundant and even absurd in some cases, but it is certainly more explicit. Fixes PHPStan warnings of the type: > Parameter #1 $message of function bbifyMessage expects string, string|null given. > Argument of an invalid type array<string, string>|null supplied for foreach, only iterables are supported. > Parameter #1 $num of function number_format expects float, int|null given.
hemberger
added a commit
that referenced
this pull request
Feb 9, 2023
Use the existence of the `TotalFine` template variable to determine whether or not illegals were found. Fixes PHPStan warning: > Parameter #1 $num of function number_format expects float, int|null given.
hemberger
added a commit
that referenced
this pull request
Feb 9, 2023
PHPStan does not seem to understand that if a variable is an array key of an array that only takes strings as keys, then that variable must be a string. It is simple enough to additionally guard against it being null. Fixes PHPStan warning: > Parameter #1 $viewType of static method Smr\HallOfFame::getHofRank() expects string, string|null given.
hemberger
added a commit
that referenced
this pull request
Feb 9, 2023
When defining the "Other Info" for history games in the Summary class, construct the database query in such a way that we can safely return the result even if there are no players. We do this by adding IFNULL checks on the min/max operations in the query. As a result, we can remove the nullable from the template typehints. Fixes PHPStan warnings of the type: > Parameter #1 $num of function number_format expects float, int|null given. Note that we make the query identical to the same page for non-history games in the GameStats class. There, we remove an unnecessary ORDER BY from the query.
hemberger
added a commit
that referenced
this pull request
Feb 9, 2023
This replaces the second parameter of the `escapeString` method. It is more user-friendly to have a separate method for nullable input than to overload one method with two purposes. Furthermore, there's no way to tell PHPStan that the first argument of `escapeString` is only allowed to be null if the 2nd argument is true. Fixes PHPStan warning: > Parameter #1 $string of method mysqli::real_escape_string() expects string, string|null given.
hemberger
added a commit
that referenced
this pull request
Feb 9, 2023
Eschew the additional `valid` property in favor of dynamically checking that both `userID` and `email` are non-empty in `isValid()`. This allows us to provide a more direct relationship between the result of `isValid()` and the return types of `getUserID()` and `getEmail()`. Fixes PHPStan warning in Account: > Parameter #1 $string of method Smr\Database::escapeString() expects string, string|null given.
hemberger
added a commit
that referenced
this pull request
Feb 9, 2023
PHPStan cannot remember that `$doAjaxMiddle` is false if `$mid` is null, so it doesn't understand that `$mid` is guaranteed to be a DOMElement when used as such. That said, it's probably more straightforward for humans as well to scope that entire logic inside a `$mid !==` null check. Fixes PHPStan warnings: > Parameter #1 $node of closure expects DOMNode, DOMElement|null given. > Cannot call method getAttribute() on DOMElement|null.
hemberger
added a commit
that referenced
this pull request
Feb 9, 2023
Check that the record fetched from the mysqli_result is not null in the `record` method. This could occur if, for example, the method is called twice on the same DatabaseResult (since by definition it only has one record). Fixes PHPStan warning: > Parameter #1 $dbRecord of class Smr\DatabaseRecord constructor expects array<string, mixed>, array<string, string>|null given.
hemberger
added a commit
that referenced
this pull request
Feb 9, 2023
Since PHPStan cannot remember that `$MapPlayer` is null iff `$UniGen` is true, we need to rely more on checks against `$MapPlayer` so that PHPStan knows we're not trying to call Player methods on null. Fixes the following PHPStan warnings: > Cannot call method getAlliance() on Smr\Player|null. > Cannot call method isPartOfCourse() on Smr\Player|null. > Parameter #1 $player of method Smr\Sector::hasPlayerForces() expects Smr\AbstractPlayer, Smr\Player|null given. > Cannot call method isFlagship() on Smr\Player|null.
hemberger
added a commit
that referenced
this pull request
Feb 9, 2023
The methods `hasCachedPort` and `hasFriendlyForces` return false if a null argument is given. We need a way to express this relationship with PHPStan. Two annotations that did NOT work: 1. @return ($player is null ? false : bool) This seems to express the relationship most clearly, but unfortunately it did not actually narrow the type of `$player`. 2. @phpstan-assert-if-true !null $player This had the surprising behavior of also asserting the inverse -- that if the function returned false, then `$player` was not null, which we obviously do not want. Presumably because of how "assertions" are used traditionally (in a simple function with no complex logic), this is a desirable side effect (phpstan/phpstan#8351). As alluded to in the linked PHPStan issue, we can achieve the desired behavior by exploiting "equality assertions", for which the inverse is not also asserted. With this, we can narrow the type of `$player` to AbstractPlayer (and thus `!null`) if the function returns true. See https://psalm.dev/docs/annotating_code/assertion_syntax/#equality-assertions. Fixes from SectorMap.inc.php: > Parameter #1 $player of method Smr\Sector::getCachedPort() expects Smr\AbstractPlayer, Smr\Player|null given. And from SectorsFile.php: > Parameter #1 $player of method Smr\Sector::getFriendlyForces() expects Smr\AbstractPlayer, Smr\AbstractPlayer|null given.
hemberger
added a commit
that referenced
this pull request
Feb 9, 2023
This allows the page to display even if there are no valid games for which NPCs can be created, since we expect the game ID to be an int, and there is never a real game ID of 0. Alternatively, we could have skipped a lot of the logic if the game ID was null, but that is a more involved refactor (which creates other problems with conditionally defined template variables). Fixes PHPStan warnings: > Parameter #1 $selectedGameID of class Smr\Pages\Admin\NpcManageAddAccountProcessor constructor expects int, int|null given. > Parameter $selectedGameID of class Smr\Pages\Admin\NpcManageProcessor constructor expects int, int|null given. > Parameter #2 $gameID of static method Smr\AbstractPlayer::getPlayer() expects int, int|null given.
hemberger
added a commit
that referenced
this pull request
Feb 9, 2023
These constructor arguments come as a package - either they are all set or they are all null. It's tricky to demand this relationship when they are all separately nullable arguments. This suggests that they should be grouped (ideally in a data class, but we use an array for simplicity here, which is made much safer with PHPStan array shapes). Fixes PHPStan warning: > Parameter #1 $bytes of closure expects int, int|null given.
hemberger
added a commit
that referenced
this pull request
Feb 9, 2023
The constructor arguments are only nullable under certain conditions, due to relationships with other args that are cumbersome to enumerate. This suggests that the class needs to be refactored, but until then, using an empty array for some of the arguments fixes the following PHPStan warnings: > Parameter #1 $array of method Smr\Database::escapeArray() expects array, array<int>|null given. > Offset 0 does not exist on array<int>|null. > Offset 1 does not exist on array<int>|null. This is kind of a cop out, as it doesn't address the underlying dangers in the design, but it does placate PHPStan.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.