Remove GameQ and use "own" implementation for queries #94
Conversation
Boy132
changed the title
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReplaces a monolithic GameQ approach with a schema-driven QueryTypeService and multiple QueryTypeSchema implementations; GameQuery now delegates to registered schemas, query_type is a string, runQuery returns nullable standardized results, dependencies updated, and seeders/provider/UI/API adapted to the new model. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as PlayerCounterController
participant Model as GameQuery
participant Service as QueryTypeService
participant Schema
participant Server as RemoteGameServer
Client->>API: GET /servers/{id}/players
API->>Model: runQuery(allocation)
Model->>Service: get(schemaId)
Service-->>Model: Schema instance
Model->>Schema: process(ip, port)
Schema->>Server: perform protocol-specific query (SRV/HTTP/UDP/etc.)
Server-->>Schema: response (info/players) or error
Schema-->>Model: standardized array or null
Model-->>API: result or null
API-->>Client: 200 JSON or 406/404
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
player-counter/src/Filament/Server/Widgets/ServerPlayerWidget.php (1)
24-48: Add allocation guard ingetStats()for type safety.The
canRunQuery()method already handles null allocations safely (line 53-54 of GameQuery.php). However,runQuery()has a non-nullable Allocation type hint whilegetStats()passes$server->allocationwithout verifying it's not null. Add a guard before line 48 to match type expectations:protected function getStats(): array { /** `@var` Server $server */ $server = Filament::getTenant(); /** `@var` ?GameQuery $gameQuery */ $gameQuery = $server->egg->gameQuery; // `@phpstan-ignore` property.notFound if (!$gameQuery) { return []; } + if (!$server->allocation) { + return []; + } + $data = $gameQuery->runQuery($server->allocation) ?? [];The
canView()method (line 24) is safe as-is sincecanRunQuery()accepts nullable allocations.player-counter/src/Http/Controllers/Api/Client/Servers/PlayerCounterController.php (1)
23-28: Guard against null query results and empty player lists.
runQuery()can returnnullon query failures, and$data['players']is falsey for an empty list. Both lead to errors or 406s for valid empty states. Consider explicit null/missing checks and allow empty arrays.🔧 Proposed fix
$data = $this->runQuery($server); + if (!$data) { + abort(Response::HTTP_NOT_ACCEPTABLE, 'Server query failed'); + } return response()->json(array_except($data, 'players'));$data = $this->runQuery($server); - if (!$data['players']) { + if (!$data || !array_key_exists('players', $data) || $data['players'] === null) { abort(Response::HTTP_NOT_ACCEPTABLE, 'Server query has no player list'); } /** `@var` string[] $players */ $players = array_map(fn ($player) => $player['name'], $data['players']);Also applies to: 37-46
🤖 Fix all issues with AI agents
In @.github/workflows/lint.yml:
- Line 64: The PHPStan workflow's composer require line (the step that runs
composer require "stripe/stripe-php:^18.0" "kovah/laravel-socialite-oidc:^0.6"
"socialiteproviders/pocketid:^5.0") is missing the query packages used by the
player-counter plugin; update that composer require invocation to include
xpaw/php-minecraft-query:^5.0.0 and xpaw/php-source-query-class:^5.0.0 so
PHPStan installs those dependencies and can analyze types for the plugin.
In `@player-counter/database/Seeders/PlayerCounterSeeder.php`:
- Around line 81-95: The seeder currently swallows exceptions in the mapping
loop in PlayerCounterSeeder which hides failures in GameQuery::firstOrCreate and
EggGameQuery::firstOrCreate; catch the Exception as $e and either rethrow or log
it (e.g., via Laravel's Log::error or throw new RuntimeException) including
$e->getMessage() and relevant context (egg id/name and
$mapping['query_type']/query_port_offset) so failures are visible and unmapped
eggs can be diagnosed; update the catch block to use the chosen reporting
mechanism instead of an empty block.
In `@player-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.php`:
- Around line 32-37: The guard that treats an empty players list as failure
should be changed to use explicit null checks: in CitizenFXQueryTypeSchema (the
block calling $http->get('dynamic.json')->json() and
$http->get('players.json')->json()), replace the truthy check if (!$info ||
!$players) with checks that only return null when the responses are actually
null (e.g., $info === null || $players === null) so that empty arrays/empty
player lists are accepted while still handling missing responses.
In
`@player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php`:
- Around line 68-69: The current legacy query path treats an empty player list
as failure because it uses a truthiness check (if (!$info || !$players)); update
that condition to only treat explicit null/false as failure so empty arrays are
accepted. In the MinecraftJavaQueryTypeSchema legacy handling, replace the
truthy check around $info and $players with strict checks (e.g., $info === null
|| $info === false || $players === null || $players === false) so GetPlayers()
returning [] is allowed to continue rather than falling back to ping.
- Around line 39-44: The hostname assignment in the return array uses
is_string($data['description']) ? $data['description'] :
$data['description']['text'] and can raise a notice when $data['description'] is
an array without a 'text' key; change the extraction in the method that builds
the return array (the 'hostname' entry in MinecraftJavaQueryTypeSchema) to
safely handle all cases by checking if $data['description'] is a string, else if
it's an array and has a 'text' key, else fall back to a safe default (e.g.,
empty string or $data['description']['extra'] if appropriate) so no undefined
index notice is raised.
In `@player-counter/src/Models/GameQuery.php`:
- Around line 35-48: The runQuery method can fatal when
QueryTypeService::get($this->query_type) returns null; update
GameQuery::runQuery to check the returned value from QueryTypeService::get()
(which returns ?QueryTypeSchemaInterface) before calling ->process(...), and if
it's null return null (or abort) and optionally log/throw a clear error; locate
the app(QueryTypeService::class) call and the subsequent
->get($this->query_type)->process(...) invocation and add the null guard there.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.github/workflows/lint.ymlplayer-counter/README.mdplayer-counter/database/Seeders/PlayerCounterSeeder.phpplayer-counter/plugin.jsonplayer-counter/src/Enums/GameQueryType.phpplayer-counter/src/Extensions/Query/QueryTypeSchemaInterface.phpplayer-counter/src/Extensions/Query/QueryTypeService.phpplayer-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.phpplayer-counter/src/Extensions/Query/Schemas/GoldSourceQueryTypeSchema.phpplayer-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.phpplayer-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.phpplayer-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.phpplayer-counter/src/Filament/Admin/Resources/GameQueries/GameQueryResource.phpplayer-counter/src/Filament/Server/Pages/PlayersPage.phpplayer-counter/src/Filament/Server/Widgets/ServerPlayerWidget.phpplayer-counter/src/Http/Controllers/Api/Client/Servers/PlayerCounterController.phpplayer-counter/src/Models/GameQuery.phpplayer-counter/src/Providers/PlayerCounterPluginProvider.php
💤 Files with no reviewable changes (1)
- player-counter/src/Enums/GameQueryType.php
🧰 Additional context used
🧬 Code graph analysis (10)
player-counter/src/Extensions/Query/QueryTypeService.php (7)
player-counter/src/Providers/PlayerCounterPluginProvider.php (1)
register(22-41)player-counter/src/Extensions/Query/QueryTypeSchemaInterface.php (2)
getId(7-7)getName(9-9)player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php (2)
getId(11-14)getName(16-19)player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php (2)
getId(12-15)getName(17-20)player-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.php (2)
getId(11-14)getName(16-19)player-counter/src/Extensions/Query/Schemas/GoldSourceQueryTypeSchema.php (2)
getId(9-12)getName(14-17)player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (2)
getId(11-14)getName(16-19)
player-counter/database/Seeders/PlayerCounterSeeder.php (3)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (1)
run(28-52)player-counter/src/Models/GameQuery.php (1)
GameQuery(19-61)player-counter/src/Models/EggGameQuery.php (1)
EggGameQuery(11-14)
player-counter/src/Extensions/Query/QueryTypeSchemaInterface.php (5)
player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php (3)
getId(11-14)getName(16-19)process(22-47)player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php (3)
getId(12-15)getName(17-20)process(23-55)player-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.php (3)
getId(11-14)getName(16-19)process(22-51)player-counter/src/Extensions/Query/Schemas/GoldSourceQueryTypeSchema.php (3)
getId(9-12)getName(14-17)process(20-23)player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (3)
getId(11-14)getName(16-19)process(22-25)
player-counter/src/Filament/Admin/Resources/GameQueries/GameQueryResource.php (1)
player-counter/src/Extensions/Query/QueryTypeService.php (2)
QueryTypeService(5-29)getMappings(25-28)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (2)
player-counter/src/Extensions/Query/QueryTypeSchemaInterface.php (3)
getId(7-7)getName(9-9)process(12-12)player-counter/src/Extensions/Query/Schemas/GoldSourceQueryTypeSchema.php (3)
getId(9-12)getName(14-17)process(20-23)
player-counter/src/Extensions/Query/Schemas/GoldSourceQueryTypeSchema.php (2)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (5)
SourceQueryTypeSchema(9-53)getId(11-14)getName(16-19)process(22-25)run(28-52)player-counter/src/Extensions/Query/QueryTypeSchemaInterface.php (3)
getId(7-7)getName(9-9)process(12-12)
player-counter/src/Filament/Server/Pages/PlayersPage.php (2)
player-counter/src/Models/GameQuery.php (2)
GameQuery(19-61)canRunQuery(51-60)player-counter/src/Http/Controllers/Api/Client/Servers/PlayerCounterController.php (1)
players(37-49)
player-counter/src/Providers/PlayerCounterPluginProvider.php (5)
player-counter/src/Extensions/Query/QueryTypeService.php (2)
QueryTypeService(5-29)register(15-22)player-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.php (1)
CitizenFXQueryTypeSchema(9-73)player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php (1)
MinecraftBedrockQueryTypeSchema(9-48)player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php (1)
MinecraftJavaQueryTypeSchema(10-85)player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (1)
SourceQueryTypeSchema(9-53)
player-counter/src/Models/GameQuery.php (5)
player-counter/src/Extensions/Query/QueryTypeService.php (2)
QueryTypeService(5-29)get(10-13)player-counter/src/Extensions/Query/QueryTypeSchemaInterface.php (1)
process(12-12)player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php (1)
process(22-47)player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php (1)
process(23-55)player-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.php (1)
process(22-51)
player-counter/src/Http/Controllers/Api/Client/Servers/PlayerCounterController.php (1)
player-counter/src/Models/GameQuery.php (3)
runQuery(36-49)GameQuery(19-61)canRunQuery(51-60)
🪛 GitHub Check: PHPStan (8.2)
player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php
[failure] 65-65:
Call to method GetInfo() on an unknown class xPaw\MinecraftQuery.
[failure] 63-63:
Call to method Connect() on an unknown class xPaw\MinecraftQuery.
[failure] 60-60:
Instantiated class xPaw\MinecraftQuery not found.
[failure] 50-50:
Call to method Close() on an unknown class xPaw\MinecraftPing.
[failure] 33-33:
Call to method Query() on an unknown class xPaw\MinecraftPing.
[failure] 31-31:
Instantiated class xPaw\MinecraftPing not found.
player-counter/src/Extensions/Query/Schemas/GoldSourceQueryTypeSchema.php
[failure] 22-22:
Access to constant GOLDSOURCE on an unknown class xPaw\SourceQuery\SourceQuery.
player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php
[failure] 29-29:
Call to method GetInfo() on an unknown class xPaw\MinecraftQuery.
[failure] 27-27:
Call to method ConnectBedrock() on an unknown class xPaw\MinecraftQuery.
[failure] 24-24:
Instantiated class xPaw\MinecraftQuery not found.
🪛 GitHub Check: PHPStan (8.3)
player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php
[failure] 65-65:
Call to method GetInfo() on an unknown class xPaw\MinecraftQuery.
[failure] 63-63:
Call to method Connect() on an unknown class xPaw\MinecraftQuery.
[failure] 60-60:
Instantiated class xPaw\MinecraftQuery not found.
[failure] 50-50:
Call to method Close() on an unknown class xPaw\MinecraftPing.
[failure] 33-33:
Call to method Query() on an unknown class xPaw\MinecraftPing.
[failure] 31-31:
Instantiated class xPaw\MinecraftPing not found.
player-counter/src/Extensions/Query/Schemas/GoldSourceQueryTypeSchema.php
[failure] 22-22:
Access to constant GOLDSOURCE on an unknown class xPaw\SourceQuery\SourceQuery.
player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php
[failure] 29-29:
Call to method GetInfo() on an unknown class xPaw\MinecraftQuery.
[failure] 27-27:
Call to method ConnectBedrock() on an unknown class xPaw\MinecraftQuery.
[failure] 24-24:
Instantiated class xPaw\MinecraftQuery not found.
🪛 GitHub Check: PHPStan (8.4)
player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php
[failure] 65-65:
Call to method GetInfo() on an unknown class xPaw\MinecraftQuery.
[failure] 63-63:
Call to method Connect() on an unknown class xPaw\MinecraftQuery.
[failure] 60-60:
Instantiated class xPaw\MinecraftQuery not found.
[failure] 50-50:
Call to method Close() on an unknown class xPaw\MinecraftPing.
[failure] 33-33:
Call to method Query() on an unknown class xPaw\MinecraftPing.
[failure] 31-31:
Instantiated class xPaw\MinecraftPing not found.
player-counter/src/Extensions/Query/Schemas/GoldSourceQueryTypeSchema.php
[failure] 22-22:
Access to constant GOLDSOURCE on an unknown class xPaw\SourceQuery\SourceQuery.
player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php
[failure] 29-29:
Call to method GetInfo() on an unknown class xPaw\MinecraftQuery.
[failure] 27-27:
Call to method ConnectBedrock() on an unknown class xPaw\MinecraftQuery.
[failure] 24-24:
Instantiated class xPaw\MinecraftQuery not found.
🪛 GitHub Check: PHPStan (8.5)
player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php
[failure] 65-65:
Call to method GetInfo() on an unknown class xPaw\MinecraftQuery.
[failure] 63-63:
Call to method Connect() on an unknown class xPaw\MinecraftQuery.
[failure] 60-60:
Instantiated class xPaw\MinecraftQuery not found.
[failure] 50-50:
Call to method Close() on an unknown class xPaw\MinecraftPing.
[failure] 33-33:
Call to method Query() on an unknown class xPaw\MinecraftPing.
[failure] 31-31:
Instantiated class xPaw\MinecraftPing not found.
player-counter/src/Extensions/Query/Schemas/GoldSourceQueryTypeSchema.php
[failure] 22-22:
Access to constant GOLDSOURCE on an unknown class xPaw\SourceQuery\SourceQuery.
player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php
[failure] 29-29:
Call to method GetInfo() on an unknown class xPaw\MinecraftQuery.
[failure] 27-27:
Call to method ConnectBedrock() on an unknown class xPaw\MinecraftQuery.
[failure] 24-24:
Instantiated class xPaw\MinecraftQuery not found.
🔇 Additional comments (19)
.github/workflows/lint.yml (1)
36-36: Whitespace-only change.
No review action needed.player-counter/README.md (2)
7-9: Docs update looks good.
Setup guidance matches the new query behavior.
14-14: Feature list update looks good.
Clearly communicates multi-protocol support.player-counter/database/Seeders/PlayerCounterSeeder.php (3)
8-8: Import change is fine.
No action needed.
13-74: Mapping table is clear and maintainable.
Nice consolidation of per-egg logic into a single mapping list.
102-102: Status message update looks good.
Accurate summary of the new behavior.player-counter/plugin.json (1)
18-19: Confirm panel supports PHP 8.1+.
The packages exist on Packagist: xpaw/php-minecraft-query v5.0.0 requires PHP >= 8.0, and xpaw/php-source-query-class v5.0.0 requires PHP >= 8.1. Ensure the panel's minimum PHP version is 8.1 or higher to satisfy the stricter requirement from xpaw/php-source-query-class.player-counter/src/Extensions/Query/QueryTypeSchemaInterface.php (1)
5-12: LGTM — clear schema contract.Concise interface and return shape documentation look solid.
player-counter/src/Providers/PlayerCounterPluginProvider.php (1)
9-40: LGTM — centralized registration of default query schemas.Clear, straightforward wiring for the new registry.
player-counter/src/Filament/Admin/Resources/GameQueries/GameQueryResource.php (1)
49-52: LGTM — UI now reflects registered query types.The mapping-based badge label and select options integrate cleanly.
Also applies to: 77-81
player-counter/src/Extensions/Query/QueryTypeService.php (1)
10-13: UseArr::get()instead of thearray_get()helper for better compatibility.The
array_get()helper function is not available in modern Laravel versions without thelaravel/helperspackage installed. Since the plugin structure prevents verification of dependencies, usingIlluminate\Support\Arr::get()is the safer, more explicit approach.♻️ Suggested update
<?php namespace Boy132\PlayerCounter\Extensions\Query; +use Illuminate\Support\Arr; + class QueryTypeService { /** `@var` QueryTypeSchemaInterface[] */ private array $schemas = []; public function get(string $id): ?QueryTypeSchemaInterface { - return array_get($this->schemas, $id); + return Arr::get($this->schemas, $id); }player-counter/src/Extensions/Query/Schemas/GoldSourceQueryTypeSchema.php (1)
19-22: The constantSourceQuery::GOLDSOURCEis defined inxpaw/php-source-query-class(v5.0.0+) as declared in plugin.json. No issue here.player-counter/src/Http/Controllers/Api/Client/Servers/PlayerCounterController.php (1)
51-69: LGTM — centralized allocation/offline checks before querying.player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (1)
21-49: LGTM — Source schema normalizes info/players cleanly.player-counter/src/Models/GameQuery.php (1)
51-59: LGTM — centralized allocation validation.player-counter/src/Filament/Server/Pages/PlayersPage.php (2)
38-49: LGTM — access now uses GameQuery::canRunQuery.
86-243: LGTM — name-field normalization is consistent across UI/actions.player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php (1)
5-29: Dependency is properly declared—xPaw packages are configured in plugin.jsonThe
xpaw/php-minecraft-querypackage is already specified as^5.0.0inplayer-counter/plugin.jsonundercomposer_packages, which provides thexPaw\MinecraftQueryclass. The PHPStan warning may reflect missing PHPStan configuration rather than an actual missing dependency.Likely an incorrect or invalid review comment.
player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php (1)
7-8: Remove the dependency concern—xPaw\MinecraftPingandxPaw\MinecraftQueryare properly declared inplayer-counter/plugin.jsonundercomposer_packages.The xPaw packages (
xpaw/php-minecraft-queryandxpaw/php-source-query-classat version ^5.0.0) are correctly declared. The plugin system resolves these dependencies, and PHPStan should not flag them as missing when properly configured.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
player-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.php
Show resolved
Hide resolved
player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php
Show resolved
Hide resolved
player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@player-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.php`:
- Around line 27-30: The HTTP client setup in CitizenFXQueryTypeSchema uses
connectTimeout(5) which only limits TCP connect time but not total request
duration; update the fluent chain on Http (where acceptJson(), connectTimeout(),
throw(), baseUrl(...) are called) to include a total response timeout via
->timeout(...) (choose an appropriate seconds value, e.g., 5 or more) so
requests can't hang indefinitely; place the timeout call in the chain (e.g.,
after connectTimeout()) to enforce overall request duration.
- Around line 55-57: The helper calls is_ip($ip) and is_ipv6($ip) are undefined
and must be replaced with PHP filter_var checks; update all occurrences (e.g.,
in GameQuery methods referenced in GameQuery.php and in CitizenFXQueryTypeSchema
around the is_ip call) to use filter_var($ip, FILTER_VALIDATE_IP) !== false for
is_ip and filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false for
is_ipv6 (also update the same patterns in the subdomains plugin files) so the
code no longer calls non-existent helper functions.
🧹 Nitpick comments (5)
player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php (1)
35-41: Consider defensive array key access.The code accesses array keys directly without verifying their existence. While
is_array($info)check is present, ifGetInfo()returns an array missing expected keys (e.g., during protocol changes or partial responses), this could cause undefined index warnings.This is consistent with
MinecraftJavaQueryTypeSchema, so it's not a blocker, but consider adding null coalescing for robustness.♻️ Optional defensive improvement
return [ - 'hostname' => $info['HostName'], - 'map' => $info['Map'], - 'current_players' => $info['Players'], - 'max_players' => $info['MaxPlayers'], + 'hostname' => $info['HostName'] ?? '', + 'map' => $info['Map'] ?? '', + 'current_players' => $info['Players'] ?? 0, + 'max_players' => $info['MaxPlayers'] ?? 0, 'players' => null, // Bedrock has no player list ];player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php (2)
42-44: Consider defensive access for players data.While the SLP protocol should guarantee
players.onlineandplayers.max, a malformed response could cause errors. Consider adding defensive checks or validation.♻️ Optional defensive approach
return [ 'hostname' => $hostname, 'map' => 'world', // No map from MinecraftPing - 'current_players' => $data['players']['online'], - 'max_players' => $data['players']['max'], + 'current_players' => $data['players']['online'] ?? 0, + 'max_players' => $data['players']['max'] ?? 0, 'players' => $data['players']['sample'] ?? [], ];
72-78: Consider defensive access for info array keys.Direct access to
$info['HostName'],$info['Map'], etc. could throw notices if the server returns an unexpected response structure. Theis_array($info)check confirms it's an array but not that all required keys exist.♻️ Optional defensive approach
return [ - 'hostname' => $info['HostName'], - 'map' => $info['Map'], - 'current_players' => $info['Players'], - 'max_players' => $info['MaxPlayers'], + 'hostname' => $info['HostName'] ?? '', + 'map' => $info['Map'] ?? 'world', + 'current_players' => $info['Players'] ?? 0, + 'max_players' => $info['MaxPlayers'] ?? 0, 'players' => array_map(fn ($player) => ['id' => (string) $player, 'name' => (string) $player], $players), ];player-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.php (2)
39-45: Consider defensive array access for robustness.Direct key access will produce warnings if the API response is malformed. Using null coalescing or explicit checks would make this more resilient to unexpected responses.
♻️ Proposed defensive access
return [ - 'hostname' => $info['hostname'], - 'map' => $info['mapname'], - 'current_players' => $info['clients'], - 'max_players' => $info['sv_maxclients'], - 'players' => array_map(fn ($player) => ['id' => (string) $player['id'], 'name' => (string) $player['name']], $players), + 'hostname' => $info['hostname'] ?? '', + 'map' => $info['mapname'] ?? '', + 'current_players' => $info['clients'] ?? 0, + 'max_players' => $info['sv_maxclients'] ?? 0, + 'players' => array_map(fn ($player) => ['id' => (string) ($player['id'] ?? ''), 'name' => (string) ($player['name'] ?? '')], $players), ];
59-71: Guard against malformed DNS records.After the
!$recordcheck, the code assumes$record[0]exists withtargetandportkeys. Ifdns_get_recordreturns an array with a different structure, this could cause warnings.♻️ Proposed safer access
$record = dns_get_record('_cfx._udp.' . $ip, DNS_SRV); - if (!$record) { + if (!$record || !isset($record[0])) { return; } - if ($record[0]['target']) { + if (!empty($record[0]['target'])) { $ip = $record[0]['target']; } - if ($record[0]['port']) { + if (!empty($record[0]['port'])) { $port = (int) $record[0]['port']; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
player-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.phpplayer-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.phpplayer-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.phpplayer-counter/src/Models/GameQuery.php
🚧 Files skipped from review as they are similar to previous changes (1)
- player-counter/src/Models/GameQuery.php
🧰 Additional context used
🧬 Code graph analysis (3)
player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php (1)
player-counter/src/Extensions/Query/QueryTypeSchemaInterface.php (3)
getId(7-7)getName(9-9)process(12-12)
player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php (3)
player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php (3)
getId(12-15)getName(17-20)process(23-55)player-counter/src/Extensions/Query/QueryTypeSchemaInterface.php (3)
getId(7-7)getName(9-9)process(12-12)player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (3)
getId(11-14)getName(16-19)process(22-25)
player-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.php (5)
player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php (3)
getId(11-14)getName(16-19)process(22-47)player-counter/src/Extensions/Query/QueryTypeSchemaInterface.php (3)
getId(7-7)getName(9-9)process(12-12)player-counter/src/Extensions/Query/Schemas/GoldSourceQueryTypeSchema.php (3)
getId(9-12)getName(14-17)process(20-23)player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (3)
getId(11-14)getName(16-19)process(22-25)player-counter/src/Extensions/Query/QueryTypeService.php (1)
get(10-13)
🔇 Additional comments (6)
player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php (3)
9-10: LGTM!Class declaration and interface implementation follow the established pattern from other schema implementations.
11-19: LGTM!The
getId()andgetName()implementations are consistent with other schema classes likeMinecraftJavaQueryTypeSchema.
42-47: LGTM!Error handling follows the established pattern: exceptions are reported for observability, and
nullis returned to indicate query failure, allowing the caller to handle it gracefully.player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php (2)
12-20: LGTM!The
getIdandgetNamemethods correctly implement the interface with appropriate return values.
46-54: LGTM on exception handling.Good use of try-catch-finally pattern to ensure the MinecraftPing connection is properly closed regardless of success or failure.
player-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.php (1)
11-14: Note: AI summary inconsistency.The AI summary states
getId()returns"citizen_fx", but the actual implementation returns'cfx'. The code is correct; this is just a discrepancy in the summary.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary by CodeRabbit
New Features
Improvements
Documentation
Dependencies
✏️ Tip: You can customize this high-level summary in your review settings.