Player Counter: Added Palworld Support#125
Player Counter: Added Palworld Support#125makepeacej wants to merge 7 commits intopelican-dev:mainfrom
Conversation
…ings for admin password.
📝 WalkthroughWalkthroughAdds Palworld REST query support and updates the query schema contract to accept a ChangesPalworld + Schema Signature Update
Sequence DiagramsequenceDiagram
participant GQ as GameQuery
participant Schema as PalworldQueryTypeSchema
participant PA as Palworld API
GQ->>Schema: process(server, ip, port)
activate Schema
Schema->>Schema: read ADMIN_PASSWORD from server variables
alt no password
Schema-->>GQ: return null
else password present
Schema->>PA: HTTP Pool request /v1/api/players (basic auth)
PA-->>Schema: players response
Schema->>PA: HTTP Pool request /v1/api/metrics (basic auth)
PA-->>Schema: metrics response
alt players OK
Schema->>Schema: map players, count current_players
alt metrics OK
Schema->>Schema: set max_players from metrics
else
Schema->>Schema: set max_players = 32
end
Schema-->>GQ: return normalized snapshot
else
Schema-->>GQ: return null
end
end
deactivate Schema
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related issues
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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.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]
|
|
||
| use App\Models\Server; | ||
|
|
||
| interface ServerAwareQueryTypeSchemaInterface extends QueryTypeSchemaInterface |
There was a problem hiding this comment.
I don't think we need a separate interface for this. Simply add the server as param to the existing process function. Doesn't harm to always have a reference to the server.
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)
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
New Features
Refactor