[Player Counter] Add Palworld Support#125
Conversation
…ings for admin password.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughA new Palworld query schema is introduced alongside an interface signature update. Existing query schema implementations are updated to accept a Server model parameter, and the new Palworld schema is registered in the plugin provider. The Palworld implementation fetches player and metrics data via pooled HTTP requests with basic authentication. ChangesPalworld Query Schema Integration
Sequence DiagramsequenceDiagram
participant Client
participant PlayerCounterService
participant QueryTypeService
participant PalworldSchema
participant PalworldAPI as Palworld API
Client->>PlayerCounterService: Query server info
PlayerCounterService->>QueryTypeService: Get schema for palworld
QueryTypeService->>PalworldSchema: process(Server, ip, port)
alt ADMIN_PASSWORD not configured
PalworldSchema-->>PlayerCounterService: null
else ADMIN_PASSWORD configured
PalworldSchema->>PalworldAPI: Pool request: /v1/api/players (basic auth, 5s timeout)
PalworldSchema->>PalworldAPI: Pool request: /v1/api/metrics (basic auth, 5s timeout)
alt Players response not OK or metrics unavailable
PalworldSchema-->>PlayerCounterService: null or partial data
else Both responses OK
PalworldSchema->>PalworldSchema: Map players array, compute max_players
PalworldSchema-->>PlayerCounterService: Server snapshot {hostname, map, current_players, max_players, players}
end
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (1)
player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php (1)
60-62: Hardcoded map name and player cap fallback.
'Palpagos Islands'is Palworld's vanilla world name and32is the default server cap; both can differ for modded/custom servers. If a suitable field is returned by/v1/api/metrics(or another endpoint), prefer dynamic values; otherwise, consider translating the map label or making it configurable. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php` around lines 60 - 62, The code currently hardcodes the map name and fallback max player cap ('Palpagos Islands' and 32) in the array built in PalworldQueryTypeSchema; replace those literals with dynamic values by checking $metrics (e.g. use $metrics['map'] or $metrics['world_name'] if present) and prefer a configurable fallback from a config/service (or translate/label the map name) and use $metrics['maxplayers'] ?? config('palworld.default_max_players') instead of the literal 32; update the array keys 'map' and 'max_players' to pull from these sources while leaving 'current_players' => count($players) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php`:
- Around line 47-64: The array and return key/value pairs use vertical alignment
around the => operator; update the mappings in the array_map callback (the
$players mapping using 'id' => and 'name' =>) and the returned associative array
keys ('hostname' =>, 'map' =>, 'current_players' =>, 'max_players' =>, 'players'
=>) to remove extra spaces so each => has a single space on both sides (collapse
multiple spaces to one) to conform with Pint's default preset.
- Around line 53-64: The code reads wrong metric keys and skips status checks:
before calling ->json() on the metrics response, check ->ok() (like the players
call) and handle non-OK responses; then read the actual API fields from the
decoded $metrics array—use 'currentplayernum' for current players and
'maxplayernum' for max players (fall back to count($players) and 32 respectively
if absent), and do not attempt to read a non-existent 'servername' (use
$server->name as the hostname fallback). Locate the metrics HTTP call and the
return array in PalworldQueryTypeSchema (the $metrics variable and the returned
'hostname','current_players','max_players' entries) and apply these changes.
---
Nitpick comments:
In `@player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php`:
- Around line 60-62: The code currently hardcodes the map name and fallback max
player cap ('Palpagos Islands' and 32) in the array built in
PalworldQueryTypeSchema; replace those literals with dynamic values by checking
$metrics (e.g. use $metrics['map'] or $metrics['world_name'] if present) and
prefer a configurable fallback from a config/service (or translate/label the map
name) and use $metrics['maxplayers'] ?? config('palworld.default_max_players')
instead of the literal 32; update the array keys 'map' and 'max_players' to pull
from these sources while leaving 'current_players' => count($players) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 85bff950-3d20-413b-84be-549eb638dbad
📒 Files selected for processing (4)
player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.phpplayer-counter/src/Extensions/Query/ServerAwareQueryTypeSchemaInterface.phpplayer-counter/src/Models/GameQuery.phpplayer-counter/src/Providers/PlayerCounterPluginProvider.php
📜 Review details
🧰 Additional context used
🪛 GitHub Actions: Lint
player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php
[error] 1-1: Laravel Pint style check failed: 1 style issue found in 210 files. Failed at PalworldQueryTypeSchema.php.
🪛 PHPMD (2.15.0)
player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php
[warning] 22-22: Avoid unused parameters such as '$ip'. (undefined)
(UnusedFormalParameter)
[warning] 22-22: Avoid unused parameters such as '$port'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (4)
player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php (1)
22-25: Intentional null-return fallback — PHPMD warnings are false positives.The PHPMD
UnusedFormalParameterwarnings on$ip/$portcan be safely ignored here: the method signature is fixed byQueryTypeSchemaInterface::process(), and returningnullfrom the non-server-aware path is the documented fallback for when auth context is unavailable.player-counter/src/Extensions/Query/ServerAwareQueryTypeSchemaInterface.php (1)
1-11: LGTM!Clean interface design. Extending
QueryTypeSchemaInterfacepreserves backward compatibility (implementors must still provideprocess()), and the return-type contract onprocessWithServer()matches the parent'sprocess()signature so both code paths inGameQuery::runQuery()yield the same shape.player-counter/src/Providers/PlayerCounterPluginProvider.php (1)
14-14: LGTM!Registration follows the same pattern as the other default schemas;
QueryTypeService::register()accepts anyQueryTypeSchemaInterface, soServerAwareQueryTypeSchemaInterfaceimplementors are fine here.Also applies to: 39-39
player-counter/src/Models/GameQuery.php (1)
63-68: LGTM — branching preserves null-safety.Since
QueryTypeService::get()can returnnull, theinstanceofcheck cleanly falls through to the existing$schema?->process($ip, $port)null-safe call. Behavior for all previously registered (non-server-aware) schemas is unchanged.
- hostname now uses $server->name directly (metrics has no servername field) - max_players now reads maxplayernum from metrics (not maxplayers) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php (1)
37-57: Consider parallelizing the two independent REST API calls usingHttp::pool().The
playersandmetricsrequests run sequentially with 5s timeouts each, resulting in worst-case ~10s latency. Since they are independent,Http::pool()would execute them concurrently and reduce worst-case latency to ~5s on slow/unreachable servers.♻️ Sketch using Http::pool
- $response = Http::timeout(5) - ->withBasicAuth('admin', $adminPassword) - ->get("http://{$ip}:{$port}/v1/api/players"); - - if (!$response->ok()) { - return null; - } - - $data = $response->json(); - $players = array_map(fn ($p) => [ - 'id' => $p['playeruid'] ?? $p['steamid'] ?? '', - 'name' => $p['name'] ?? '', - ], $data['players'] ?? []); - - // Fetch metrics for max_players - $metricsResponse = Http::timeout(5) - ->withBasicAuth('admin', $adminPassword) - ->get("http://{$ip}:{$port}/v1/api/metrics"); - - $maxPlayers = $metricsResponse->ok() ? ($metricsResponse->json()['maxplayernum'] ?? 32) : 32; + $base = "http://{$ip}:{$port}"; + [$playersResp, $metricsResp] = Http::pool(fn ($pool) => [ + $pool->timeout(5)->withBasicAuth('admin', $adminPassword)->get("{$base}/v1/api/players"), + $pool->timeout(5)->withBasicAuth('admin', $adminPassword)->get("{$base}/v1/api/metrics"), + ]); + + if (!$playersResp->ok()) { + return null; + } + + $data = $playersResp->json(); + $players = array_map(fn ($p) => [ + 'id' => $p['playeruid'] ?? $p['steamid'] ?? '', + 'name' => $p['name'] ?? '', + ], $data['players'] ?? []); + + $maxPlayers = $metricsResp->ok() ? ($metricsResp->json()['maxplayernum'] ?? 32) : 32;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php` around lines 37 - 57, The two independent HTTP requests in PalworldQueryTypeSchema.php (the player list request that sets $response/$players and the metrics request that sets $metricsResponse/$maxPlayers) should be executed in parallel using Http::pool(): create two request closures with the same ->timeout(5)->withBasicAuth('admin', $adminPassword) setup and call Http::pool([...]) to obtain both responses, then handle each response (check ok(), call ->json(), defaulting players to empty array and maxPlayers to 32) and keep the existing mapping logic (array_map for players and the fallback for maxPlayers) so behavior remains unchanged but latency is reduced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php`:
- Around line 37-57: The two independent HTTP requests in
PalworldQueryTypeSchema.php (the player list request that sets
$response/$players and the metrics request that sets
$metricsResponse/$maxPlayers) should be executed in parallel using Http::pool():
create two request closures with the same ->timeout(5)->withBasicAuth('admin',
$adminPassword) setup and call Http::pool([...]) to obtain both responses, then
handle each response (check ok(), call ->json(), defaulting players to empty
array and maxPlayers to 32) and keep the existing mapping logic (array_map for
players and the fallback for maxPlayers) so behavior remains unchanged but
latency is reduced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 10ab2963-aca1-43d6-9bb6-4b9c21041a9c
📒 Files selected for processing (1)
player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php
📜 Review details
🧰 Additional context used
🪛 PHPMD (2.15.0)
player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php
[warning] 22-22: Avoid unused parameters such as '$ip'. (undefined)
(UnusedFormalParameter)
[warning] 22-22: Avoid unused parameters such as '$port'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (3)
player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php (3)
22-25: LGTM — sensible no-auth fallback.Returning
nullfromprocess()is the right choice here since Palworld's REST API requires the admin password, which is only available viaprocessWithServer(). The dispatch inGameQuery::runQuery()correctly checksinstanceof ServerAwareQueryTypeSchemaInterfacefirst, so this method should not be hit in practice for this schema.Note: PHPMD's
UnusedFormalParameterwarnings on$ip/$portare false positives — the parameters are mandated by theQueryTypeSchemaInterfacecontract.
53-57: Past review concerns addressed.The metrics call now uses the correct
maxplayernumfield, falls back to$server->namefor hostname (since the metrics endpoint has noservernamefield), and the inline->ok()check guards against decoding error bodies. Looks good.
61-61: [Your rewritten review comment text here]
[Exactly ONE classification tag]
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@player-counter/src/Extensions/Query/QueryTypeSchemaInterface.php`:
- Line 14: The fallback call to the schema processor uses the old 2-arg
signature; update the call in GameQuery (where $schema is used) from
$schema?->process($ip, $port) to include the Server object so it becomes
$schema?->process($server, $ip, $port), matching the
QueryTypeSchemaInterface::process(Server $server, string $ip, int $port)
signature and using the already-in-scope $server, $ip, and $port variables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab1a176c-7890-47df-9e87-04533e952ee1
📒 Files selected for processing (7)
player-counter/src/Extensions/Query/QueryTypeSchemaInterface.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/PalworldQueryTypeSchema.phpplayer-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php
🚧 Files skipped from review as they are similar to previous changes (1)
- player-counter/src/Extensions/Query/Schemas/PalworldQueryTypeSchema.php
📜 Review details
🧰 Additional context used
🪛 PHPMD (2.15.0)
player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php
[warning] 24-24: Avoid unused parameters such as '$server'. (undefined)
(UnusedFormalParameter)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php
[warning] 23-23: Avoid unused parameters such as '$server'. (undefined)
(UnusedFormalParameter)
player-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.php
[warning] 23-23: Avoid unused parameters such as '$server'. (undefined)
(UnusedFormalParameter)
player-counter/src/Extensions/Query/Schemas/GoldSourceQueryTypeSchema.php
[warning] 21-21: Avoid unused parameters such as '$server'. (undefined)
(UnusedFormalParameter)
player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php
[warning] 23-23: Avoid unused parameters such as '$server'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (5)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (1)
23-26: Signature update is clean and behavior-preserving.The schema now matches the new contract while keeping existing Source query logic unchanged.
player-counter/src/Extensions/Query/Schemas/GoldSourceQueryTypeSchema.php (1)
21-24: Looks good — contract alignment without functional drift.
process(...)now matches the shared interface and keeps GoldSrc behavior intact.player-counter/src/Extensions/Query/Schemas/MinecraftJavaQueryTypeSchema.php (1)
24-37: Good contract migration for Java schema.The updated signature is consistent, and the query→ping fallback flow remains unchanged.
player-counter/src/Extensions/Query/Schemas/CitizenFXQueryTypeSchema.php (1)
23-52: Nice signature-only migration with stable behavior.CitizenFX request/validation flow is preserved while adopting the new schema contract.
player-counter/src/Extensions/Query/Schemas/MinecraftBedrockQueryTypeSchema.php (1)
23-48: Bedrock schema update is consistent and safe.The method now conforms to the new interface and keeps the prior Bedrock behavior.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
player-counter/src/Models/GameQuery.php (1)
62-64: 💤 Low valueLGTM — the
$serverpassthrough is correct.
$service->get()returns?QueryTypeSchemaInterfaceand the null-safe?->correctly propagatesnullwhen the query type is unregistered. The new$serverargument satisfies the updated interface signature inQueryTypeSchemaInterface.The intermediate
$schemavariable is only used once; it can optionally be collapsed to keep the method concise:♻️ Optional one-liner
- $schema = $service->get($this->query_type); - - return $schema?->process($server, $ip, $port); + return $service->get($this->query_type)?->process($server, $ip, $port);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@player-counter/src/Models/GameQuery.php` around lines 62 - 64, Collapse the unnecessary intermediate variable $schema by inlining the null-safe call: replace the two-line sequence where $schema = $service->get($this->query_type); is immediately followed by return $schema?->process($server, $ip, $port); with a single return that directly invokes $service->get($this->query_type)?->process($server, $ip, $port); ensuring you still pass $server, $ip, $port to process and preserve the null-safe behavior of QueryTypeSchemaInterface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@player-counter/src/Models/GameQuery.php`:
- Around line 62-64: Collapse the unnecessary intermediate variable $schema by
inlining the null-safe call: replace the two-line sequence where $schema =
$service->get($this->query_type); is immediately followed by return
$schema?->process($server, $ip, $port); with a single return that directly
invokes $service->get($this->query_type)?->process($server, $ip, $port);
ensuring you still pass $server, $ip, $port to process and preserve the
null-safe behavior of QueryTypeSchemaInterface.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8780cb19-90d4-416d-87d9-2e281692c068
📒 Files selected for processing (1)
player-counter/src/Models/GameQuery.php
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: PHPStan (8.2)
- GitHub Check: PHPStan (8.3)
Co-authored-by: Boy132 <Boy132@users.noreply.github.com>
Boy132
changed the title
2 participants
This pull request introduces support for Palworld dedicated servers in the Player Counter plugin. Palworld does not
implement the Steam A2S query protocol despite binding the query port, so this adds a new interface and schema to
support REST API-based querying with server credential context.
*Assisted by Claude to match existing patterns
Relates to #124
Summary by CodeRabbit