Skip to content

Patch 4#5

Open
KanadeBlue wants to merge 44 commits into
stablefrom
Patch-4
Open

Patch 4#5
KanadeBlue wants to merge 44 commits into
stablefrom
Patch-4

Conversation

@KanadeBlue
Copy link
Copy Markdown

@KanadeBlue KanadeBlue commented Jun 4, 2025

Related issues & PRs

Changes

API changes

Behavioural changes

Backwards compatibility

Follow-up

Tests

Summary by CodeRabbit

  • New Features

    • Introduced a comprehensive form API with customizable, simple, modal, menu, and server settings forms, supporting various input elements, validation, and response handling.
    • Added player-specific task scheduling to manage tasks per player.
    • Added new block and item types, including multiple colored harnesses, a new structure void block, and new block state constants.
    • Added inventory menu system with graphical representations, network translation, and transaction handling.
    • Added support for Minecraft: Bedrock Edition 1.21.80 and removed support for earlier versions.
  • Improvements

    • Updated biome definitions to use structured JSON format for better compatibility.
    • Enforced string-only tags in enchantment registry to prevent crashes.
    • Enhanced validation and error handling in entity fire ticks, player item damage, and inventory logic.
    • Updated dependencies and PHPStan configurations.
    • Improved PHPStan error reporting for array key types.
    • Refined network packet handling and player input processing.
    • Improved synchronization in thread crash info retrieval to ensure complete data.
    • Updated block data serialization and deserialization for new block types.
    • Added network latency management and animation wait handling for inventory menus.
    • Simplified chunk request task by pre-encoding chunk data.
    • Improved update check task with better error handling and JSON mapping.
    • Refined world light population logic with safer chunk handling and state checks.
    • Streamlined population task with clearer chunk management and serialization.
    • Improved light population task code clarity and structure.
  • Bug Fixes

    • Fixed PlayStation player joining issues and version constraint errors.
    • Addressed RakLib ping timestamp handling.
  • Chores

    • Updated GitHub Actions workflows with newer runner images and action versions.
    • Added exclusions to code style checks to preserve doc comments.
    • Refactored label removal logic in workflows for better error handling.
    • Added a scheduled workflow to trigger branch syncs regularly to prevent workflow disabling.

dktapps and others added 30 commits April 6, 2025 04:52
as written in the comments, it's not reasonable to propagate this limitation, since it
ultimately comes from a shortfall in the Mojang save format, not a limitation of PM's
capability. It's also not obvious how this would be propagated to the likes of setOnFire(),
as this would translate into a max time of 1638 seconds, a value no one is going to
remember.

There's a case to be made for truncating this on save rather than on initial set, but
this is at least better than having Fire Aspect level 1000 cause crashes and whatever
other gameplay logic that would have to work around this stupid limitation.
these are actually two separate concerns: one for dodgy PHPStan type suppression on implicit keys, and the other for arrays being casted to strings by PHP.
* Bedrock 1.21.80 support

* Update bedrock-data

* Add required tags to models

* Fixed biome data loading

* Support newest world format
apparently I messed up the blockstate data version last time around... it hasn't changed since 1.21.60

* always CS has to complain...

* Sync with release versions

* Ready 5.28.0 release

* this might help...

---------

Co-authored-by: Dylan T. <dktapps@pmmp.io>
and fix an error found by new PHPStan update
we don't need this dependency anyway because mbstring is already provided.
we're having problems with the restricted action getting disabled due to repo inactivity,
so it's best we trigger it from here, since this repo's activity is what it's
interested in anyway.
…info

closes pmmp#6669

this happens because isTerminated returns true before the thread's shutdown handler runs,
so we join with the thread to make sure that shutdown handlers are done before returning.

... hopefully we don't get servers randomly deadlocking in shutdown handlers ???
actions/stale is far too slow to do this itself since it processes lots of irrelevant crap on every run
API additions need to wait for the next minor release

Revert "Entity: make stepHeight accessable (pmmp#6702)"

This reverts commit 5527a0c.
this can happen if the old item had a lower max damage than the new one, and the new
one has a damage higher than the old one's max damage.

it can also happen if the damage was overridden to some illegal value by a custom item
as seen in https://crash.pmmp.io/view/12754811
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2025

Walkthrough

This update introduces a comprehensive overhaul and expansion of the form API, adds new data models for biome definitions, and transitions biome data handling from NBT to JSON. It updates dependencies, constants, and workflow configurations to support Minecraft: Bedrock Edition 1.21.80, and implements new per-player task scheduling. Numerous workflow files are modernized, and changelogs document the latest releases and fixes.

Changes

File(s) / Path(s) Change Summary
.github/workflows/*.yml Updated workflow runner environments to Ubuntu 22.04, upgraded action versions, added a new branch sync trigger workflow, and made minor script improvements (e.g., label removal logic, Docker and PHP setup, Discord notification).
changelogs/5.27.md, changelogs/5.28.md Added changelogs for versions 5.27.1 and 5.28.0–5.28.2, detailing feature support, fixes, and plugin compatibility notes.
composer.json Updated dependencies for Bedrock data/protocol and RakLib, bumped PHPStan, and added a "replace" section for polyfills.
.php-cs-fixer.php Excluded specific directories from PHP CS Fixer due to doc comment FQN requirements.
phpstan.neon.dist Switched to a new PHPStan rule for safer foreach usage.
build/dump-version-info.php, src/block/tile/Spawnable.php, src/utils/Utils.php, src/world/World.php Adjusted PHPStan or PHPDoc annotations for types and validator closures; no logic changes.
src/VersionInfo.php Changed software name to "STCraft-MP" and updated base version to 5.28.1.
src/data/bedrock/BedrockDataFiles.php Replaced NBT biome definition constants with a JSON file constant.
src/data/bedrock/block/BlockStateData.php, src/world/format/io/data/BedrockWorldData.php Updated version constants for block state data and world data to match new protocol.
src/data/bedrock/block/BlockStateNames.php, src/data/bedrock/block/BlockTypeNames.php, src/data/bedrock/item/ItemTypeNames.php Added new constants for block states, block types, and item types (notably harnesses and a ghast spawn egg).
src/entity/Entity.php Modified fire tick setter: now clamps values above the max instead of throwing, and only errors on negatives.
src/inventory/BaseInventory.php Refactored item addition logic for early return when items are fully added.
src/item/enchantment/AvailableEnchantmentRegistry.php Enforced string type validation for enchantment tag arrays.
src/network/mcpe/cache/StaticPacketCache.php, tools/generate-bedrock-data-from-packets.php Migrated biome definition loading and saving from NBT to structured JSON; added strict type mapping and validation.
src/network/mcpe/handler/InGamePacketHandler.php Removed unused packet handler and improved input flag comparison logic.
src/network/mcpe/handler/LoginPacketHandler.php Added check for "GraphicsMode" key in client data claims, defaulting to 1 if missing.
src/player/Player.php Added validation for item durability when restoring held items.
src/scheduler/TaskScheduler.php Added per-player task scheduling, canceling, and tracking methods; improved task queue handling.
src/thread/CommonThreadPartsTrait.php, src/thread/ThreadCrashInfo.php Ensured thread crash info is fully populated before access; removed fallback for crash type message.
src/world/biome/model/BiomeDefinitionEntryData.php, src/world/biome/model/ColorData.php Introduced new data model classes for biome definitions and color data, used in JSON serialization.
src/form/FormAPI/Form.php, src/form/FormAPI/IForm.php, src/form/FormAPI/SimpleForm.php, src/form/FormAPI/CustomForm.php Introduced a new form API: interface, abstract base, and implementations for simple and custom forms with validation and serialization.
src/form/PMForm/* Added a new, extensible Bedrock UI form framework: base form, menu, modal, custom, server settings forms, menu options, form icons, and a variety of custom form elements (label, toggle, slider, step slider, dropdown, input), each with strict validation and serialization.
src/form/ChestAPI/* Added a comprehensive inventory menu system with classes for menus, event handling, session management, network handling, transaction management, menu types, graphics, network translators, and builder utilities.
tests/phpstan/rules/UnsafeForeachRule.php Renamed and restructured a PHPStan rule to improve error reporting for unsafe foreach usage.
src/block/BlockTypeIds.php, src/block/VanillaBlocks.php, src/data/bedrock/block/convert/*.php, src/item/StringToItemParser.php Added new block type STRUCTURE_VOID with registration, aliasing, and serializer/deserializer support.

Sequence Diagram(s)

New Form API – Simple Form Submission

sequenceDiagram
    participant Plugin as Plugin Code
    participant SimpleForm as SimpleForm
    participant Player as Player

    Plugin->>SimpleForm: create (with callable)
    Plugin->>SimpleForm: setTitle(), setContent(), addButton()
    Plugin->>Player: sendForm(SimpleForm)
    Player->>SimpleForm: submit (button index)
    SimpleForm->>SimpleForm: processData(index)
    SimpleForm->>Plugin: invoke callable (Player, label)
Loading

Biome Definition Loading (JSON-based)

sequenceDiagram
    participant StaticPacketCache
    participant JSONFile as BiomeDefinition JSON
    participant JsonMapper
    participant BiomeDefinitionEntry

    StaticPacketCache->>JSONFile: read
    StaticPacketCache->>JsonMapper: map JSON to BiomeDefinitionEntryData
    JsonMapper->>BiomeDefinitionEntry: create objects (with color/tags)
    StaticPacketCache->>StaticPacketCache: build BiomeDefinitionListPacket
Loading

Per-Player Task Scheduling

sequenceDiagram
    participant Plugin as Plugin Code
    participant TaskScheduler
    participant Player

    Plugin->>TaskScheduler: scheduleForPlayer(Player, Task)
    TaskScheduler->>TaskScheduler: check if already scheduled
    TaskScheduler->>TaskScheduler: add repeating task for Player
    TaskScheduler->>Player: execute Task on tick
    Plugin->>TaskScheduler: cancelForPlayer(Player)
    TaskScheduler->>TaskScheduler: remove Player's task
Loading

Poem

Hopping through code with a whisker twitch,
New forms and biomes—JSON’s the switch!
Tasks for each bunny, scheduled with care,
Harnesses, ghasts, new constants to share.
From workflows updated to changelogs so neat,
This patch is a carrot-flavored treat! 🥕🐇

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 24

🔭 Outside diff range comments (1)
.github/workflows/pr-remove-waiting-label.yml (1)

35-38: ⚠️ Potential issue

Await label-removal calls and use correct PR number
Invoking removeLabel() without await may exit before the promise resolves. Also, context.payload.number is not guaranteed for pull_request events (should use context.issue.number).

Apply this diff:

-            removeLabel(owner, repo, context.payload.number, "Status: Waiting on Author");
-            removeLabel(owner, repo, context.payload.number, "Stale");
+            await removeLabel(owner, repo, context.issue.number, "Status: Waiting on Author");
+            await removeLabel(owner, repo, context.issue.number, "Stale");
♻️ Duplicate comments (2)
src/form/PMForm/FormIcon.php (2)

17-17: Fix the comment block formatting.

The pipeline failure indicates a missing space before the closing comment block.

🧰 Tools
🪛 GitHub Actions: CI

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.


55-61: Consider adding a return type declaration.

Same as MenuOption.php - consider adding return type if PHP 8.1+ is targeted.

🧹 Nitpick comments (19)
.github/workflows/branch-sync-cron-trigger.yml (1)

1-11: Configure schedule and manual trigger
The cron (0 0 * * *) and workflow_dispatch entries are correctly set.

Optional: add a concurrency block to prevent overlapping runs:

 name: Trigger branch sync
+concurrency:
+  group: branch-sync
+  cancel-in-progress: false
 on:
   schedule:
     - cron: "0 0 * * *"
src/VersionInfo.php (1)

33-34: Ensure branding and version consistency across project.
Project NAME and BASE_VERSION were updated; verify that composer.json, documentation, and CI workflows reflect the new STCraft-MP branding and version 5.28.3. Also consider updating the file header comment from “PocketMine Team” to align with STCraft branding.

src/form/PMForm/element/Label.php (1)

17-17: Fix the formatting issue flagged by PHP CS Fixer.

Add a space before the closing comment block as indicated by the pipeline failure.

- *
+ * 
 */
🧰 Tools
🪛 GitHub Actions: CI

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/element/StepSlider.php (1)

17-17: Fix the formatting issue flagged by PHP CS Fixer.

Add a space before the closing comment block as indicated by the pipeline failure.

- *
+ * 
 */
🧰 Tools
🪛 GitHub Actions: CI

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/element/Toggle.php (1)

17-17: Fix the formatting issue flagged by PHP CS Fixer.

Add a space before the closing comment block as indicated by the pipeline failure.

- *
+ * 
 */
🧰 Tools
🪛 GitHub Actions: CI

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/MenuOption.php (1)

31-38: Use typed properties for better type safety.

Since this code uses declare(strict_types=1), consider using PHP typed properties instead of @var comments for consistency with modern PHP practices.

Apply this diff to use typed properties:

-	/** @var string */
-	private $text;
-	/** @var FormIcon|null */
-	private $image;
+	private string $text;
+	private ?FormIcon $image;
src/form/PMForm/FormIcon.php (1)

33-44: Use typed properties and clarify the type limitation.

  1. Consider using PHP typed properties for consistency
  2. The comment mentions only "url" type works - consider documenting this limitation more prominently or validating the type in the constructor

Apply this diff:

-	/** @var string */
-	private $type;
-	/** @var string */
-	private $data;
+	private string $type;
+	private string $data;

 	/**
 	 * @param string $data URL or path depending on the type chosen.
-	 * @param string $type Can be one of the constants at the top of the file, but only "url" is known to work.
+	 * @param string $type IMAGE_TYPE_URL or IMAGE_TYPE_PATH. Note: Only IMAGE_TYPE_URL is known to work.
 	 */
 	public function __construct(string $data, string $type = self::IMAGE_TYPE_URL){
+		if($type !== self::IMAGE_TYPE_URL && $type !== self::IMAGE_TYPE_PATH){
+			throw new \InvalidArgumentException("Invalid image type: $type");
+		}
 		$this->type = $type;
 		$this->data = $data;
 	}
src/network/mcpe/cache/StaticPacketCache.php (1)

53-103: Well-implemented JSON loading with robust error handling.

The method properly validates input, uses JsonMapper with strict settings, and handles exceptions appropriately. The use of Utils::promoteKeys correctly handles numeric string keys in the JSON.

Consider extracting the BiomeDefinitionEntry construction into a factory method to improve readability given the 14 parameters.

src/form/PMForm/element/Input.php (1)

36-39: Consider using typed properties for better type safety.

Since the codebase appears to support modern PHP features (strict types, named parameters), consider using typed properties instead of docblock annotations.

-	/** @var string */
-	private $hint;
-	/** @var string */
-	private $default;
+	private string $hint;
+	private string $default;
src/form/FormAPI/IForm.php (1)

63-65: Remove unnecessary reference parameter.

The processData method accepts a reference parameter but doesn't modify the data, making the reference unnecessary and potentially confusing.

Apply this diff to fix the parameter signature:

-    public function processData(&$data) : void {
+    public function processData($data) : void {
         // Process the data as needed.
     }
src/form/PMForm/ModalForm.php (1)

39-49: Use native property type declarations instead of PHPDoc.

The code is using PHPDoc for type declarations on properties. PHP 7.4+ supports native typed properties which provide better type safety and IDE support.

-	/** @var string */
-	private $content;
+	private string $content;
 	/**
 	 * @var \Closure
 	 * @phpstan-var OnSubmit
 	 */
 	private $onSubmit;
-	/** @var string */
-	private $button1;
-	/** @var string */
-	private $button2;
+	private string $button1;
+	private string $button2;
src/form/PMForm/element/Slider.php (1)

33-40: Use native property type declarations instead of PHPDoc.

-	/** @var float */
-	private $min;
-	/** @var float */
-	private $max;
-	/** @var float */
-	private $step;
-	/** @var float */
-	private $default;
+	private float $min;
+	private float $max;
+	private float $step;
+	private float $default;
src/form/PMForm/CustomForm.php (1)

41-44: Use native property type declarations where possible.

-	/** @var CustomFormElement[] */
-	private $elements;
-	/** @var CustomFormElement[] */
-	private $elementMap = [];
+	/** @var CustomFormElement[] */
+	private array $elements;
+	/** @var CustomFormElement[] */
+	private array $elementMap = [];
src/form/PMForm/element/CustomFormElement.php (1)

32-35: Use typed properties for better type safety.

Consider using typed properties instead of @var annotations for modern PHP compatibility.

-	/** @var string */
-	private $name;
-	/** @var string */
-	private $text;
+	private string $name;
+	private string $text;
src/scheduler/TaskScheduler.php (2)

16-28: Add generic type specifications for PHPStan.

The PHPStan annotations are missing the generic type parameter TTask for TaskHandler.

-    /** @phpstan-var ReversePriorityQueue<int, TaskHandler> */
+    /** @phpstan-var ReversePriorityQueue<int, TaskHandler<Task>> */
     protected ReversePriorityQueue $queue;

     /**
      * @var ObjectSet|TaskHandler[]
-     * @phpstan-var ObjectSet<TaskHandler>
+     * @phpstan-var ObjectSet<TaskHandler<Task>>
      */
     protected ObjectSet $tasks;

-    /** @var array<string, TaskHandler> */
+    /** @var array<string, TaskHandler<Task>> */
     private array $playerTasks = [];
🧰 Tools
🪛 PHPStan (2.1.15)

17-17: Property pocketmine\scheduler\TaskScheduler::$queue with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


23-23: Property pocketmine\scheduler\TaskScheduler::$tasks with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


28-28: Property pocketmine\scheduler\TaskScheduler::$playerTasks with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


109-109: Remove trailing whitespace.

         $uniqueId = $player->getUniqueId()->toString();
-        
+
         if (isset($this->playerTasks[$uniqueId])) {
src/form/PMForm/MenuForm.php (1)

42-55: Use typed properties for consistency.

Consider using typed properties instead of @var annotations.

-	/** @var string */
-	protected $content;
-	/** @var MenuOption[] */
-	private $options;
+	protected string $content;
+	/** @var MenuOption[] */
+	private array $options;
 	/**
 	 * @var \Closure
 	 * @phpstan-var OnSubmit
 	 */
-	private $onSubmit;
+	private \Closure $onSubmit;
 	/**
 	 * @var \Closure|null
 	 * @phpstan-var OnClose
 	 */
-	private $onClose = null;
+	private ?\Closure $onClose = null;
src/form/FormAPI/SimpleForm.php (1)

11-13: Use visibility modifiers for class constants

Consider using visibility modifiers for better clarity and consistency with modern PHP standards.

-    const IMAGE_TYPE_PATH = 0;
-    const IMAGE_TYPE_URL = 1;
-    const IMAGE_TYPE_NONE = -1;
+    public const IMAGE_TYPE_PATH = 0;
+    public const IMAGE_TYPE_URL = 1;
+    public const IMAGE_TYPE_NONE = -1;
src/form/FormAPI/CustomForm.php (1)

14-15: Add callable signature to property type annotation

The validationMethods property should specify the callable signature for better type safety.

-    /** @var array<callable> */
+    /** @var array<callable(mixed): bool> */
     private array $validationMethods = [];
🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 15-15:
Property pocketmine\form\FormAPI\CustomForm::$validationMethods type has no signature specified for callable.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 15-15:
Property pocketmine\form\FormAPI\CustomForm::$validationMethods type has no signature specified for callable.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 15-15:
Property pocketmine\form\FormAPI\CustomForm::$validationMethods type has no signature specified for callable.

🪛 PHPStan (2.1.15)

15-15: Property pocketmine\form\FormAPI\CustomForm::$validationMethods type has no signature specified for callable.

(missingType.callable)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7117dd3 and 69b3980.

⛔ Files ignored due to path filters (1)
  • composer.lock is excluded by !**/*.lock
📒 Files selected for processing (58)
  • .github/workflows/branch-sync-cron-trigger.yml (1 hunks)
  • .github/workflows/build-docker-image.yml (5 hunks)
  • .github/workflows/discord-release-notify.yml (1 hunks)
  • .github/workflows/draft-release-pr-check.yml (2 hunks)
  • .github/workflows/draft-release.yml (3 hunks)
  • .github/workflows/main-php-matrix.yml (1 hunks)
  • .github/workflows/main.yml (2 hunks)
  • .github/workflows/pr-remove-waiting-label.yml (1 hunks)
  • .github/workflows/team-pr-auto-approve.yml (1 hunks)
  • .php-cs-fixer.php (1 hunks)
  • build/dump-version-info.php (1 hunks)
  • changelogs/5.27.md (1 hunks)
  • changelogs/5.28.md (1 hunks)
  • composer.json (2 hunks)
  • phpstan.neon.dist (1 hunks)
  • src/VersionInfo.php (1 hunks)
  • src/block/tile/Spawnable.php (2 hunks)
  • src/data/bedrock/BedrockDataFiles.php (1 hunks)
  • src/data/bedrock/block/BlockStateData.php (1 hunks)
  • src/data/bedrock/block/BlockStateNames.php (1 hunks)
  • src/data/bedrock/block/BlockTypeNames.php (1 hunks)
  • src/data/bedrock/item/ItemTypeNames.php (13 hunks)
  • src/entity/Entity.php (3 hunks)
  • src/form/FormAPI/CustomForm.php (1 hunks)
  • src/form/FormAPI/Form.php (1 hunks)
  • src/form/FormAPI/IForm.php (1 hunks)
  • src/form/FormAPI/SimpleForm.php (1 hunks)
  • src/form/PMForm/BaseForm.php (1 hunks)
  • src/form/PMForm/CustomForm.php (1 hunks)
  • src/form/PMForm/CustomFormResponse.php (1 hunks)
  • src/form/PMForm/FormIcon.php (1 hunks)
  • src/form/PMForm/MenuForm.php (1 hunks)
  • src/form/PMForm/MenuOption.php (1 hunks)
  • src/form/PMForm/ModalForm.php (1 hunks)
  • src/form/PMForm/ServerSettingsForm.php (1 hunks)
  • src/form/PMForm/element/BaseSelector.php (1 hunks)
  • src/form/PMForm/element/CustomFormElement.php (1 hunks)
  • src/form/PMForm/element/Dropdown.php (1 hunks)
  • src/form/PMForm/element/Input.php (1 hunks)
  • src/form/PMForm/element/Label.php (1 hunks)
  • src/form/PMForm/element/Slider.php (1 hunks)
  • src/form/PMForm/element/StepSlider.php (1 hunks)
  • src/form/PMForm/element/Toggle.php (1 hunks)
  • src/inventory/BaseInventory.php (2 hunks)
  • src/item/enchantment/AvailableEnchantmentRegistry.php (3 hunks)
  • src/network/mcpe/cache/StaticPacketCache.php (2 hunks)
  • src/network/mcpe/handler/InGamePacketHandler.php (1 hunks)
  • src/player/Player.php (1 hunks)
  • src/scheduler/TaskScheduler.php (1 hunks)
  • src/thread/CommonThreadPartsTrait.php (1 hunks)
  • src/thread/ThreadCrashInfo.php (1 hunks)
  • src/utils/Utils.php (1 hunks)
  • src/world/World.php (1 hunks)
  • src/world/biome/model/BiomeDefinitionEntryData.php (1 hunks)
  • src/world/biome/model/ColorData.php (1 hunks)
  • src/world/format/io/data/BedrockWorldData.php (1 hunks)
  • tests/phpstan/rules/UnsafeForeachRule.php (3 hunks)
  • tools/generate-bedrock-data-from-packets.php (3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
src/form/PMForm/element/Label.php (6)
src/form/PMForm/element/CustomFormElement.php (4)
  • CustomFormElement (31-87)
  • getType (45-45)
  • validateValue (68-68)
  • serializeElementData (86-86)
src/form/PMForm/BaseForm.php (1)
  • getType (65-65)
src/form/PMForm/CustomForm.php (1)
  • getType (130-132)
src/form/PMForm/element/Input.php (3)
  • getType (47-49)
  • validateValue (51-55)
  • serializeElementData (71-76)
src/form/PMForm/element/Toggle.php (3)
  • getType (42-44)
  • validateValue (50-54)
  • serializeElementData (56-60)
src/form/PMForm/element/BaseSelector.php (1)
  • validateValue (52-59)
src/item/enchantment/AvailableEnchantmentRegistry.php (1)
src/utils/Utils.php (2)
  • Utils (105-702)
  • validateArrayValueType (589-597)
tests/phpstan/rules/UnsafeForeachRule.php (1)
src/utils/Utils.php (4)
  • Utils (105-702)
  • getNiceClosureName (124-147)
  • promoteKeys (623-625)
  • stringifyKeys (609-613)
src/form/PMForm/element/Toggle.php (1)
src/form/PMForm/element/CustomFormElement.php (5)
  • CustomFormElement (31-87)
  • __construct (37-40)
  • getType (45-45)
  • validateValue (68-68)
  • serializeElementData (86-86)
src/network/mcpe/cache/StaticPacketCache.php (3)
src/data/bedrock/BedrockDataFiles.php (1)
  • BedrockDataFiles (28-53)
src/utils/Utils.php (2)
  • Utils (105-702)
  • promoteKeys (623-625)
src/world/biome/model/BiomeDefinitionEntryData.php (1)
  • BiomeDefinitionEntryData (29-69)
src/form/PMForm/ModalForm.php (4)
src/utils/Utils.php (2)
  • Utils (105-702)
  • validateCallableSignature (575-582)
src/form/PMForm/BaseForm.php (4)
  • BaseForm (34-73)
  • __construct (39-41)
  • getType (65-65)
  • serializeFormData (71-71)
src/form/PMForm/MenuForm.php (4)
  • __construct (65-75)
  • handleResponse (81-94)
  • getType (96-98)
  • serializeFormData (100-105)
src/form/PMForm/CustomForm.php (4)
  • __construct (66-82)
  • handleResponse (99-128)
  • getType (130-132)
  • serializeFormData (134-138)
src/world/biome/model/BiomeDefinitionEntryData.php (1)
src/world/biome/model/ColorData.php (1)
  • ColorData (29-41)
src/network/mcpe/handler/InGamePacketHandler.php (1)
src/data/bedrock/block/BlockStateData.php (1)
  • equals (140-151)
tools/generate-bedrock-data-from-packets.php (2)
src/world/biome/model/BiomeDefinitionEntryData.php (1)
  • BiomeDefinitionEntryData (29-69)
src/world/biome/model/ColorData.php (1)
  • ColorData (29-41)
🪛 GitHub Actions: CI
src/form/PMForm/element/Dropdown.php

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/element/Label.php

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/element/StepSlider.php

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/element/Toggle.php

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/MenuOption.php

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/FormIcon.php

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/CustomFormResponse.php

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/ModalForm.php

[error] 17-93: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/element/BaseSelector.php

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/FormAPI/Form.php

[error] 23-72: PHP CS Fixer formatting issues: added use statement for InvalidArgumentException and adjusted indentation.

src/form/PMForm/element/Input.php

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/FormAPI/IForm.php

[error] 1-72: PHP CS Fixer formatting issues: added license header comment block and adjusted indentation.

src/form/PMForm/element/CustomFormElement.php

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/ServerSettingsForm.php

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/scheduler/TaskScheduler.php

[error] 1-213: PHP CS Fixer formatting issues detected. The file requires code style fixes as indicated by the diff output.

src/form/PMForm/BaseForm.php

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/element/Slider.php

[error] 49-100: PHP CS Fixer formatting issues: replaced 'or' with '||' and 'and' with '&&' for logical operators.

src/form/PMForm/CustomForm.php

[error] 17-136: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/FormAPI/SimpleForm.php

[error] 1-112: PHP CS Fixer formatting issues: added license header comment block and adjusted indentation.

src/form/PMForm/MenuForm.php

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/FormAPI/CustomForm.php

[error] 1-174: PHP CS Fixer formatting issues: added license header comment block and adjusted indentation.

🪛 LanguageTool
changelogs/5.28.md

[uncategorized] ~17-~17: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...es - AvailableEnchantmentRegistry now requires provided tags to always be string. Pr...

(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)

🪛 PHPStan (2.1.15)
src/form/PMForm/CustomFormResponse.php

46-46: Method pocketmine\form\PMForm\CustomFormResponse::getInt() should return int but returns mixed.

(return.type)


51-51: Method pocketmine\form\PMForm\CustomFormResponse::getString() should return string but returns mixed.

(return.type)


56-56: Method pocketmine\form\PMForm\CustomFormResponse::getFloat() should return float but returns mixed.

(return.type)


61-61: Method pocketmine\form\PMForm\CustomFormResponse::getBool() should return bool but returns mixed.

(return.type)

src/form/FormAPI/IForm.php

31-31: Parameter #1 $form of method pocketmine\player\Player::sendForm() expects pocketmine\form\Form, $this(pocketmine\form\FormAPI\IForm) given.

(argument.type)

src/scheduler/TaskScheduler.php

17-17: Property pocketmine\scheduler\TaskScheduler::$queue with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


23-23: Property pocketmine\scheduler\TaskScheduler::$tasks with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


28-28: Property pocketmine\scheduler\TaskScheduler::$playerTasks with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


38-38: Method pocketmine\scheduler\TaskScheduler::scheduleTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


45-45: Method pocketmine\scheduler\TaskScheduler::scheduleDelayedTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


52-52: Method pocketmine\scheduler\TaskScheduler::scheduleRepeatingTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


59-59: Method pocketmine\scheduler\TaskScheduler::scheduleDelayedRepeatingTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


71-71: Call to an undefined method pocketmine\utils\ReversePriorityQueue<int, pocketmine\scheduler\TaskHandler>::clear().

(method.notFound)


77-77: Method pocketmine\scheduler\TaskScheduler::isQueued() has parameter $task with generic class pocketmine\scheduler\TaskHandler but does not specify its types: TTask

(missingType.generics)


91-91: Method pocketmine\scheduler\TaskScheduler::scheduleForPlayer() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


119-119: Method pocketmine\scheduler\TaskScheduler::addTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


133-133: Method pocketmine\scheduler\TaskScheduler::handle() has parameter $handler with generic class pocketmine\scheduler\TaskHandler but does not specify its types: TTask

(missingType.generics)


133-133: Method pocketmine\scheduler\TaskScheduler::handle() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


169-169: PHPDoc tag @var for variable $task contains generic class pocketmine\scheduler\TaskHandler but does not specify its types: TTask

(missingType.generics)


193-193: Cannot call method getNextRun() on array<string, int|pocketmine\scheduler\TaskHandler>|int|pocketmine\scheduler\TaskHandler.

(method.nonObject)

src/form/PMForm/CustomForm.php

111-111: Possible unreported errors in foreach on array with unspecified key type.
PHPStan might not be reporting some type errors if the key type is not specified.
Declare a key type using @phpstan-var or @phpstan-param, or use pocketmine\utils\Utils::promoteKeys() to force PHPStan to report proper errors.

(pocketmine.foreach.implicitKeys)

src/form/FormAPI/SimpleForm.php

37-37: Instantiated class pocketmine\form\FormAPI\FormValidationException not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


37-37: Throwing object of an unknown class pocketmine\form\FormAPI\FormValidationException.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


40-40: Instantiated class pocketmine\form\FormAPI\FormValidationException not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


40-40: Throwing object of an unknown class pocketmine\form\FormAPI\FormValidationException.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


59-59: Method pocketmine\form\FormAPI\SimpleForm::getTitle() should return string but returns mixed.

(return.type)


66-66: Method pocketmine\form\FormAPI\SimpleForm::getContent() should return string but returns mixed.

(return.type)

src/form/FormAPI/CustomForm.php

15-15: Property pocketmine\form\FormAPI\CustomForm::$validationMethods type has no signature specified for callable.

(missingType.callable)


20-20: Method pocketmine\form\FormAPI\CustomForm::__construct() has parameter $callable with no value type specified in iterable type array.
See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type

(missingType.iterableValue)


21-21: Parameter #1 $callable of method pocketmine\form\FormAPI\IForm::__construct() expects (callable(pocketmine\player\Player, mixed): void)|null, (callable(array|null): void)|null given.

(argument.type)


36-36: Possible unreported errors in foreach on array with unspecified key type.
PHPStan might not be reporting some type errors if the key type is not specified.
Declare a key type using @phpstan-var or @phpstan-param, or use pocketmine\utils\Utils::promoteKeys() to force PHPStan to report proper errors.

(pocketmine.foreach.implicitKeys)


63-63: Method pocketmine\form\FormAPI\CustomForm::getTitle() should return string but returns mixed.

(return.type)


169-169: Cannot access an offset on mixed.

(offsetAccess.nonOffsetAccessible)

🪛 GitHub Check: PHP 8.3 / PHPStan analysis
src/form/FormAPI/IForm.php

[failure] 31-31:
Parameter #1 $form of method pocketmine\player\Player::sendForm() expects pocketmine\form\Form, $this(pocketmine\form\FormAPI\IForm) given.

src/form/FormAPI/SimpleForm.php

[failure] 40-40:
Instantiated class pocketmine\form\FormAPI\FormValidationException not found.


[failure] 37-37:
Throwing object of an unknown class pocketmine\form\FormAPI\FormValidationException.


[failure] 37-37:
Instantiated class pocketmine\form\FormAPI\FormValidationException not found.

src/form/FormAPI/CustomForm.php

[failure] 169-169:
Cannot access an offset on mixed.


[failure] 63-63:
Method pocketmine\form\FormAPI\CustomForm::getTitle() should return string but returns mixed.


[failure] 36-36:
Possible unreported errors in foreach on array with unspecified key type.


[failure] 21-21:
Parameter #1 $callable of method pocketmine\form\FormAPI\IForm::__construct() expects (callable(pocketmine\player\Player, mixed): void)|null, (callable(array|null): void)|null given.


[failure] 20-20:
Method pocketmine\form\FormAPI\CustomForm::__construct() has parameter $callable with no value type specified in iterable type array.


[failure] 15-15:
Property pocketmine\form\FormAPI\CustomForm::$validationMethods type has no signature specified for callable.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis
src/form/FormAPI/IForm.php

[failure] 31-31:
Parameter #1 $form of method pocketmine\player\Player::sendForm() expects pocketmine\form\Form, $this(pocketmine\form\FormAPI\IForm) given.

src/form/FormAPI/SimpleForm.php

[failure] 40-40:
Instantiated class pocketmine\form\FormAPI\FormValidationException not found.


[failure] 37-37:
Throwing object of an unknown class pocketmine\form\FormAPI\FormValidationException.


[failure] 37-37:
Instantiated class pocketmine\form\FormAPI\FormValidationException not found.

src/form/FormAPI/CustomForm.php

[failure] 169-169:
Cannot access an offset on mixed.


[failure] 63-63:
Method pocketmine\form\FormAPI\CustomForm::getTitle() should return string but returns mixed.


[failure] 36-36:
Possible unreported errors in foreach on array with unspecified key type.


[failure] 21-21:
Parameter #1 $callable of method pocketmine\form\FormAPI\IForm::__construct() expects (callable(pocketmine\player\Player, mixed): void)|null, (callable(array|null): void)|null given.


[failure] 20-20:
Method pocketmine\form\FormAPI\CustomForm::__construct() has parameter $callable with no value type specified in iterable type array.


[failure] 15-15:
Property pocketmine\form\FormAPI\CustomForm::$validationMethods type has no signature specified for callable.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis
src/form/FormAPI/IForm.php

[failure] 31-31:
Parameter #1 $form of method pocketmine\player\Player::sendForm() expects pocketmine\form\Form, $this(pocketmine\form\FormAPI\IForm) given.

src/form/FormAPI/SimpleForm.php

[failure] 40-40:
Instantiated class pocketmine\form\FormAPI\FormValidationException not found.


[failure] 37-37:
Throwing object of an unknown class pocketmine\form\FormAPI\FormValidationException.


[failure] 37-37:
Instantiated class pocketmine\form\FormAPI\FormValidationException not found.

src/form/FormAPI/CustomForm.php

[failure] 169-169:
Cannot access an offset on mixed.


[failure] 63-63:
Method pocketmine\form\FormAPI\CustomForm::getTitle() should return string but returns mixed.


[failure] 36-36:
Possible unreported errors in foreach on array with unspecified key type.


[failure] 21-21:
Parameter #1 $callable of method pocketmine\form\FormAPI\IForm::__construct() expects (callable(pocketmine\player\Player, mixed): void)|null, (callable(array|null): void)|null given.


[failure] 20-20:
Method pocketmine\form\FormAPI\CustomForm::__construct() has parameter $callable with no value type specified in iterable type array.


[failure] 15-15:
Property pocketmine\form\FormAPI\CustomForm::$validationMethods type has no signature specified for callable.

🔇 Additional comments (67)
src/thread/ThreadCrashInfo.php (1)

87-87:

Details

✅ Verification successful

Verify that $this->type is always guaranteed to be non-null in multithreaded contexts.

The removal of the null coalescing fallback assumes $this->type is never null. While the constructor ensures this during normal instantiation, in a ThreadSafe context, verify that serialization/deserialization or other threading operations cannot result in a null $this->type.


🏁 Script executed:

#!/bin/bash
# Description: Search for any usage patterns that might result in null type values
# Expected: No direct property assignments or serialization issues

# Search for direct assignments to the type property
rg -A 3 -B 3 '\$this->type\s*='

# Search for any serialization/unserialization related code in thread classes
rg -A 5 -B 5 'serialize|unserialize' --type php src/thread/

# Search for any nullable type declarations
rg 'type.*\?' --type php src/thread/

Length of output: 5069


ThreadCrashInfo::$type is always non-null, so the null‐coalescing fallback is safe

The constructor enforces string $type, and there are no reassignments or (un)serialization calls in src/thread/ThreadCrashInfo.php that could introduce a null value. Removing the ?? '' is therefore safe.

src/thread/CommonThreadPartsTrait.php (1)

97-107:

Details

❓ Verification inconclusive

Good fix for the race condition, but monitor for potential issues.

The synchronization logic properly addresses the timing issue where isTerminated=true is set before shutdown handlers complete. This ensures crash info is fully populated before being returned.

However, consider the following:

  • Calling join() on a crashed thread might have performance implications or could potentially cause deadlocks in complex scenarios
  • The TODO comment indicates this is a workaround for pmmpthread 6.1.1 behavior

🏁 Script executed:

#!/bin/bash
# Description: Check for other usages of join() and isTerminated() patterns to ensure consistency
# Expected: Similar patterns should follow the same synchronization approach

# Search for other join() usage patterns
rg -A 5 -B 5 '->join\(\)' --type php

# Search for isTerminated() checks without corresponding join() calls
rg -A 3 -B 3 'isTerminated\(\)' --type php | rg -v 'join\(\)'

# Look for potential deadlock scenarios in thread management
rg -A 5 -B 5 'synchronized.*join|join.*synchronized' --type php

Length of output: 3524


🏁 Script executed:

#!/bin/bash
# Searching for all join(), isJoined(), isTerminated(), and synchronized() usages across PHP files

grep -R --include='*.php' '->join(' -n src
grep -R --include='*.php' 'isJoined(' -n src
grep -R --include='*.php' 'isTerminated(' -n src
grep -R --include='*.php' 'synchronized' -n src

Length of output: 3758


Race condition fix approved; monitor join() usage for deadlocks

The new getCrashInfo() logic in src/thread/CommonThreadPartsTrait.php (lines 97–107) correctly ensures the shutdown handler runs before returning crash information.

Please verify and monitor the following:

  • Calling join() on a crashed thread may block indefinitely or impact performance under load.
  • Currently, join() is used only in:
    src/thread/CommonThreadPartsTrait.php (lines 97–107 & 135–140)
    src/utils/MainLogger.php (around line 218)
  • No other code paths use join() in combination with synchronized() locks—ensure this remains true as the code evolves.
  • Retain the TODO workaround for pmmpthread 6.1.1 and plan to remove it once the library runs shutdown handlers before marking termination.
.github/workflows/discord-release-notify.yml (1)

16-16: Upgrade PHP setup action to v2.33.0
Consistent with other workflows; version bump looks safe and contains the latest patches.

.github/workflows/team-pr-auto-approve.yml (1)

25-25: Bump actions/create-github-app-token to v2
Inputs and behavior remain unchanged; this standardizes the token generation step across workflows.

.github/workflows/build-docker-image.yml (2)

11-11: Switch runner to Ubuntu 22.04
Aligns the Docker build workflow with the upgraded CI environment.


56-56: Update docker/build-push-action to v6.18.0
Consistent version bump across all build steps; no breaking changes noted in the minor release.

Also applies to: 69-69, 82-82, 95-95

.github/workflows/main-php-matrix.yml (1)

18-18: Set default runner image to Ubuntu 22.04
Standardizes the execution environment for all PHP matrix jobs.

.github/workflows/draft-release-pr-check.yml (2)

27-27: Upgrade runner to Ubuntu 22.04 for both jobs
Keeps release validation aligned with the latest LTS environment.

Also applies to: 46-46


52-52: Bump shivammathur/setup-php to v2.33.0
Ensures the workflow uses the latest PHP setup action across release checks.

.github/workflows/draft-release.yml (3)

26-26: Standardize runners to Ubuntu 22.04
All three jobs now run on Ubuntu 22.04, which aligns this workflow with the rest of the CI/CD pipelines. Please verify that any OS-specific dependencies, caches, or build scripts continue to work as expected on 22.04.

Also applies to: 57-57, 82-82


62-62: Upgrade actions/create-github-app-token to v2
Switching to actions/create-github-app-token@v2 brings improved token rotation without altering the input schema (app-id, private-key, etc.). No breaking changes here.


90-90: Bump shivammathur/setup-php to v2.33.0
This matches the version bump in other workflows. Ensure that v2.33.0 fully supports the PHP 8.2 environment defined by PHP_VERSION.

.github/workflows/main.yml (2)

23-23: Switch CI jobs to Ubuntu 22.04
Both the codestyle and shellcheck jobs are now on Ubuntu 22.04 for consistency. Confirm that caching layers, shellcheck versions, and any OS-specific checks remain functional.

Also applies to: 43-43


31-34: Bump PHP setup action and tools
Upgraded to shivammathur/setup-php@2.33.0 with PHP 8.3 and PHP-CS-Fixer 3.75. Please validate that the new fixer version doesn’t introduce unexpected formatting changes or deprecate any CLI flags in your code style rules.

.github/workflows/pr-remove-waiting-label.yml (1)

18-33: Encapsulate label removal with error handling
The new removeLabel function cleanly abstracts the API call and handles 404s gracefully, avoiding noisy failures when the label is already absent.

.github/workflows/branch-sync-cron-trigger.yml (3)

15-17: Run on Ubuntu 22.04
Aligns with the rest of the repository’s workflows. No additional changes needed.


18-25: Generate GitHub App token with v2
Using actions/create-github-app-token@v2 matches other workflows. Ensure the GitHub App associated with RESTRICTED_ACTIONS_DISPATCH_ID has the necessary repo-dispatch permissions on RestrictedActions.


27-32: Dispatch pocketmine_mp_branch_sync event
The peter-evans/repository-dispatch@v3 step is correct. Confirm that the target repo’s workflow is listening for the pocketmine_mp_branch_sync event-type.

changelogs/5.27.md (1)

19-25: Changelog entry for 5.27.1 looks good.

The new patch release is clearly documented with the correct version heading, release date, and fix description.

build/dump-version-info.php (1)

34-35: PHPDoc type annotations simplified correctly.

Replacing fully-qualified \Closure with unqualified Closure aligns with global namespace usage, and the @phpstan-var annotation accurately describes the array shape. No functional impact.

src/block/tile/Spawnable.php (2)

35-35: PHPStan annotation updated correctly for $spawnCompoundCache.

Using the imported CompoundTag with the generic CacheableNbt<CompoundTag>|null is accurate and maintains clarity.


76-76: Return type annotation refined for getSerializedSpawnCompound().

The @phpstan-return CacheableNbt<CompoundTag> matches the actual return type and aligns with the previous change, improving consistency.

src/world/World.php (1)

363-364: Shortened PHPDoc class-string annotation
The updated PHPDoc for $generator now uses a relative namespace (generator\Generator), which correctly resolves to pocketmine\world\generator\Generator under the current namespace. This matches the project's streamlined annotation style.

src/entity/Entity.php (2)

63-63: LGTM: Imports added for new functionality.

The new imports Limits and min function are necessary for the fire ticks clamping implementation.

Also applies to: 80-80


705-713: Excellent improvement: Graceful handling of large fire tick values.

This change improves compatibility by clamping large fire tick values to Limits::INT16_MAX instead of throwing exceptions. The extensive comments clearly explain the rationale - many plugins use large values without being aware of Mojang save format limitations. This graceful degradation approach prevents crashes while maintaining functionality.

The implementation correctly:

  • Maintains validation for negative values
  • Uses min() to clamp values to the maximum acceptable range
  • Preserves existing behavior for valid ranges
src/network/mcpe/handler/InGamePacketHandler.php (1)

214-214: Improved BitSet comparison logic.

The change from strict inequality (!==) to using the equals() method is correct for BitSet comparison. This ensures proper value-based equality checking rather than reference comparison, which aligns with the BitSet class design pattern shown in the relevant code snippets.

phpstan.neon.dist (1)

18-18: LGTM: PHPStan rule updated to enhanced version.

The rule change from UnsafeForeachArrayOfStringRule to UnsafeForeachRule represents an improvement in static analysis capabilities with better error reporting for unsafe foreach usage patterns.

src/data/bedrock/block/BlockTypeNames.php (1)

395-395: Confirm new block type constant.
The DRIED_GHAST constant follows alphabetical ordering and matches the expected Minecraft identifier. No issues detected.

src/data/bedrock/block/BlockStateNames.php (1)

116-116: Confirm new block state constant.
The REHYDRATION_LEVEL constant is correctly inserted in alphabetical order and matches the expected property name.

src/data/bedrock/block/BlockStateData.php (1)

48-49: Verify blockstate CURRENT_VERSION bump.
The patch and revision bits were updated to 60 and 33; please confirm these values match the BedrockBlockUpgradeSchema for version 1.21.80.

.php-cs-fixer.php (1)

9-13: Verify PHP CS Fixer exclusions for JSON directories.
The new notPath entries prevent JSON/model directories from being modified by PHP CS Fixer. Ensure all JsonMapper-managed directories (e.g., additional JSON or model paths) are included in these exclusions.

src/utils/Utils.php (1)

587-587: LGTM! Type annotation improvement aligns with actual usage.

The change from void to mixed for the validator closure return type is sensible since the method ignores the return value anyway and this provides more flexibility for validator implementations.

src/item/enchantment/AvailableEnchantmentRegistry.php (1)

133-133: Excellent type safety addition!

Adding validation to ensure tag arrays contain only strings is a great improvement that will prevent crashes from invalid tag types. The validator closure will throw a TypeError with clear error messages for any non-string values.

Also applies to: 157-157

src/world/biome/model/ColorData.php (1)

29-41: Well-designed data model class.

The ColorData class is appropriately designed with:

  • Final class modifier preventing inheritance
  • Properly typed public properties for RGBA color channels
  • @required annotations for JSON deserialization requirements
  • Clear, focused responsibility for color data modeling
src/inventory/BaseInventory.php (1)

259-259: Good optimization with early returns.

Replacing the break statements with early returns when $newItem->getCount() <= 0 is a smart performance improvement. Once the item is fully added, there's no need to continue processing, making the control flow more efficient and clearer.

Also applies to: 273-273

src/world/format/io/data/BedrockWorldData.php (2)

58-58: Patch version update is consistent with the network version change.

The patch version update from 70 to 80 correctly reflects support for Minecraft Bedrock Edition 1.21.80.


54-54:

Details

✅ Verification successful

Version constant update looks correct.

The network version bump from 786 to 800 aligns with Minecraft Bedrock Edition 1.21.80 support as indicated in the PR summary.

Verify that all code handling these version constants has been updated accordingly:


🏁 Script executed:

#!/bin/bash
# Description: Check for any hardcoded references to the old version numbers

# Search for potential hardcoded references to the old network version
rg "786" --type php -A 2 -B 2

# Search for references to the old patch version 70 in context of Bedrock
rg "1\.21\.70" --type php -A 2 -B 2

Length of output: 67


🏁 Script executed:

#!/bin/bash
# Verify usage of the updated version constants across the codebase

# Search for all references to the storage network version constant
rg "CURRENT_STORAGE_NETWORK_VERSION" --type php -A 2 -B 2

# Search for all references to the client version target constant
rg "CURRENT_CLIENT_VERSION_TARGET" --type php -A 2 -B 2

Length of output: 3767


Version constants updated and verified

The bump from 786→800 and client target to 1.21.80 is applied consistently in src/world/format/io/data/BedrockWorldData.php. All usages of CURRENT_STORAGE_NETWORK_VERSION and CURRENT_CLIENT_VERSION_TARGET have been updated, and no hardcoded references to the old versions remain.

Approving these changes.

composer.json (4)

37-39: Dependency versions correctly updated for Bedrock 1.21.80 support.

The bedrock-data and bedrock-protocol package versions are properly aligned with the new Minecraft version support.


48-48: Raklib version update looks appropriate.

The raklib update to 1.1.2 likely includes fixes or improvements needed for the new Bedrock version support.


55-55: PHPStan upgrade improves static analysis capabilities.

Upgrading from 2.1.8 to 2.1.16 brings the latest static analysis improvements and bug fixes.


60-63: New replace section helps prevent dependency conflicts.

The replace section indicates this package provides its own implementations of symfony polyfills, which can help reduce dependency conflicts and bundle size.

src/data/bedrock/BedrockDataFiles.php (1)

34-34:

Details

✅ Verification successful

New JSON constant for biome definitions migration.

The addition of BIOME_DEFINITIONS_JSON supports the migration from NBT to JSON format for biome definitions data.

Verify that all references to the removed NBT constants have been updated to use the new JSON constant:


🏁 Script executed:

#!/bin/bash
# Description: Check for any remaining references to old NBT biome definition constants

# Search for references to the old NBT constants that were removed
rg "BIOME_DEFINITIONS_NBT|BIOME_DEFINITIONS_FULL_NBT" --type php -A 2 -B 2

# Verify the new JSON constant is being used
rg "BIOME_DEFINITIONS_JSON" --type php -A 2 -B 2

Length of output: 1184


Migration verified: all NBT references removed, JSON constant in use

No remaining references to BIOME_DEFINITIONS_NBT or BIOME_DEFINITIONS_FULL_NBT, and BIOME_DEFINITIONS_JSON is correctly consumed in:

  • src/network/mcpe/cache/StaticPacketCache.php (used in BiomeDefinitionListPacket::fromDefinitions)

All related formatting changes are complete—no further action required.

src/form/PMForm/element/Dropdown.php (2)

26-37: Well-structured dropdown form element implementation.

The class correctly extends BaseSelector and implements the required methods. The getType() method returns the appropriate "dropdown" identifier, and serializeElementData() properly structures the options and default index for form serialization.


17-17: ⚠️ Potential issue

Fix PHP CS Fixer formatting issue.

The pipeline failure indicates a missing space before the closing comment block.

Apply this diff to fix the formatting issue:

- * @link http://www.pocketmine.net/
+ * @link http://www.pocketmine.net/

Likely an incorrect or invalid review comment.

🧰 Tools
🪛 GitHub Actions: CI

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/player/Player.php (1)

1644-1647: Excellent addition of damage validation for durable items.

This validation ensures that damage values are within valid bounds (0 to maximum durability) before setting them on item clones, preventing potential issues with invalid damage states.

src/form/PMForm/element/Label.php (1)

31-44: Well-structured Label element implementation.

The overall design is sound - Label elements are correctly implemented as display-only components with no input value or serialization data.

src/form/PMForm/element/StepSlider.php (1)

26-38: Clean and well-designed StepSlider implementation.

The class appropriately extends BaseSelector to leverage existing validation logic while providing step slider-specific serialization. The type "step_slider" and the serialization format with "steps" and "default" fields align with expected Minecraft form standards.

src/form/PMForm/element/Toggle.php (1)

33-61: Excellent Toggle element implementation.

The class is well-structured with:

  • Proper constructor pattern calling parent and setting default value
  • Robust type validation with clear error messages
  • Clean serialization including only necessary data
  • Appropriate use of boolean type checking

The implementation follows the established patterns from other form elements and integrates well with the CustomFormElement base class.

src/form/PMForm/MenuOption.php (1)

17-17: ⚠️ Potential issue

Fix the comment block formatting.

The pipeline failure indicates a missing space before the closing comment block.

Apply this diff to fix the formatting:

- * @link http://www.pocketmine.net/
+ * @link http://www.pocketmine.net/ 

Likely an incorrect or invalid review comment.

🧰 Tools
🪛 GitHub Actions: CI

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

tests/phpstan/rules/UnsafeForeachRule.php (1)

44-115: Excellent improvements to the foreach safety rule!

The refactored rule now provides:

  • More comprehensive detection of unsafe foreach patterns
  • Clear, actionable error messages with specific solutions
  • Proper handling of both implicit key types and numeric string coercion issues
  • Well-structured error accumulation instead of single error returns

The references to Utils::promoteKeys() and Utils::stringifyKeys() provide users with immediate solutions to fix the detected issues.

src/network/mcpe/cache/StaticPacketCache.php (1)

107-107: Correct transition from NBT to JSON biome loading.

The change properly uses the new JSON loader and the factory method for creating the packet.

tools/generate-bedrock-data-from-packets.php (1)

574-607: Excellent implementation of JSON biome definition generation.

The method properly:

  • Rounds float values to 3 decimal places to avoid precision issues
  • Maps data to strongly-typed model objects
  • Ensures consistent JSON output with sorting
  • Uses pretty-printing for readability

The structured approach with model classes improves maintainability compared to the previous NBT approach.

src/form/PMForm/element/Input.php (2)

51-76: Well-implemented validation and serialization logic.

The validation correctly ensures string input with clear error messages, and the serialization properly maps internal properties to the expected form structure.


17-17: ⚠️ Potential issue

Fix PHP CS Fixer formatting issue.

Add a space before the closing comment block as required by the coding standards.

- * @link http://www.pocketmine.net/
+ * @link http://www.pocketmine.net/ 

Likely an incorrect or invalid review comment.

🧰 Tools
🪛 GitHub Actions: CI

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

src/form/PMForm/ServerSettingsForm.php (1)

33-59: Well-designed server settings form implementation.

The class correctly extends CustomForm and provides specialized functionality for server settings with optional icon support. The implementation properly handles icon serialization and follows the established form API patterns.

src/world/biome/model/BiomeDefinitionEntryData.php (1)

29-69: Excellent data model implementation.

The class provides a clean, strongly-typed data model for biome definitions with proper JSON deserialization support. All properties are appropriately typed and marked as required, and the integration with ColorData follows good composition practices.

changelogs/5.28.md (2)

17-17: Grammar is correct - ignore static analysis false positive.

The static analysis tool flagged this line for verb agreement, but "requires provided tags to always be string" is grammatically correct in this technical context.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~17-~17: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...es - AvailableEnchantmentRegistry now requires provided tags to always be string. Pr...

(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)


1-35: Comprehensive and well-structured changelog.

The changelog provides excellent documentation of version changes, compatibility notes, and fixes. The warnings about plugin compatibility and API constraints are particularly valuable for developers.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~17-~17: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...es - AvailableEnchantmentRegistry now requires provided tags to always be string. Pr...

(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)

src/form/FormAPI/IForm.php (1)

1-73:

Details

❓ Verification inconclusive

Address PHP CS Fixer formatting issues.

The pipeline indicates formatting issues including missing license header and indentation problems.

Run PHP CS Fixer to identify and resolve formatting issues:


🏁 Script executed:

#!/bin/bash
# Description: Run PHP CS Fixer to fix formatting issues
vendor/bin/php-cs-fixer fix src/form/FormAPI/IForm.php --diff --dry-run

Length of output: 195


Ensure PHP CS Fixer is installed and run formatting fixes

The CI pipeline reports missing license header and indentation issues in src/form/FormAPI/IForm.php, but the vendor/bin/php-cs-fixer binary wasn’t found. Please verify your setup and run PHP CS Fixer manually:

  • Confirm that PHP CS Fixer is installed (e.g., via Composer).
  • Execute the fixer against the file:
    php-cs-fixer fix src/form/FormAPI/IForm.php --diff --dry-run
  • Address any reported issues (license header, indentation, etc.) and commit the changes.
🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 31-31:
Parameter #1 $form of method pocketmine\player\Player::sendForm() expects pocketmine\form\Form, $this(pocketmine\form\FormAPI\IForm) given.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 31-31:
Parameter #1 $form of method pocketmine\player\Player::sendForm() expects pocketmine\form\Form, $this(pocketmine\form\FormAPI\IForm) given.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 31-31:
Parameter #1 $form of method pocketmine\player\Player::sendForm() expects pocketmine\form\Form, $this(pocketmine\form\FormAPI\IForm) given.

🪛 PHPStan (2.1.15)

31-31: Parameter #1 $form of method pocketmine\player\Player::sendForm() expects pocketmine\form\Form, $this(pocketmine\form\FormAPI\IForm) given.

(argument.type)

🪛 GitHub Actions: CI

[error] 1-72: PHP CS Fixer formatting issues: added license header comment block and adjusted indentation.

src/form/PMForm/element/BaseSelector.php (2)

42-50: Well-implemented constructor with proper validation.

The constructor correctly validates the default option index and uses array_values() to ensure sequential indexing, which is good practice for array-based options.


52-59: Robust validation method with clear error messages.

The validation logic properly checks both type and bounds, providing informative error messages.

src/form/PMForm/BaseForm.php (2)

54-60: Excellent use of the template method pattern.

The jsonSerialize() method properly delegates form-specific serialization to subclasses while maintaining consistent structure. The final modifier prevents overriding, ensuring consistent serialization format.


65-71: Well-defined abstract methods for extensibility.

The abstract methods clearly define the contract for subclasses with proper documentation and return type declarations.

src/form/FormAPI/Form.php (1)

43-46: Proper deprecation handling.

The deprecated method is correctly annotated with @deprecated and provides clear guidance to use Player::sendForm() instead.

src/scheduler/TaskScheduler.php (1)

124-125: Good simplification of delay and period handling.

The use of max() to ensure minimum values and the simplified next run calculation improve code readability.

Also applies to: 134-134

src/form/PMForm/MenuForm.php (1)

81-94: Well-implemented response handling with proper validation.

The method correctly handles null responses, validates integer data, checks option bounds, and provides clear error messages.

src/data/bedrock/item/ItemTypeNames.php (1)

71-602: New item type constants look good

The additions for harness items and happy ghast spawn egg are properly alphabetized and follow the existing naming conventions.

Comment on lines +37 to +39
public function validateValue($value) : void{
assert($value === null);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Replace assert() with proper exception handling.

Using assert() for validation in production code is not recommended. Consider throwing a proper exception instead.

 public function validateValue($value) : void{
-	assert($value === null);
+	if($value !== null){
+		throw new FormValidationException("Label elements should not have values, got " . gettype($value));
+	}
 }
🤖 Prompt for AI Agents
In src/form/PMForm/element/Label.php around lines 37 to 39, replace the use of
assert() for validating $value with proper exception handling. Instead of
assert($value === null), check if $value is not null and throw a relevant
exception to indicate invalid input. This ensures validation errors are handled
explicitly and appropriately in production code.

Comment on lines +53 to +64
#[\ReturnTypeWillChange]
public function jsonSerialize(){
$json = [
"text" => $this->text
];

if($this->hasImage()){
$json["image"] = $this->image;
}

return $json;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Consider adding a return type declaration.

The #[\ReturnTypeWillChange] attribute is typically used for PHP 8.1 compatibility. If the codebase targets PHP 8.1+, you can add the proper return type instead.

If PHP 8.1+ is required, consider:

-	#[\ReturnTypeWillChange]
-	public function jsonSerialize(){
+	public function jsonSerialize() : array{

🏁 Script executed:

#!/bin/bash
# Check PHP version requirements in composer.json
fd -e json -x grep -l "composer" | xargs jq -r '.require.php // empty' 2>/dev/null | grep -v '^$' | head -5

Length of output: 100


Add explicit return type and remove obsolete attribute

Since your composer.json requires PHP ^8.1, you can drop #[\ReturnTypeWillChange] and declare the return type directly:

-   #[\ReturnTypeWillChange]
-   public function jsonSerialize(){
+   public function jsonSerialize(): array {
        $json = [
            "text" => $this->text
        ];

        if ($this->hasImage()) {
            $json["image"] = $this->image;
        }

        return $json;
    }

This removes the compatibility attribute and enforces the correct array return type.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[\ReturnTypeWillChange]
public function jsonSerialize(){
$json = [
"text" => $this->text
];
if($this->hasImage()){
$json["image"] = $this->image;
}
return $json;
}
public function jsonSerialize(): array {
$json = [
"text" => $this->text
];
if($this->hasImage()){
$json["image"] = $this->image;
}
return $json;
}
🤖 Prompt for AI Agents
In src/form/PMForm/MenuOption.php around lines 53 to 64, the jsonSerialize
method uses the #[\ReturnTypeWillChange] attribute for PHP 8.1 compatibility but
your composer.json requires PHP ^8.1. Remove the #[\ReturnTypeWillChange]
attribute and add an explicit return type declaration of : array to the
jsonSerialize method to enforce the correct return type and clean up obsolete
compatibility code.

* (at your option) any later version.
*
* @author PocketMine Team
* @link http://www.pocketmine.net/
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Address the PHP CS Fixer formatting issue.

The pipeline indicates a formatting issue on this line regarding missing space before closing comment block, though the current code appears correctly formatted.

Please run PHP CS Fixer to identify and resolve the specific formatting issue:


🏁 Script executed:

#!/bin/bash
# Description: Run PHP CS Fixer to identify formatting issues
vendor/bin/php-cs-fixer fix src/form/PMForm/ServerSettingsForm.php --diff --dry-run

Length of output: 207


🏁 Script executed:

#!/bin/bash
# Display the top 30 lines of the form to inspect the docblock around line 17
sed -n '1,30p' src/form/PMForm/ServerSettingsForm.php

Length of output: 1177


Fix missing space before closing comment block

The file-level license header ends with */ without the required leading space. Update the closing of the comment in src/form/PMForm/ServerSettingsForm.php so it reads:

  *
-*/
+ */

Then run PHP CS Fixer locally to confirm the “missing space before closing comment block” warning is resolved:

vendor/bin/php-cs-fixer fix src/form/PMForm/ServerSettingsForm.php --diff --dry-run
🧰 Tools
🪛 GitHub Actions: CI

[error] 17-17: PHP CS Fixer formatting issue: missing space before closing comment block.

🤖 Prompt for AI Agents
In src/form/PMForm/ServerSettingsForm.php at line 17, the closing of the
file-level license header comment block lacks a required leading space before
the closing '*/'. Edit the comment block to ensure there is a space before the
'*/' at the end of the header. After making this change, run PHP CS Fixer on the
file to verify that the missing space before closing comment block warning is
resolved.

Comment on lines +30 to +32
public function sendToPlayer(Player $player) : void {
$player->sendForm($this);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Fix type mismatch in deprecated sendToPlayer method.

PHPStan reports that Player::sendForm() expects pocketmine\form\Form but receives $this(pocketmine\form\FormAPI\IForm). This suggests a type incompatibility issue.

Verify the inheritance hierarchy and interface implementation:


🏁 Script executed:

#!/bin/bash
# Description: Check the Form interface and its relationship with IForm
ast-grep --pattern 'interface Form {
  $$$
}'

# Check if IForm properly implements the expected interface
ast-grep --pattern 'class IForm implements $_ {
  $$$
}'

# Check Player::sendForm method signature
ast-grep --pattern 'function sendForm($_) {
  $$$
}'

Length of output: 161


🏁 Script executed:

#!/bin/bash
# Search for the Form interface definition
rg -n "interface Form" -t php

# Search for IForm declaration (could be interface or class)
rg -n "interface IForm" -t php
rg -n "class IForm" -t php

# Search for Player::sendForm signature
rg -n "function sendForm" -t php

Length of output: 403


Fix Form interface mismatch in IForm

IForm is currently implementing the Form interface from FormAPI, but Player::sendForm() only accepts the base pocketmine\form\Form interface. Update IForm to implement the correct Form and alias it to avoid namespace collisions.

Files to update:

  • src/form/FormAPI/IForm.php

Suggested diff:

--- a/src/form/FormAPI/IForm.php
+++ b/src/form/FormAPI/IForm.php
@@ 1,7c1,8
 namespace pocketmine\form\FormAPI;

+use pocketmine\form\Form as BaseForm;
 use pocketmine\player\Player;

-abstract class IForm implements Form {
+abstract class IForm implements BaseForm {
     /**
      * @deprecated Use Player::sendForm() directly with your Form implementation.
      */
     public function sendToPlayer(Player $player) : void {
         $player->sendForm($this);
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function sendToPlayer(Player $player) : void {
$player->sendForm($this);
}
namespace pocketmine\form\FormAPI;
use pocketmine\form\Form as BaseForm;
use pocketmine\player\Player;
abstract class IForm implements BaseForm {
/**
* @deprecated Use Player::sendForm() directly with your Form implementation.
*/
public function sendToPlayer(Player $player) : void {
$player->sendForm($this);
}
}
🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 31-31:
Parameter #1 $form of method pocketmine\player\Player::sendForm() expects pocketmine\form\Form, $this(pocketmine\form\FormAPI\IForm) given.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 31-31:
Parameter #1 $form of method pocketmine\player\Player::sendForm() expects pocketmine\form\Form, $this(pocketmine\form\FormAPI\IForm) given.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 31-31:
Parameter #1 $form of method pocketmine\player\Player::sendForm() expects pocketmine\form\Form, $this(pocketmine\form\FormAPI\IForm) given.

🪛 PHPStan (2.1.15)

31-31: Parameter #1 $form of method pocketmine\player\Player::sendForm() expects pocketmine\form\Form, $this(pocketmine\form\FormAPI\IForm) given.

(argument.type)

🤖 Prompt for AI Agents
In src/form/FormAPI/IForm.php around lines 30 to 32, the sendToPlayer method
passes $this to Player::sendForm(), but $this implements
pocketmine\form\FormAPI\IForm, which is incompatible with the expected
pocketmine\form\Form interface. To fix this, update the IForm interface to
implement or extend the base pocketmine\form\Form interface instead of the
FormAPI Form interface, and use aliasing if needed to avoid namespace conflicts.
This ensures type compatibility when passing $this to sendForm.

Comment on lines +54 to +57
public function getFloat(string $name) : float{
$this->checkExists($name);
return $this->data[$name];
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add runtime type validation for float values.

Type safety issue needs to be addressed.

 public function getFloat(string $name) : float{
 	$this->checkExists($name);
-	return $this->data[$name];
+	$value = $this->data[$name];
+	if(!is_float($value) && !is_int($value)){
+		throw new \InvalidArgumentException("Value \"$name\" is not a numeric value");
+	}
+	return (float)$value;
 }
🧰 Tools
🪛 PHPStan (2.1.15)

56-56: Method pocketmine\form\PMForm\CustomFormResponse::getFloat() should return float but returns mixed.

(return.type)

🤖 Prompt for AI Agents
In src/form/PMForm/CustomFormResponse.php around lines 54 to 57, the getFloat
method returns a value without verifying it is actually a float, causing a type
safety issue. Add a runtime check to confirm that $this->data[$name] is a float
before returning it. If the value is not a float, throw an appropriate exception
or handle the error to ensure type safety.

Comment on lines +58 to +67
public function getTitle() : string {
return $this->data["title"];
}

/**
* @return string
*/
public function getContent() : string {
return $this->data["content"];
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix type safety issues in getter methods

The getter methods return values from the $data array which has mixed type, but the methods promise to return strings.

Add type assertions to ensure type safety:

     public function getTitle() : string {
-        return $this->data["title"];
+        return (string) $this->data["title"];
     }
 
     /**
      * @return string
      */
     public function getContent() : string {
-        return $this->data["content"];
+        return (string) $this->data["content"];
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function getTitle() : string {
return $this->data["title"];
}
/**
* @return string
*/
public function getContent() : string {
return $this->data["content"];
}
public function getTitle() : string {
return (string) $this->data["title"];
}
/**
* @return string
*/
public function getContent() : string {
return (string) $this->data["content"];
}
🧰 Tools
🪛 PHPStan (2.1.15)

59-59: Method pocketmine\form\FormAPI\SimpleForm::getTitle() should return string but returns mixed.

(return.type)


66-66: Method pocketmine\form\FormAPI\SimpleForm::getContent() should return string but returns mixed.

(return.type)

🤖 Prompt for AI Agents
In src/form/FormAPI/SimpleForm.php around lines 58 to 67, the getter methods
getTitle() and getContent() promise to return strings but directly return values
from the mixed-type $data array, risking type safety. To fix this, add explicit
type assertions or checks to ensure the returned values are strings before
returning them, such as casting or validating the type and handling unexpected
types appropriately.

Comment on lines +1 to +9
<?php

declare(strict_types = 1);

namespace pocketmine\form\FormAPI;

use pocketmine\player\Player;

class SimpleForm extends IForm {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing import for FormValidationException

The class uses FormValidationException but doesn't import it. This will cause a fatal error at runtime.

Add the missing import:

 namespace pocketmine\form\FormAPI;
 
+use pocketmine\form\FormValidationException;
 use pocketmine\player\Player;
🧰 Tools
🪛 GitHub Actions: CI

[error] 1-112: PHP CS Fixer formatting issues: added license header comment block and adjusted indentation.

🤖 Prompt for AI Agents
In src/form/FormAPI/SimpleForm.php at the beginning of the file (lines 1 to 9),
the class uses FormValidationException but does not import it, which will cause
a fatal error. Add a use statement to import FormValidationException from its
correct namespace to ensure the class can reference it without errors.

Comment on lines +62 to +64
public function getTitle() : string {
return $this->data["title"];
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix type safety issue in getTitle method

The method returns a value from the $data array which has mixed type, but promises to return string.

     public function getTitle() : string {
-        return $this->data["title"];
+        return (string) $this->data["title"];
     }
🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 63-63:
Method pocketmine\form\FormAPI\CustomForm::getTitle() should return string but returns mixed.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 63-63:
Method pocketmine\form\FormAPI\CustomForm::getTitle() should return string but returns mixed.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 63-63:
Method pocketmine\form\FormAPI\CustomForm::getTitle() should return string but returns mixed.

🪛 PHPStan (2.1.15)

63-63: Method pocketmine\form\FormAPI\CustomForm::getTitle() should return string but returns mixed.

(return.type)

🤖 Prompt for AI Agents
In src/form/FormAPI/CustomForm.php around lines 62 to 64, the getTitle method
promises to return a string but directly returns a value from the $data array
which may have mixed types. To fix this type safety issue, ensure the returned
value is explicitly cast or validated as a string before returning. You can do
this by checking if the value exists and is a string, or by casting it to string
to guarantee the method's return type.

Comment on lines +168 to +171
private function addContent(array $content) : self {
$this->data["content"][] = $content;
return $this;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Ensure safe array access in addContent method

The method accesses $this->data["content"] which might not be an array.

Add a type check or ensure the array is properly initialized:

     private function addContent(array $content) : self {
+        if (!isset($this->data["content"]) || !is_array($this->data["content"])) {
+            $this->data["content"] = [];
+        }
         $this->data["content"][] = $content;
         return $this;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private function addContent(array $content) : self {
$this->data["content"][] = $content;
return $this;
}
private function addContent(array $content) : self {
if (!isset($this->data["content"]) || !is_array($this->data["content"])) {
$this->data["content"] = [];
}
$this->data["content"][] = $content;
return $this;
}
🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 169-169:
Cannot access an offset on mixed.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 169-169:
Cannot access an offset on mixed.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 169-169:
Cannot access an offset on mixed.

🪛 PHPStan (2.1.15)

169-169: Cannot access an offset on mixed.

(offsetAccess.nonOffsetAccessible)

🤖 Prompt for AI Agents
In src/form/FormAPI/CustomForm.php around lines 168 to 171, the addContent
method assumes $this->data["content"] is an array but it might not be
initialized or could be another type. To fix this, add a check to verify if
$this->data["content"] is an array before appending; if not, initialize it as an
empty array first. This ensures safe array access and prevents runtime errors.

Comment on lines +18 to +21
* @param callable(?array): void|null $callable
*/
public function __construct(?callable $callable) {
parent::__construct($callable);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix constructor parameter type to match parent class

The callable parameter type doesn't match the parent class signature, which violates the Liskov Substitution Principle.

The parent expects callable(Player, mixed): void but this constructor expects callable(?array): void. Consider updating the parameter documentation to match:

     /**
-     * @param callable(?array): void|null $callable
+     * @param callable(Player, mixed): void|null $callable
      */
     public function __construct(?callable $callable) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* @param callable(?array): void|null $callable
*/
public function __construct(?callable $callable) {
parent::__construct($callable);
/**
* @param callable(Player, mixed): void|null $callable
*/
public function __construct(?callable $callable) {
parent::__construct($callable);
🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 21-21:
Parameter #1 $callable of method pocketmine\form\FormAPI\IForm::__construct() expects (callable(pocketmine\player\Player, mixed): void)|null, (callable(array|null): void)|null given.


[failure] 20-20:
Method pocketmine\form\FormAPI\CustomForm::__construct() has parameter $callable with no value type specified in iterable type array.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 21-21:
Parameter #1 $callable of method pocketmine\form\FormAPI\IForm::__construct() expects (callable(pocketmine\player\Player, mixed): void)|null, (callable(array|null): void)|null given.


[failure] 20-20:
Method pocketmine\form\FormAPI\CustomForm::__construct() has parameter $callable with no value type specified in iterable type array.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 21-21:
Parameter #1 $callable of method pocketmine\form\FormAPI\IForm::__construct() expects (callable(pocketmine\player\Player, mixed): void)|null, (callable(array|null): void)|null given.


[failure] 20-20:
Method pocketmine\form\FormAPI\CustomForm::__construct() has parameter $callable with no value type specified in iterable type array.

🪛 PHPStan (2.1.15)

20-20: Method pocketmine\form\FormAPI\CustomForm::__construct() has parameter $callable with no value type specified in iterable type array.
See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type

(missingType.iterableValue)


21-21: Parameter #1 $callable of method pocketmine\form\FormAPI\IForm::__construct() expects (callable(pocketmine\player\Player, mixed): void)|null, (callable(array|null): void)|null given.

(argument.type)

🤖 Prompt for AI Agents
In src/form/FormAPI/CustomForm.php around lines 18 to 21, the constructor's
callable parameter type and its docblock do not match the parent class
signature, violating the Liskov Substitution Principle. Update the docblock and
the callable parameter type to match the parent class, which expects a callable
with the signature callable(Player, mixed): void. Adjust the parameter type hint
and documentation accordingly to ensure compatibility.

KanadeBlue added 2 commits June 6, 2025 17:30
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 36

🔭 Outside diff range comments (1)
src/form/ChestAPI/session/PlayerSession.php (1)

1-103: ⚠️ Potential issue

Fix PHP CS Fixer formatting issues

Add the required license header, remove redundant docblock annotations, and ensure the file ends with a newline.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-110: PHP CS Fixer formatting issues detected. Added license header and removed redundant docblock annotations.

♻️ Duplicate comments (2)
src/form/ChestAPI/InvMenuEventHandler.php (1)

1-14: ⚠️ Potential issue

Fix PHP CS Fixer formatting issues.

The pipeline failure indicates missing license header and incorrect use statement ordering.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-75: PHP CS Fixer formatting issues detected. Added license header and reordered use statements.

src/form/ChestAPI/session/network/PlayerNetwork.php (1)

1-22: ⚠️ Potential issue

Fix PHP CS Fixer formatting issues.

The pipeline failure indicates missing license header, incorrect use statement ordering, and spacing issues in docblocks.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-146: PHP CS Fixer formatting issues detected. Added license header, reordered use statements, fixed spacing in docblocks.

🧹 Nitpick comments (21)
src/network/mcpe/handler/LoginPacketHandler.php (1)

197-200: Consider adding a comment explaining the default value choice.

The GraphicsMode default value of 1 should be documented to explain why this specific value was chosen for backward compatibility.

Apply this diff to improve documentation:

-		// Ensure GraphicsMode exists
+		// Ensure GraphicsMode exists for MC:BE 1.21.80+ compatibility (default: 1 = standard graphics)
src/form/ChestAPI/type/graphic/network/MultiInvMenuGraphicNetworkTranslator.php (1)

16-18: Consider adding runtime type validation for the translators array.

While the PHPDoc indicates the expected type, there's no runtime validation that all array elements are InvMenuGraphicNetworkTranslator instances.

 public function __construct(
 	readonly private array $translators
-){} 
+){
+	foreach($translators as $translator){
+		if(!($translator instanceof InvMenuGraphicNetworkTranslator)){
+			throw new \InvalidArgumentException("All translators must implement InvMenuGraphicNetworkTranslator");
+		}
+	}
+}
src/form/ChestAPI/type/graphic/network/BlockInvMenuGraphicNetworkTranslator.php (1)

24-32: Consider improving type safety and error handling.

The current implementation has a few areas for improvement:

  1. The type check could be more defensive
  2. The position casting to int may lose precision without validation
  3. Consider using more specific exception types
 public function translate(PlayerSession $session, InvMenuInfo $current, ContainerOpenPacket $packet) : void{
 	$graphic = $current->graphic;
-	if(!($graphic instanceof PositionedInvMenuGraphic)){
-		throw new InvalidArgumentException("Expected " . PositionedInvMenuGraphic::class . ", got " . get_class($graphic));
+	if(!($graphic instanceof PositionedInvMenuGraphic)){
+		throw new InvalidArgumentException(sprintf(
+			"Expected %s, got %s", 
+			PositionedInvMenuGraphic::class, 
+			get_class($graphic)
+		));
 	}

 	$pos = $graphic->getPosition();
+	// Validate position values are within valid range for BlockPosition
 	$packet->blockPosition = new BlockPosition((int) $pos->x, (int) $pos->y, (int) $pos->z);
 }
src/form/ChestAPI/session/network/handler/PlayerNetworkHandlerRegistry.php (1)

24-27: Document the PlayStation-specific timestamp multiplier rationale.

The PlayStation handler uses a different timestamp multiplier (* 1000 vs * 1000000 for default). This OS-specific behavior should be documented to explain why PlayStation requires different timing handling.

Add a comment explaining the reasoning:

 $this->register(DeviceOS::PLAYSTATION, new ClosurePlayerNetworkHandler(static function(Closure $then) : NetworkStackLatencyEntry{
 	$timestamp = mt_rand();
+	// PlayStation requires different latency timing due to platform-specific network stack behavior
 	return new NetworkStackLatencyEntry($timestamp * 1000000, $then, $timestamp * 1000);
 }));
src/form/ChestAPI/transaction/InvMenuTransaction.php (1)

20-34: Consider simplifying docblock annotations.

The pipeline indicates redundant docblock annotations. The @link and @return tags may be redundant since the method signatures already provide type information, and the @link references to other methods in the same interface may not add value.

 /**
  * Returns the item that was clicked / taken out of the inventory.
- *
- * @link InvMenuTransaction::getOut()
- * @return Item
  */
 public function getItemClicked() : Item;

 /**
  * Returns the item that an item was clicked with / placed in the inventory.
- *
- * @link InvMenuTransaction::getIn()
- * @return Item
  */
 public function getItemClickedWith() : Item;

The descriptive comments are valuable, but the redundant annotations can be removed.

src/form/ChestAPI/session/network/NetworkStackLatencyEntry.php (1)

14-14: Consider making the sent_at property initialization more explicit.

The $sent_at property is initialized to 0.0 but appears to be updated later. Consider adding documentation to clarify its purpose and lifecycle.

Add a comment to clarify the property's usage:

+	/** @var float Unix timestamp when the entry was sent, updated after construction */
	public float $sent_at = 0.0;
src/form/ChestAPI/type/util/builder/GraphicNetworkTranslatableInvMenuTypeBuilderTrait.php (1)

31-32: Consider using a more direct array access approach.

The use of array_key_first() is correct but slightly verbose for this simple case where you just need the first element.

Consider this slightly more direct approach:

-		return $this->graphic_network_translators[array_key_first($this->graphic_network_translators)];
+		return $this->graphic_network_translators[0];

Since you're appending translators with [] operator, the first element will always be at index 0.

src/form/ChestAPI/type/graphic/InvMenuGraphic.php (1)

15-15: Consider documenting the return value of sendInventory.

The sendInventory method returns a boolean, but the interface doesn't document what true or false represents.

Add documentation to clarify the return value:

+	/**
+	 * @param Player $player
+	 * @param Inventory $inventory
+	 * @return bool True if the inventory was successfully sent, false otherwise
+	 */
	public function sendInventory(Player $player, Inventory $inventory) : bool;
src/form/ChestAPI/type/ActorFixedInvMenuType.php (1)

37-39: Remove unnecessary nullable return type from createGraphic method.

PHPStan correctly identifies that this method never returns null, so the nullable return type can be removed.

-public function createGraphic(InvMenu $menu, Player $player) : ?InvMenuGraphic{
+public function createGraphic(InvMenu $menu, Player $player) : InvMenuGraphic{
     return new ActorInvMenuGraphic($this->actor_identifier, $this->actor_runtime_identifier, $this->actor_metadata, $this->network_translator);
 }
🧰 Tools
🪛 PHPStan (2.1.15)

37-37: Method pocketmine\form\ChestAPI\type\ActorFixedInvMenuType::createGraphic() never returns null so it can be removed from the return type.

(return.unusedType)

src/form/ChestAPI/type/InvMenuTypeRegistry.php (2)

19-39: Consider extracting menu type configurations for better maintainability.

While the current pre-registration works, consider extracting the menu type configurations to a separate method or configuration array for better maintainability and easier testing.

 public function __construct(){
+	$this->registerDefaultTypes();
+}
+
+private function registerDefaultTypes() : void{
+	$defaultTypes = [
+		InvMenuTypeIds::TYPE_CHEST => InvMenuTypeBuilders::BLOCK_ACTOR_FIXED()
+			->setBlock(VanillaBlocks::CHEST())
+			->setSize(27)
+			->setBlockActorId("Chest")
+			->build(),
+		// ... other types
+	];
+	
+	foreach($defaultTypes as $id => $type){
+		$this->register($id, $type);
+	}
+}

42-44: Simplify the cleanup logic for better readability.

The current cleanup logic on line 43 is complex and hard to read. Consider extracting the cleanup into a separate method.

 public function register(string $identifier, InvMenuType $type) : void{
 	if(isset($this->types[$identifier])){
-		unset($this->identifiers[spl_object_id($this->types[$identifier])], $this->types[$identifier]);
+		$this->unregisterType($identifier);
 	}
 
 	$this->types[$identifier] = $type;
 	$this->identifiers[spl_object_id($type)] = $identifier;
 }
+
+private function unregisterType(string $identifier) : void{
+	$oldType = $this->types[$identifier];
+	unset($this->identifiers[spl_object_id($oldType)]);
+	unset($this->types[$identifier]);
+}
src/form/ChestAPI/type/util/builder/ActorFixedInvMenuTypeBuilder.php (1)

16-21: Consider caching metadata instance for performance.

The constructor calls getActorMetadata() which might create a new instance each time. Consider caching the result if this method is expensive.

 public function __construct(){
-	$metadata = $this->getActorMetadata();
+	$metadata = $this->getActorMetadata(); // Cache this if getActorMetadata() is expensive
 	$metadata->setFloat(EntityMetadataProperties::BOUNDING_BOX_HEIGHT, 0.01);
 	$metadata->setFloat(EntityMetadataProperties::BOUNDING_BOX_WIDTH, 0.01);
 	$metadata->setGenericFlag(EntityMetadataFlags::INVISIBLE, true);
 }
src/form/ChestAPI/inventory/SharedInventoryNotifier.php (1)

13-16: Consider adding validation for constructor parameters.

While the readonly properties are good for immutability, consider adding null checks or type validation if there are specific requirements for the inventory and synchronizer instances.

 public function __construct(
 	readonly private Inventory $inventory,
 	readonly private SharedInventorySynchronizer $synchronizer
-){} 
+){
+	// Add validation if needed, e.g.:
+	// if ($inventory->getSize() <= 0) throw new InvalidArgumentException("Invalid inventory size");
+}
src/form/ChestAPI/type/graphic/BlockInvMenuGraphic.php (1)

29-31: Improve readability by breaking down the long method chain.

The send() method has a very long line that impacts readability.

Apply this diff to improve readability:

 public function send(Player $player, ?string $name) : void{
-    $player->getNetworkSession()->sendDataPacket(UpdateBlockPacket::create(BlockPosition::fromVector3($this->position), TypeConverter::getInstance()->getBlockTranslator()->internalIdToNetworkId($this->block->getStateId()), UpdateBlockPacket::FLAG_NETWORK, UpdateBlockPacket::DATA_LAYER_NORMAL));
+    $blockPosition = BlockPosition::fromVector3($this->position);
+    $networkId = TypeConverter::getInstance()->getBlockTranslator()->internalIdToNetworkId($this->block->getStateId());
+    $packet = UpdateBlockPacket::create($blockPosition, $networkId, UpdateBlockPacket::FLAG_NETWORK, UpdateBlockPacket::DATA_LAYER_NORMAL);
+    $player->getNetworkSession()->sendDataPacket($packet);
 }
src/form/ChestAPI/transaction/SimpleInvMenuTransaction.php (1)

34-40: Consider removing redundant proxy methods.

The methods getItemClicked() and getItemClickedWith() are simple proxies to getOut() and getIn() respectively. These add unnecessary indirection without providing additional value.

Consider either:

  1. Remove these proxy methods and use getOut()/getIn() directly, or
  2. Deprecate getOut()/getIn() in favor of the more descriptive names

If you choose option 1, apply this diff:

-public function getItemClicked() : Item{
-    return $this->getOut();
-}
-
-public function getItemClickedWith() : Item{
-    return $this->getIn();
-}
src/form/ChestAPI/type/util/builder/ActorInvMenuTypeBuilderTrait.php (1)

22-26: Document the implicit contract for addGraphicNetworkTranslator()

The trait calls addGraphicNetworkTranslator() which must be implemented by the class using this trait. Consider either documenting this requirement or creating an interface that enforces this contract.

+/**
+ * Trait for building actor-based inventory menu types.
+ * 
+ * Classes using this trait must implement:
+ * - addGraphicNetworkTranslator(InvMenuGraphicNetworkTranslator $translator): self
+ */
 trait ActorInvMenuTypeBuilderTrait{
src/form/ChestAPI/session/PlayerSession.php (1)

84-89: Remove redundant return type annotation

The @return PlayerNetwork annotation is redundant since the method already has a return type declaration.

 	/**
 	 * @deprecated Access {@see PlayerSession::$network} directly
-	 * @return PlayerNetwork
 	 */
 	public function getNetwork() : PlayerNetwork{
src/form/ChestAPI/InvMenuEventHandler.php (1)

51-51: Document the magic number for animation delay.

The 325ms delay value appears arbitrary. Consider extracting this to a named constant with documentation explaining why this specific duration was chosen.

+    private const ANIMATION_CLOSE_DELAY_MS = 325; // Delay to ensure smooth client-side animation
+
     /**
      * @param InventoryCloseEvent $event
      * @priority MONITOR
      */
     public function onInventoryClose(InventoryCloseEvent $event) : void{
         // ... existing code ...
-        $session->network->waitUntil(PlayerNetwork::DELAY_TYPE_ANIMATION_WAIT, 325, static fn(bool $success) : bool => false);
+        $session->network->waitUntil(PlayerNetwork::DELAY_TYPE_ANIMATION_WAIT, self::ANIMATION_CLOSE_DELAY_MS, static fn(bool $success) : bool => false);
     }
src/form/ChestAPI/session/network/PlayerNetwork.php (2)

129-142: Consider using integer arithmetic for timing calculations.

The current implementation uses floating-point arithmetic for millisecond calculations, which could lead to precision issues. Consider using integer microseconds throughout to avoid floating-point precision problems.

-    $elapsed_ms = 0.0;
+    $elapsed_us = 0;
     $this->wait($type, function(bool $success) use($wait_ms, $then, &$elapsed_ms) : bool{
         if($this->current === null){
             $then(false);
             return false;
         }

-        $elapsed_ms += (microtime(true) * 1000) - $this->current->sent_at;
-        if(!$success || $elapsed_ms >= $wait_ms){
+        $elapsed_us += (int)(microtime(true) * 1000000) - $this->current->sent_at_us;
+        if(!$success || $elapsed_us >= $wait_ms * 1000){
             $then($success);
             return false;
         }

         return true;
     });

Note: This would also require updating the sent_at property in NetworkStackLatencyEntry to use microseconds.


192-195: Add detailed documentation for callback priority management.

The callback manipulation logic is complex. Consider adding more detailed documentation explaining why this priority override is necessary and what specific PocketMine behavior it's working around.

-        // Take priority over other container open callbacks.
-        // PocketMine's default container open callback disallows any BlockInventory
-        // from having a custom callback
+        /**
+         * Take priority over other container open callbacks by temporarily removing all existing
+         * callbacks and re-adding them after ours. This is necessary because PocketMine's default
+         * container open callback disallows any BlockInventory from having a custom callback,
+         * which would prevent our custom packet translation from working correctly.
+         * 
+         * The callback order matters because the first callback to return non-null packets
+         * will be used, and subsequent callbacks won't be called.
+         */
src/form/ChestAPI/InvMenu.php (1)

148-152: Consider making the rate limit configurable.

The hard-coded limit of 8 pending requests is reasonable, but consider making it configurable or at least document why 8 was chosen.

Add a class constant for better maintainability:

+private const MAX_PENDING_REQUESTS = 8;
+
// ... in the send() method:
-if($network->getPending() >= 8){
+if($network->getPending() >= self::MAX_PENDING_REQUESTS){
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 69b3980 and 67a9164.

📒 Files selected for processing (52)
  • src/VersionInfo.php (1 hunks)
  • src/form/ChestAPI/InvMenu.php (1 hunks)
  • src/form/ChestAPI/InvMenuEventHandler.php (1 hunks)
  • src/form/ChestAPI/InvMenuHandler.php (1 hunks)
  • src/form/ChestAPI/inventory/InvMenuInventory.php (1 hunks)
  • src/form/ChestAPI/inventory/SharedInvMenuSynchronizer.php (1 hunks)
  • src/form/ChestAPI/inventory/SharedInventoryNotifier.php (1 hunks)
  • src/form/ChestAPI/inventory/SharedInventorySynchronizer.php (1 hunks)
  • src/form/ChestAPI/session/InvMenuInfo.php (1 hunks)
  • src/form/ChestAPI/session/PlayerManager.php (1 hunks)
  • src/form/ChestAPI/session/PlayerSession.php (1 hunks)
  • src/form/ChestAPI/session/network/NetworkStackLatencyEntry.php (1 hunks)
  • src/form/ChestAPI/session/network/PlayerNetwork.php (1 hunks)
  • src/form/ChestAPI/session/network/handler/ClosurePlayerNetworkHandler.php (1 hunks)
  • src/form/ChestAPI/session/network/handler/PlayerNetworkHandler.php (1 hunks)
  • src/form/ChestAPI/session/network/handler/PlayerNetworkHandlerRegistry.php (1 hunks)
  • src/form/ChestAPI/transaction/DeterministicInvMenuTransaction.php (1 hunks)
  • src/form/ChestAPI/transaction/InvMenuTransaction.php (1 hunks)
  • src/form/ChestAPI/transaction/InvMenuTransactionResult.php (1 hunks)
  • src/form/ChestAPI/transaction/SimpleInvMenuTransaction.php (1 hunks)
  • src/form/ChestAPI/type/ActorFixedInvMenuType.php (1 hunks)
  • src/form/ChestAPI/type/BlockActorFixedInvMenuType.php (1 hunks)
  • src/form/ChestAPI/type/BlockFixedInvMenuType.php (1 hunks)
  • src/form/ChestAPI/type/DoublePairableBlockActorFixedInvMenuType.php (1 hunks)
  • src/form/ChestAPI/type/FixedInvMenuType.php (1 hunks)
  • src/form/ChestAPI/type/InvMenuType.php (1 hunks)
  • src/form/ChestAPI/type/InvMenuTypeIds.php (1 hunks)
  • src/form/ChestAPI/type/InvMenuTypeRegistry.php (1 hunks)
  • src/form/ChestAPI/type/graphic/ActorInvMenuGraphic.php (1 hunks)
  • src/form/ChestAPI/type/graphic/BlockActorInvMenuGraphic.php (1 hunks)
  • src/form/ChestAPI/type/graphic/BlockInvMenuGraphic.php (1 hunks)
  • src/form/ChestAPI/type/graphic/InvMenuGraphic.php (1 hunks)
  • src/form/ChestAPI/type/graphic/MultiBlockInvMenuGraphic.php (1 hunks)
  • src/form/ChestAPI/type/graphic/PositionedInvMenuGraphic.php (1 hunks)
  • src/form/ChestAPI/type/graphic/network/ActorInvMenuGraphicNetworkTranslator.php (1 hunks)
  • src/form/ChestAPI/type/graphic/network/BlockInvMenuGraphicNetworkTranslator.php (1 hunks)
  • src/form/ChestAPI/type/graphic/network/InvMenuGraphicNetworkTranslator.php (1 hunks)
  • src/form/ChestAPI/type/graphic/network/MultiInvMenuGraphicNetworkTranslator.php (1 hunks)
  • src/form/ChestAPI/type/graphic/network/WindowTypeInvMenuGraphicNetworkTranslator.php (1 hunks)
  • src/form/ChestAPI/type/util/InvMenuTypeBuilders.php (1 hunks)
  • src/form/ChestAPI/type/util/InvMenuTypeHelper.php (1 hunks)
  • src/form/ChestAPI/type/util/builder/ActorFixedInvMenuTypeBuilder.php (1 hunks)
  • src/form/ChestAPI/type/util/builder/ActorInvMenuTypeBuilderTrait.php (1 hunks)
  • src/form/ChestAPI/type/util/builder/AnimationDurationInvMenuTypeBuilderTrait.php (1 hunks)
  • src/form/ChestAPI/type/util/builder/BlockActorFixedInvMenuTypeBuilder.php (1 hunks)
  • src/form/ChestAPI/type/util/builder/BlockFixedInvMenuTypeBuilder.php (1 hunks)
  • src/form/ChestAPI/type/util/builder/BlockInvMenuTypeBuilderTrait.php (1 hunks)
  • src/form/ChestAPI/type/util/builder/DoublePairableBlockActorFixedInvMenuTypeBuilder.php (1 hunks)
  • src/form/ChestAPI/type/util/builder/FixedInvMenuTypeBuilderTrait.php (1 hunks)
  • src/form/ChestAPI/type/util/builder/GraphicNetworkTranslatableInvMenuTypeBuilderTrait.php (1 hunks)
  • src/form/ChestAPI/type/util/builder/InvMenuTypeBuilder.php (1 hunks)
  • src/network/mcpe/handler/LoginPacketHandler.php (2 hunks)
✅ Files skipped from review due to trivial changes (7)
  • src/form/ChestAPI/type/util/builder/InvMenuTypeBuilder.php
  • src/form/ChestAPI/type/InvMenuTypeIds.php
  • src/form/ChestAPI/type/util/InvMenuTypeBuilders.php
  • src/form/ChestAPI/inventory/InvMenuInventory.php
  • src/form/ChestAPI/type/graphic/network/ActorInvMenuGraphicNetworkTranslator.php
  • src/form/ChestAPI/session/InvMenuInfo.php
  • src/form/ChestAPI/type/graphic/PositionedInvMenuGraphic.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/VersionInfo.php
🧰 Additional context used
🧬 Code Graph Analysis (11)
src/form/ChestAPI/type/FixedInvMenuType.php (5)
src/form/ChestAPI/type/ActorFixedInvMenuType.php (1)
  • getSize (33-35)
src/form/ChestAPI/type/BlockActorFixedInvMenuType.php (1)
  • getSize (32-34)
src/form/ChestAPI/type/BlockFixedInvMenuType.php (1)
  • getSize (25-27)
src/form/ChestAPI/type/DoublePairableBlockActorFixedInvMenuType.php (1)
  • getSize (32-34)
src/form/ChestAPI/type/util/builder/FixedInvMenuTypeBuilderTrait.php (1)
  • getSize (18-20)
src/form/ChestAPI/type/util/builder/BlockFixedInvMenuTypeBuilder.php (7)
src/form/ChestAPI/type/BlockFixedInvMenuType.php (2)
  • BlockFixedInvMenuType (17-41)
  • getSize (25-27)
src/form/ChestAPI/type/graphic/network/BlockInvMenuGraphicNetworkTranslator.php (2)
  • BlockInvMenuGraphicNetworkTranslator (14-33)
  • instance (16-19)
src/form/ChestAPI/type/util/builder/GraphicNetworkTranslatableInvMenuTypeBuilderTrait.php (2)
  • addGraphicNetworkTranslator (16-19)
  • getGraphicNetworkTranslator (26-36)
src/form/ChestAPI/type/util/builder/InvMenuTypeBuilder.php (1)
  • build (11-11)
src/form/ChestAPI/type/util/builder/BlockInvMenuTypeBuilderTrait.php (1)
  • getBlock (19-21)
src/form/ChestAPI/type/FixedInvMenuType.php (1)
  • getSize (17-17)
src/form/ChestAPI/type/util/builder/FixedInvMenuTypeBuilderTrait.php (1)
  • getSize (18-20)
src/form/ChestAPI/type/util/builder/FixedInvMenuTypeBuilderTrait.php (6)
src/form/ChestAPI/type/util/builder/ActorFixedInvMenuTypeBuilder.php (1)
  • setSize (29-33)
src/form/ChestAPI/type/ActorFixedInvMenuType.php (1)
  • getSize (33-35)
src/form/ChestAPI/type/BlockActorFixedInvMenuType.php (1)
  • getSize (32-34)
src/form/ChestAPI/type/FixedInvMenuType.php (1)
  • getSize (17-17)
src/form/ChestAPI/type/BlockFixedInvMenuType.php (1)
  • getSize (25-27)
src/form/ChestAPI/type/DoublePairableBlockActorFixedInvMenuType.php (1)
  • getSize (32-34)
src/form/ChestAPI/type/graphic/network/WindowTypeInvMenuGraphicNetworkTranslator.php (3)
src/form/ChestAPI/session/InvMenuInfo.php (2)
  • InvMenuInfo (10-17)
  • __construct (12-16)
src/form/ChestAPI/session/PlayerSession.php (2)
  • PlayerSession (12-102)
  • __construct (16-19)
src/form/ChestAPI/type/graphic/network/InvMenuGraphicNetworkTranslator.php (1)
  • translate (13-13)
src/form/ChestAPI/type/util/builder/ActorFixedInvMenuTypeBuilder.php (6)
src/form/ChestAPI/type/ActorFixedInvMenuType.php (2)
  • ActorFixedInvMenuType (16-44)
  • getSize (33-35)
src/form/ChestAPI/type/util/builder/FixedInvMenuTypeBuilderTrait.php (2)
  • setSize (13-16)
  • getSize (18-20)
src/form/ChestAPI/type/util/builder/GraphicNetworkTranslatableInvMenuTypeBuilderTrait.php (2)
  • setNetworkWindowType (21-24)
  • getGraphicNetworkTranslator (26-36)
src/form/ChestAPI/type/util/builder/ActorInvMenuTypeBuilderTrait.php (3)
  • getActorMetadata (28-30)
  • getActorIdentifier (37-39)
  • getActorRuntimeIdentifier (18-20)
src/form/ChestAPI/type/util/builder/InvMenuTypeBuilder.php (1)
  • build (11-11)
src/form/ChestAPI/type/FixedInvMenuType.php (1)
  • getSize (17-17)
src/form/ChestAPI/type/util/builder/GraphicNetworkTranslatableInvMenuTypeBuilderTrait.php (3)
src/form/ChestAPI/type/graphic/network/MultiInvMenuGraphicNetworkTranslator.php (1)
  • MultiInvMenuGraphicNetworkTranslator (11-25)
src/form/ChestAPI/type/graphic/network/WindowTypeInvMenuGraphicNetworkTranslator.php (1)
  • WindowTypeInvMenuGraphicNetworkTranslator (11-20)
src/form/ChestAPI/type/util/builder/ActorFixedInvMenuTypeBuilder.php (1)
  • setNetworkWindowType (23-27)
src/form/ChestAPI/type/util/builder/DoublePairableBlockActorFixedInvMenuTypeBuilder.php (9)
src/form/ChestAPI/type/DoublePairableBlockActorFixedInvMenuType.php (2)
  • DoublePairableBlockActorFixedInvMenuType (22-70)
  • getSize (32-34)
src/form/ChestAPI/type/graphic/network/BlockInvMenuGraphicNetworkTranslator.php (2)
  • BlockInvMenuGraphicNetworkTranslator (14-33)
  • instance (16-19)
src/form/ChestAPI/type/util/builder/GraphicNetworkTranslatableInvMenuTypeBuilderTrait.php (1)
  • getGraphicNetworkTranslator (26-36)
src/form/ChestAPI/type/util/builder/BlockActorFixedInvMenuTypeBuilder.php (3)
  • setBlockActorId (23-26)
  • getBlockActorId (28-30)
  • build (32-34)
src/form/ChestAPI/type/util/builder/InvMenuTypeBuilder.php (1)
  • build (11-11)
src/form/ChestAPI/type/util/builder/BlockInvMenuTypeBuilderTrait.php (1)
  • getBlock (19-21)
src/form/ChestAPI/type/FixedInvMenuType.php (1)
  • getSize (17-17)
src/form/ChestAPI/type/util/builder/FixedInvMenuTypeBuilderTrait.php (1)
  • getSize (18-20)
src/form/ChestAPI/type/util/builder/AnimationDurationInvMenuTypeBuilderTrait.php (1)
  • getAnimationDuration (16-18)
src/form/ChestAPI/type/util/builder/BlockActorFixedInvMenuTypeBuilder.php (7)
src/form/ChestAPI/type/BlockActorFixedInvMenuType.php (2)
  • BlockActorFixedInvMenuType (22-54)
  • getSize (32-34)
src/form/ChestAPI/type/graphic/network/BlockInvMenuGraphicNetworkTranslator.php (2)
  • BlockInvMenuGraphicNetworkTranslator (14-33)
  • instance (16-19)
src/form/ChestAPI/type/util/builder/GraphicNetworkTranslatableInvMenuTypeBuilderTrait.php (1)
  • getGraphicNetworkTranslator (26-36)
src/form/ChestAPI/type/util/builder/BlockInvMenuTypeBuilderTrait.php (1)
  • getBlock (19-21)
src/form/ChestAPI/type/FixedInvMenuType.php (1)
  • getSize (17-17)
src/form/ChestAPI/type/util/builder/FixedInvMenuTypeBuilderTrait.php (1)
  • getSize (18-20)
src/form/ChestAPI/type/util/builder/AnimationDurationInvMenuTypeBuilderTrait.php (1)
  • getAnimationDuration (16-18)
src/form/ChestAPI/session/PlayerSession.php (3)
src/form/ChestAPI/session/network/PlayerNetwork.php (5)
  • PlayerNetwork (23-230)
  • finalize (48-52)
  • waitUntil (122-143)
  • onBeforeSendMenu (180-225)
  • wait (105-113)
src/player/Player.php (2)
  • Player (169-2854)
  • removeCurrentWindow (2793-2805)
src/form/ChestAPI/session/InvMenuInfo.php (2)
  • InvMenuInfo (10-17)
  • __construct (12-16)
src/form/ChestAPI/type/graphic/InvMenuGraphic.php (6)
src/form/ChestAPI/InvMenu.php (2)
  • send (138-176)
  • sendInventory (184-186)
src/form/ChestAPI/type/graphic/ActorInvMenuGraphic.php (5)
  • send (34-54)
  • sendInventory (56-58)
  • remove (60-62)
  • getNetworkTranslator (64-66)
  • getAnimationDuration (68-70)
src/form/ChestAPI/type/graphic/BlockActorInvMenuGraphic.php (5)
  • send (47-53)
  • sendInventory (55-57)
  • remove (59-61)
  • getNetworkTranslator (63-65)
  • getAnimationDuration (67-69)
src/form/ChestAPI/type/graphic/BlockInvMenuGraphic.php (5)
  • send (29-31)
  • sendInventory (33-35)
  • remove (37-42)
  • getNetworkTranslator (44-46)
  • getAnimationDuration (48-50)
src/form/ChestAPI/type/graphic/MultiBlockInvMenuGraphic.php (5)
  • send (31-35)
  • sendInventory (37-39)
  • remove (41-45)
  • getNetworkTranslator (47-49)
  • getAnimationDuration (55-64)
src/form/ChestAPI/type/util/builder/AnimationDurationInvMenuTypeBuilderTrait.php (1)
  • getAnimationDuration (16-18)
src/form/ChestAPI/session/network/handler/PlayerNetworkHandler.php (2)
src/form/ChestAPI/session/network/NetworkStackLatencyEntry.php (1)
  • NetworkStackLatencyEntry (9-21)
src/form/ChestAPI/session/network/handler/ClosurePlayerNetworkHandler.php (1)
  • createNetworkStackLatencyEntry (19-21)
🪛 GitHub Actions: CI
src/network/mcpe/handler/LoginPacketHandler.php

[error] 193-214: PHP CS Fixer formatting issues detected. Trailing whitespace removed and code style adjusted.

src/form/ChestAPI/type/InvMenuType.php

[error] 1-33: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/type/FixedInvMenuType.php

[error] 1-33: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/type/graphic/network/BlockInvMenuGraphicNetworkTranslator.php

[error] 1-52: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.

src/form/ChestAPI/type/graphic/network/InvMenuGraphicNetworkTranslator.php

[error] 1-34: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/inventory/SharedInvMenuSynchronizer.php

[error] 1-28: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/type/graphic/network/MultiInvMenuGraphicNetworkTranslator.php

[error] 1-41: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/InvMenuHandler.php

[error] 1-65: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/type/util/builder/BlockFixedInvMenuTypeBuilder.php

[error] 1-42: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.

src/form/ChestAPI/session/network/handler/ClosurePlayerNetworkHandler.php

[error] 1-43: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/type/util/builder/FixedInvMenuTypeBuilderTrait.php

[error] 1-40: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.

src/form/ChestAPI/session/network/handler/PlayerNetworkHandlerRegistry.php

[error] 1-29: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/type/graphic/network/WindowTypeInvMenuGraphicNetworkTranslator.php

[error] 1-37: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/transaction/InvMenuTransaction.php

[error] 1-59: PHP CS Fixer formatting issues detected. Added license header and removed redundant docblock annotations.

src/form/ChestAPI/type/ActorFixedInvMenuType.php

[error] 1-63: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/session/PlayerManager.php

[error] 1-47: PHP CS Fixer formatting issues detected. Added license header and reordered use statements.

src/form/ChestAPI/type/util/builder/BlockInvMenuTypeBuilderTrait.php

[error] 1-42: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.

src/form/ChestAPI/transaction/SimpleInvMenuTransaction.php

[error] 1-77: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/type/util/builder/ActorFixedInvMenuTypeBuilder.php

[error] 1-57: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.

src/form/ChestAPI/type/BlockFixedInvMenuType.php

[error] 1-61: PHP CS Fixer formatting issues detected. Added license header and reordered use statements.

src/form/ChestAPI/type/util/builder/GraphicNetworkTranslatableInvMenuTypeBuilderTrait.php

[error] 1-59: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.

src/form/ChestAPI/inventory/SharedInventorySynchronizer.php

[error] 1-28: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/type/util/builder/AnimationDurationInvMenuTypeBuilderTrait.php

[error] 1-39: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.

src/form/ChestAPI/type/util/builder/DoublePairableBlockActorFixedInvMenuTypeBuilder.php

[error] 1-57: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.

src/form/ChestAPI/inventory/SharedInventoryNotifier.php

[error] 1-28: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/type/util/builder/BlockActorFixedInvMenuTypeBuilder.php

[error] 1-56: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.

src/form/ChestAPI/type/graphic/MultiBlockInvMenuGraphic.php

[error] 1-86: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.

src/form/ChestAPI/transaction/InvMenuTransactionResult.php

[error] 1-66: PHP CS Fixer formatting issues detected. Added license header and removed redundant docblock annotations.

src/form/ChestAPI/type/BlockActorFixedInvMenuType.php

[error] 1-39: PHP CS Fixer formatting issues detected. Added license header and reordered use statements.

src/form/ChestAPI/type/util/builder/ActorInvMenuTypeBuilderTrait.php

[error] 1-41: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.

src/form/ChestAPI/type/util/InvMenuTypeHelper.php

[error] 1-69: PHP CS Fixer formatting issues detected. Added license header and removed unused parameter annotations.

src/form/ChestAPI/transaction/DeterministicInvMenuTransaction.php

[error] 1-80: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/type/DoublePairableBlockActorFixedInvMenuType.php

[error] 1-34: PHP CS Fixer formatting issues detected. Added license header and reordered use statements.

src/form/ChestAPI/session/PlayerSession.php

[error] 1-110: PHP CS Fixer formatting issues detected. Added license header and removed redundant docblock annotations.

src/form/ChestAPI/type/graphic/BlockActorInvMenuGraphic.php

[error] 1-35: PHP CS Fixer formatting issues detected. Added license header and reordered use statements.

src/form/ChestAPI/type/InvMenuTypeRegistry.php

[error] 1-93: PHP CS Fixer formatting issues detected. Added license header, reordered use statements, and added missing newline at end of file.

src/form/ChestAPI/InvMenuEventHandler.php

[error] 1-75: PHP CS Fixer formatting issues detected. Added license header and reordered use statements.

src/form/ChestAPI/type/graphic/BlockInvMenuGraphic.php

[error] 1-32: PHP CS Fixer formatting issues detected. Added license header and reordered use statements.

src/form/ChestAPI/type/graphic/ActorInvMenuGraphic.php

[error] 1-40: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/session/network/PlayerNetwork.php

[error] 1-146: PHP CS Fixer formatting issues detected. Added license header, reordered use statements, fixed spacing in docblocks.

src/form/ChestAPI/InvMenu.php

[error] 1-44: PHP CS Fixer formatting issues detected. Added license header and removed redundant return type annotations.

src/form/ChestAPI/type/graphic/InvMenuGraphic.php

[error] 1-40: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/session/network/NetworkStackLatencyEntry.php

[error] 1-41: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/session/network/handler/PlayerNetworkHandler.php

[error] 1-33: PHP CS Fixer formatting issues detected. Added license header.

🪛 GitHub Check: PHP 8.3 / PHPStan analysis
src/form/ChestAPI/session/network/handler/ClosurePlayerNetworkHandler.php

[failure] 19-19:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.


[failure] 15-15:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::__construct() has parameter $creator with no signature specified for Closure.

src/form/ChestAPI/session/PlayerManager.php

[failure] 38-38:
Parameter #1 $os_id of method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandlerRegistry::get() expects int, mixed given.

src/form/ChestAPI/transaction/DeterministicInvMenuTransaction.php

[failure] 29-29:
Method pocketmine\form\ChestAPI\transaction\DeterministicInvMenuTransaction::then() has parameter $callback with no signature specified for Closure.

src/form/ChestAPI/InvMenu.php

[failure] 52-52:
Property pocketmine\form\ChestAPI\InvMenu::$inventory_close_listener type has no signature specified for Closure.


[failure] 51-51:
Property pocketmine\form\ChestAPI\InvMenu::$listener type has no signature specified for Closure.


[failure] 32-32:
Parameter #2 $custom_inventory of class pocketmine\form\ChestAPI\InvMenu constructor expects pocketmine\inventory\Inventory|null, mixed given.

src/form/ChestAPI/session/network/NetworkStackLatencyEntry.php

[failure] 16-16:
Method pocketmine\form\ChestAPI\session\network\NetworkStackLatencyEntry::__construct() has parameter $then with no signature specified for Closure.


[failure] 13-13:
Property pocketmine\form\ChestAPI\session\network\NetworkStackLatencyEntry::$then type has no signature specified for Closure.

src/form/ChestAPI/session/network/handler/PlayerNetworkHandler.php

[failure] 12-12:
Method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis
src/form/ChestAPI/session/network/handler/ClosurePlayerNetworkHandler.php

[failure] 19-19:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.


[failure] 15-15:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::__construct() has parameter $creator with no signature specified for Closure.

src/form/ChestAPI/session/PlayerManager.php

[failure] 38-38:
Parameter #1 $os_id of method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandlerRegistry::get() expects int, mixed given.

src/form/ChestAPI/transaction/DeterministicInvMenuTransaction.php

[failure] 29-29:
Method pocketmine\form\ChestAPI\transaction\DeterministicInvMenuTransaction::then() has parameter $callback with no signature specified for Closure.

src/form/ChestAPI/InvMenu.php

[failure] 52-52:
Property pocketmine\form\ChestAPI\InvMenu::$inventory_close_listener type has no signature specified for Closure.


[failure] 51-51:
Property pocketmine\form\ChestAPI\InvMenu::$listener type has no signature specified for Closure.


[failure] 32-32:
Parameter #2 $custom_inventory of class pocketmine\form\ChestAPI\InvMenu constructor expects pocketmine\inventory\Inventory|null, mixed given.

src/form/ChestAPI/session/network/NetworkStackLatencyEntry.php

[failure] 16-16:
Method pocketmine\form\ChestAPI\session\network\NetworkStackLatencyEntry::__construct() has parameter $then with no signature specified for Closure.


[failure] 13-13:
Property pocketmine\form\ChestAPI\session\network\NetworkStackLatencyEntry::$then type has no signature specified for Closure.

src/form/ChestAPI/session/network/handler/PlayerNetworkHandler.php

[failure] 12-12:
Method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis
src/form/ChestAPI/session/network/handler/ClosurePlayerNetworkHandler.php

[failure] 19-19:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.


[failure] 15-15:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::__construct() has parameter $creator with no signature specified for Closure.

src/form/ChestAPI/session/PlayerManager.php

[failure] 38-38:
Parameter #1 $os_id of method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandlerRegistry::get() expects int, mixed given.

src/form/ChestAPI/transaction/DeterministicInvMenuTransaction.php

[failure] 29-29:
Method pocketmine\form\ChestAPI\transaction\DeterministicInvMenuTransaction::then() has parameter $callback with no signature specified for Closure.

src/form/ChestAPI/InvMenu.php

[failure] 52-52:
Property pocketmine\form\ChestAPI\InvMenu::$inventory_close_listener type has no signature specified for Closure.


[failure] 51-51:
Property pocketmine\form\ChestAPI\InvMenu::$listener type has no signature specified for Closure.


[failure] 32-32:
Parameter #2 $custom_inventory of class pocketmine\form\ChestAPI\InvMenu constructor expects pocketmine\inventory\Inventory|null, mixed given.

src/form/ChestAPI/session/network/NetworkStackLatencyEntry.php

[failure] 16-16:
Method pocketmine\form\ChestAPI\session\network\NetworkStackLatencyEntry::__construct() has parameter $then with no signature specified for Closure.


[failure] 13-13:
Property pocketmine\form\ChestAPI\session\network\NetworkStackLatencyEntry::$then type has no signature specified for Closure.

src/form/ChestAPI/session/network/handler/PlayerNetworkHandler.php

[failure] 12-12:
Method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

🪛 PHPStan (2.1.15)
src/form/ChestAPI/session/network/handler/ClosurePlayerNetworkHandler.php

15-15: Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::__construct() has parameter $creator with no signature specified for Closure.

(missingType.callable)


19-19: Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

(missingType.callable)

src/form/ChestAPI/type/ActorFixedInvMenuType.php

37-37: Method pocketmine\form\ChestAPI\type\ActorFixedInvMenuType::createGraphic() never returns null so it can be removed from the return type.

(return.unusedType)

src/form/ChestAPI/session/PlayerManager.php

38-38: Parameter #1 $os_id of method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandlerRegistry::get() expects int, mixed given.

(argument.type)

src/form/ChestAPI/type/util/InvMenuTypeHelper.php

45-45: Parameter #1 $x of method pocketmine\world\World::getTileAt() expects int, float|int given.

(argument.type)


45-45: Parameter #2 $y of method pocketmine\world\World::getTileAt() expects int, float|int given.

(argument.type)


45-45: Parameter #3 $z of method pocketmine\world\World::getTileAt() expects int, float|int given.

(argument.type)

src/form/ChestAPI/transaction/DeterministicInvMenuTransaction.php

29-29: Method pocketmine\form\ChestAPI\transaction\DeterministicInvMenuTransaction::then() has parameter $callback with no signature specified for Closure.

(missingType.callable)

src/form/ChestAPI/type/DoublePairableBlockActorFixedInvMenuType.php

54-54: Parameter #2 $value of method pocketmine\nbt\tag\CompoundTag::setInt() expects int, float|int given.

(argument.type)


55-55: Parameter #2 $value of method pocketmine\nbt\tag\CompoundTag::setInt() expects int, float|int given.

(argument.type)

src/form/ChestAPI/InvMenu.php

32-32: Parameter #2 $custom_inventory of class pocketmine\form\ChestAPI\InvMenu constructor expects pocketmine\inventory\Inventory|null, mixed given.

(argument.type)


51-51: Property pocketmine\form\ChestAPI\InvMenu::$listener type has no signature specified for Closure.

(missingType.callable)


52-52: Property pocketmine\form\ChestAPI\InvMenu::$inventory_close_listener type has no signature specified for Closure.

(missingType.callable)

src/form/ChestAPI/session/network/NetworkStackLatencyEntry.php

13-13: Property pocketmine\form\ChestAPI\session\network\NetworkStackLatencyEntry::$then type has no signature specified for Closure.

(missingType.callable)


16-16: Method pocketmine\form\ChestAPI\session\network\NetworkStackLatencyEntry::__construct() has parameter $then with no signature specified for Closure.

(missingType.callable)

src/form/ChestAPI/session/network/handler/PlayerNetworkHandler.php

12-12: Method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

(missingType.callable)

🔇 Additional comments (85)
src/form/ChestAPI/type/FixedInvMenuType.php (1)

1-18: Address the pipeline failure for missing license header.

The CI pipeline has detected that this file is missing a required license header. Please add the appropriate license header to resolve the formatting issue.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-33: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/session/network/handler/PlayerNetworkHandler.php (1)

1-13: Address the pipeline failure for missing license header.

The CI pipeline has detected that this file is missing a required license header. Please add the appropriate license header to resolve the formatting issue.

🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 12-12:
Method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 12-12:
Method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 12-12:
Method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

🪛 PHPStan (2.1.15)

12-12: Method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

(missingType.callable)

🪛 GitHub Actions: CI

[error] 1-33: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/type/InvMenuType.php (1)

1-17: Address the pipeline failure for missing license header.

The CI pipeline has detected that this file is missing a required license header. Please add the appropriate license header to resolve the formatting issue.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-33: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/type/util/builder/FixedInvMenuTypeBuilderTrait.php (1)

1-21: Address pipeline failures and approve the implementation.

The CI pipeline has detected formatting issues that need to be resolved:

  1. Missing license header
  2. Missing newline at the end of the file

The trait logic is well-implemented with proper validation and fluent interface pattern.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-40: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.

src/form/ChestAPI/type/graphic/network/InvMenuGraphicNetworkTranslator.php (1)

11-14: LGTM! Clean interface design.

The interface follows good practices with clear method signature and proper type hints. The void return type is appropriate for a translator pattern.

src/form/ChestAPI/type/util/builder/BlockInvMenuTypeBuilderTrait.php (2)

1-22: Address pipeline failures: missing license header and newline.

The PHP CS Fixer detected missing license header and missing newline at end of file.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-42: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.


14-21: LGTM! Solid trait implementation with proper error handling.

The trait correctly implements the builder pattern with:

  • Fluent interface using return $this
  • Proper encapsulation with private property
  • Defensive programming with LogicException for missing block
src/form/ChestAPI/type/graphic/network/MultiInvMenuGraphicNetworkTranslator.php (2)

1-25: Address the pipeline failure: missing license header.

The PHP CS Fixer detected that a license header is missing from this file.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-41: PHP CS Fixer formatting issues detected. Added license header.


20-24: LGTM! Excellent composite pattern implementation.

The delegation to each translator in sequence is correctly implemented and follows the composite pattern well.

src/form/ChestAPI/type/util/builder/BlockFixedInvMenuTypeBuilder.php (3)

1-22: Address pipeline failures: missing license header and newline.

The PHP CS Fixer detected missing license header and missing newline at end of file.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-42: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.


10-17: LGTM! Excellent use of traits for builder composition.

The class effectively combines multiple traits to build the required functionality and properly initializes with a default network translator.


19-21: LGTM! Proper builder implementation with covariant return type.

The build method correctly:

  • Uses trait methods to access required values (with proper validation)
  • Returns the more specific BlockFixedInvMenuType instead of just InvMenuType (good use of covariance)
  • Passes all required parameters to the constructor
src/form/ChestAPI/session/network/handler/ClosurePlayerNetworkHandler.php (1)

10-22: Implementation logic is sound.

The delegation pattern using the injected closure is well-implemented and provides good flexibility for customizing network latency entry creation.

🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 19-19:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.


[failure] 15-15:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::__construct() has parameter $creator with no signature specified for Closure.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 19-19:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.


[failure] 15-15:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::__construct() has parameter $creator with no signature specified for Closure.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 19-19:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.


[failure] 15-15:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::__construct() has parameter $creator with no signature specified for Closure.

🪛 PHPStan (2.1.15)

15-15: Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::__construct() has parameter $creator with no signature specified for Closure.

(missingType.callable)


19-19: Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

(missingType.callable)

src/form/ChestAPI/type/BlockFixedInvMenuType.php (3)

19-23: Well-designed constructor with immutable properties.

The use of readonly properties promotes immutability and the optional network translator parameter provides good flexibility.


29-36: Excellent error handling and validation.

The method properly validates the Y coordinate and returns null when the position is invalid, preventing potential issues with out-of-bounds block placement. The position calculation using helper methods is also well-structured.


38-40: Clean inventory creation implementation.

The method correctly creates a new inventory instance with the specified size.

src/form/ChestAPI/type/graphic/network/WindowTypeInvMenuGraphicNetworkTranslator.php (1)

11-20: Clean and focused implementation.

The class has a single, well-defined responsibility of translating window types in network packets. The use of readonly properties and the straightforward delegation in the translate method make this a solid implementation.

src/form/ChestAPI/type/util/builder/AnimationDurationInvMenuTypeBuilderTrait.php (1)

7-19: Well-implemented builder trait following best practices.

The trait properly encapsulates the animation duration property with appropriate access modifiers. The fluent interface in the setter and the protected getter provide good API design for builder patterns.

src/form/ChestAPI/type/graphic/network/BlockInvMenuGraphicNetworkTranslator.php (2)

16-19: LGTM: Proper singleton implementation.

The singleton pattern is correctly implemented with lazy initialization and private constructor to enforce the pattern.


1-33: Address pipeline formatting issues.

The pipeline indicates missing license header and newline at end of file. These formatting issues should be resolved to pass CI checks.

Based on the pipeline failure logs, you need to:

  1. Add the required license header at the top of the file
  2. Add a newline at the end of the file

Please ensure PHP CS Fixer standards are met.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-52: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.

src/form/ChestAPI/session/network/handler/PlayerNetworkHandlerRegistry.php (1)

1-42: Address pipeline formatting issues.

The pipeline indicates a missing license header. Please add the required license header to resolve the formatting check failure.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-29: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/inventory/SharedInvMenuSynchronizer.php (4)

16-26: LGTM: Well-designed bidirectional synchronization setup.

The constructor properly establishes bidirectional synchronization:

  1. Creates synchronizer for menu inventory and adds it as listener to external inventory
  2. Creates notifier for external inventory and adds it to menu inventory
  3. Initializes menu inventory contents from external inventory

This design should prevent circular updates and maintain consistency between inventories.


28-31: LGTM: Proper cleanup implementation.

The destroy method correctly removes listeners from both inventories, preventing memory leaks and ensuring clean teardown of the synchronization.


12-14: Good use of readonly properties for immutable state.

Using readonly properties for the core synchronization components ensures they cannot be accidentally modified after initialization, improving code safety.


1-32: Address pipeline formatting issues.

The pipeline indicates a missing license header. Please add the required license header to resolve the formatting check failure.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-28: PHP CS Fixer formatting issues detected. Added license header.

src/form/ChestAPI/transaction/InvMenuTransaction.php (3)

26-34: LGTM: Good semantic method aliases.

The getItemClicked() and getItemClickedWith() methods provide better semantic meaning compared to getOut() and getIn(), making the interface more intuitive for developers.


40-42: LGTM: Clean transaction control flow design.

The continue() and discard() methods provide a clean API for controlling transaction flow, returning result objects that can encapsulate the outcome and any follow-up actions.


1-43: Address pipeline formatting issues.

The pipeline indicates issues with license header and redundant docblock annotations. Please resolve these formatting issues to pass CI checks.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-59: PHP CS Fixer formatting issues detected. Added license header and removed redundant docblock annotations.

src/form/ChestAPI/type/util/builder/GraphicNetworkTranslatableInvMenuTypeBuilderTrait.php (1)

21-24: LGTM! Clean fluent interface implementation.

The setNetworkWindowType method provides a convenient way to add a WindowTypeInvMenuGraphicNetworkTranslator through the fluent interface. The implementation is clean and follows the builder pattern correctly.

src/form/ChestAPI/type/util/builder/DoublePairableBlockActorFixedInvMenuTypeBuilder.php (3)

19-21: LGTM! Good default initialization in constructor.

The constructor appropriately adds a default BlockInvMenuGraphicNetworkTranslator instance, which aligns with the pattern used in other builder classes like BlockActorFixedInvMenuTypeBuilder.


11-15: Excellent use of trait composition.

The class demonstrates good use of trait composition to build functionality from reusable components. This follows the principle of composition over inheritance and keeps the class focused.


32-34:

Details

✅ Verification successful

Verify parameter order consistency with other builders.

The build method passes parameters in this order: block, size, blockActorId, networkTranslator, animationDuration. Ensure this matches the constructor signature of DoublePairableBlockActorFixedInvMenuType.


🏁 Script executed:

#!/bin/bash
# Description: Verify the constructor signature of DoublePairableBlockActorFixedInvMenuType
# Expected: Constructor parameters should match the order used in build() method

rg -A 10 "class DoublePairableBlockActorFixedInvMenuType" src/form/ChestAPI/type/

Length of output: 2611


No action needed: build() parameter order matches constructor

Verified that the arguments passed in build()—block, size, blockActorId (tile_id), networkTranslator, animationDuration—align exactly with the constructor signature of DoublePairableBlockActorFixedInvMenuType. All parameters are in the correct order and no changes are required.

src/form/ChestAPI/type/graphic/InvMenuGraphic.php (1)

11-28: Excellent interface design with comprehensive functionality.

The interface provides a clean contract for inventory menu graphics with all essential operations:

  • Display management (send, remove)
  • Inventory handling (sendInventory)
  • Network translation (getNetworkTranslator)
  • Animation timing (getAnimationDuration)

The documentation for getAnimationDuration is particularly well-written, clearly explaining the purpose and units.

src/form/ChestAPI/inventory/SharedInventorySynchronizer.php (1)

11-15: LGTM! Good use of readonly properties.

The constructor design with readonly properties ensures immutability and thread safety.

src/form/ChestAPI/transaction/DeterministicInvMenuTransaction.php (2)

14-27: Excellent implementation of the decorator pattern.

The class correctly implements immutability by throwing exceptions on state-changing operations while delegating data access to the inner transaction. The error messages are clear and descriptive.


29-31:

Details

❓ Verification inconclusive

Add type signature for Closure parameter to fix static analysis warning.

PHPStan flags that the $callback parameter lacks a proper Closure signature specification.

-public function then(?Closure $callback) : void{
+public function then(?\Closure(): void $callback) : void{
    $this->result->then($callback);
}

🏁 Script executed:

#!/bin/bash
# Verify the expected Closure signature by examining the InvMenuTransactionResult class
ast-grep --pattern 'class InvMenuTransactionResult {
  $$$
  then($$$) {
    $$$
  }
  $$$
}'

Length of output: 95


To verify the actual signature of the then method on InvMenuTransactionResult, let’s locate its file and inspect the method definition:


🏁 Script executed:

#!/bin/bash
# Locate the InvMenuTransactionResult class and show its `then` method signature
file=$(rg -l "class InvMenuTransactionResult" -g '*.php')
echo "Found in: $file"
rg -n "function then" -C3 "$file"

Length of output: 563


Align then() Closure signature with InvMenuTransactionResult

To satisfy PHPStan and mirror the expected (Player): void signature in InvMenuTransactionResult, update the method in DeterministicInvMenuTransaction.php:

• File: src/form/ChestAPI/transaction/DeterministicInvMenuTransaction.php

- public function then(?Closure $callback) : void {
+ public function then(?\Closure(Player): void $callback) : void {
     $this->result->then($callback);
 }

– Make sure your PHP version supports Closure signatures and re-run PHPStan to confirm the warning is resolved.

🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 29-29:
Method pocketmine\form\ChestAPI\transaction\DeterministicInvMenuTransaction::then() has parameter $callback with no signature specified for Closure.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 29-29:
Method pocketmine\form\ChestAPI\transaction\DeterministicInvMenuTransaction::then() has parameter $callback with no signature specified for Closure.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 29-29:
Method pocketmine\form\ChestAPI\transaction\DeterministicInvMenuTransaction::then() has parameter $callback with no signature specified for Closure.

🪛 PHPStan (2.1.15)

29-29: Method pocketmine\form\ChestAPI\transaction\DeterministicInvMenuTransaction::then() has parameter $callback with no signature specified for Closure.

(missingType.callable)

src/form/ChestAPI/InvMenuHandler.php (1)

20-29: Well-implemented registration pattern with proper validation.

The registration method correctly validates single registration and properly initializes all components. The event listener registration is appropriately handled.

src/form/ChestAPI/type/ActorFixedInvMenuType.php (2)

25-31: Excellent constructor design with comprehensive documentation.

The constructor uses readonly properties effectively and provides clear parameter documentation. The design promotes immutability and type safety.


33-43: Clean and correct implementation of interface methods.

All interface methods are implemented correctly with appropriate return types and logic.

🧰 Tools
🪛 PHPStan (2.1.15)

37-37: Method pocketmine\form\ChestAPI\type\ActorFixedInvMenuType::createGraphic() never returns null so it can be removed from the return type.

(return.unusedType)

src/form/ChestAPI/type/InvMenuTypeRegistry.php (3)

41-48: Well-designed registry with proper cleanup.

The bidirectional mapping using spl_object_id() and proper cleanup of old entries when re-registering is a solid design pattern. The cleanup prevents memory leaks and maintains consistency.


1-72: Address PHP CS Fixer formatting issues.

The pipeline indicates formatting issues that need to be resolved, including missing license header, use statement ordering, and missing newline at end of file.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-93: PHP CS Fixer formatting issues detected. Added license header, reordered use statements, and added missing newline at end of file.


19-39: Well-designed constructor with proper menu type initialization.

The constructor properly initializes the registry with standard Minecraft inventory types using the builder pattern. The use of fluent interfaces makes the configuration clear and maintainable.

src/form/ChestAPI/type/util/builder/ActorFixedInvMenuTypeBuilder.php (8)

11-14: Excellent use of trait composition with conflict resolution.

The trait composition with method aliasing is a clean solution for avoiding naming conflicts while maintaining access to parent functionality. This design allows for proper method chaining and metadata management.


23-33: Proper method chaining with metadata synchronization.

The override methods correctly call the parent implementations and synchronize the entity metadata with the builder state. This ensures consistency between the builder's internal state and the actor metadata.


1-38: Address PHP CS Fixer formatting issues.

The pipeline indicates formatting issues including missing license header and missing newline at end of file.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-57: PHP CS Fixer formatting issues detected. Added license header and added missing newline at end of file.


35-37:

Details

❓ Verification inconclusive

Verify getActorMetadata()->getAll() returns correct format.

Ensure that getActorMetadata()->getAll() returns the expected array format that ActorFixedInvMenuType constructor expects for the $actor_metadata parameter.


🏁 Script executed:

#!/bin/bash
# Description: Verify the ActorFixedInvMenuType constructor signature and metadata format
ast-grep --pattern 'class ActorFixedInvMenuType {
  $$$
  public function __construct($$$) {
    $$$
  }
  $$$
}'

Length of output: 115


Let's locate the ActorFixedInvMenuType class definition and inspect its constructor signature:


🏁 Script executed:

#!/bin/bash
# Find where ActorFixedInvMenuType is defined
rg "class ActorFixedInvMenuType" -n

Length of output: 296


Inspecting the constructor in ActorFixedInvMenuType.php to confirm the expected type for the metadata parameter:


🏁 Script executed:

#!/bin/bash
# Show __construct signature and docblock in ActorFixedInvMenuType.php
rg "public function __construct" -n src/form/ChestAPI/type/ActorFixedInvMenuType.php -B3 -A5

Length of output: 491


Inspecting the builder’s getActorMetadata() method to confirm its return type and the getAll() signature:


🏁 Script executed:

#!/bin/bash
# Show getActorMetadata() method signature in the builder
rg "function getActorMetadata" -n src/form/ChestAPI/type/util/builder/ActorFixedInvMenuTypeBuilder.php -B2 -A5

Length of output: 112


Gathering getActorMetadata() implementation and verifying getAll() return type:


🏁 Script executed:

#!/bin/bash
# Show getActorMetadata() definition with surrounding lines
rg "getActorMetadata" -n src/form/ChestAPI/type/util/builder/ActorFixedInvMenuTypeBuilder.php -C3

# Locate ActorMetadata class file
rg "class ActorMetadata" -n

# Inspect getAll() method in ActorMetadata
rg "public function getAll" -n src/form/ChestAPI/type/ActorMetadata.php -B3 -A5

Length of output: 1432


Locating the GraphicNetworkTranslatableInvMenuTypeBuilderTrait to inspect getActorMetadata():


🏁 Script executed:

#!/bin/bash
# Find the trait that provides getActorMetadata()
rg "trait GraphicNetworkTranslatableInvMenuTypeBuilderTrait" -n

Length of output: 215


Inspecting the trait to see how getActorMetadata() is implemented:


🏁 Script executed:

#!/bin/bash
# Print the trait to inspect getActorMetadata() and its return type
sed -n '1,200p' src/form/ChestAPI/type/util/builder/GraphicNetworkTranslatableInvMenuTypeBuilderTrait.php

Length of output: 1382


Inspecting the builder class and locating the ActorMetadata definition for its getAll() return type:


🏁 Script executed:

#!/bin/bash
# Print the ActorFixedInvMenuTypeBuilder class to find getActorMetadata()
sed -n '1,200p' src/form/ChestAPI/type/util/builder/ActorFixedInvMenuTypeBuilder.php

# Locate ActorMetadata class in the repo
rg "class ActorMetadata" -n

Length of output: 1646


Verify metadata array format from getActorMetadata()->getAll()

Ensure the value passed as the third argument to ActorFixedInvMenuType::__construct is an array matching the expected metadata format.

• In ActorFixedInvMenuTypeBuilder::build()
src/form/ChestAPI/type/util/builder/ActorFixedInvMenuTypeBuilder.php:35

return new ActorFixedInvMenuType(
    $this->getActorIdentifier(),
    $this->getActorRuntimeIdentifier(),
    $this->getActorMetadata()->getAll(),  // ← must be a correctly structured array
    $this->getSize(),
    $this->getGraphicNetworkTranslator()
);

• Constructor signature in ActorFixedInvMenuType
src/form/ChestAPI/type/ActorFixedInvMenuType.php:25–30

public function __construct(
    string $actor_identifier,
    int    $actor_runtime_identifier,
    array  $actor_metadata,              // ← expects an array
    int    $size,
    ?InvMenuGraphicNetworkTranslator $network_translator = null
) { … }

Please manually confirm that getActorMetadata()->getAll() returns an array of metadata entries in the correct key–value structure.


11-14: Excellent use of trait composition with proper conflict resolution.

The trait usage with method aliasing (setSize as parentSetSize, setNetworkWindowType as parentSetNetworkWindowType) elegantly resolves potential method conflicts while maintaining clear delegation patterns.


16-21: Proper entity metadata initialization for invisible menu entities.

The constructor correctly sets up the actor metadata for an invisible entity with minimal bounding box dimensions (0.01), which is appropriate for inventory menu actors that shouldn't be visually intrusive.


23-33: Well-implemented method overrides with proper metadata synchronization.

Both setNetworkWindowType() and setSize() methods correctly delegate to their parent trait methods while keeping the entity metadata in sync. This ensures consistency between the builder state and the actor's runtime properties.


35-37: Comprehensive build method with all required parameters.

The build() method correctly constructs the ActorFixedInvMenuType with all necessary parameters from the builder's state, including metadata conversion via getAll().

src/form/ChestAPI/inventory/SharedInventoryNotifier.php (6)

18-22: Well-designed synchronization pattern prevents recursive updates.

The temporary removal of the synchronizer listener during content updates is a clean solution to prevent infinite recursion while maintaining inventory synchronization.


24-30: Good defensive programming with bounds checking.

The slot bounds checking ($slot < $inventory->getSize()) prevents potential out-of-bounds errors when synchronizing slot changes. This is a solid defensive programming practice.


1-31: Address PHP CS Fixer formatting issues.

The pipeline indicates missing license header formatting issue.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-28: PHP CS Fixer formatting issues detected. Added license header.


18-22: Effective synchronization pattern to prevent recursive updates.

The approach of temporarily removing the synchronizer listener during content updates is well-designed and prevents infinite recursion while maintaining proper synchronization between inventories.


24-30: Good bounds checking and slot synchronization logic.

The slot change handler properly validates the slot index before performing updates and uses the same pattern of temporarily removing the synchronizer to prevent recursive notifications.


13-16: Clean dependency injection with readonly properties.

The constructor uses readonly properties for immutable dependencies, which is a good practice for ensuring the notifier's state cannot be accidentally modified.

src/form/ChestAPI/transaction/InvMenuTransactionResult.php (9)

10-17: Clean design with modern PHP features.

The use of readonly properties and proper type hints creates a clean, immutable transaction result. The optional callback mechanism is well-designed for handling post-transaction actions.


19-25: Proper deprecation handling with clear migration path.

The deprecated method includes clear documentation directing users to the preferred direct property access. This provides a smooth migration path while maintaining backward compatibility.


36-39: Fluent interface design enables method chaining.

The then() method returns self enabling fluent method chaining, which improves the developer experience when configuring transaction results.


28-35: Excellent documentation explains the callback purpose.

The detailed comment explaining when and why the callback is useful (escaping event and network stack traces) provides valuable context for developers using this API.


1-48: Address PHP CS Fixer formatting issues.

The pipeline indicates formatting issues including missing license header and redundant docblock annotations.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-66: PHP CS Fixer formatting issues detected. Added license header and removed redundant docblock annotations.


15-17: Clean constructor design with readonly property.

Using a readonly public property for the cancellation status is a modern, clean approach that provides immutability while allowing direct access.


36-39: Well-designed fluent callback mechanism.

The then() method provides a clean fluent interface for setting post-transaction callbacks, with proper type hints and method chaining support.


19-25: Proper backwards compatibility with clear deprecation notices.

The deprecated methods are properly marked and provide clear migration guidance, ensuring smooth transitions for existing code while encouraging adoption of the new direct property access pattern.

Also applies to: 41-47


27-35: Excellent documentation for callback usage.

The comprehensive comment explaining when and why the callback is useful (escaping event and network stack traces) provides valuable context for developers using this API.

src/form/ChestAPI/type/util/InvMenuTypeHelper.php (3)

16-17: LGTM: World bounds constants are appropriate.

The Y coordinate bounds align with Minecraft's world height limits (-64 to 320), which is correct for modern Minecraft versions.


19-26: LGTM: Position offset calculation looks correct.

The logic properly calculates a position behind the player using the direction vector and player size dimensions.


28-30: LGTM: Y coordinate validation is straightforward and correct.

Simple bounds checking against the defined constants.

src/form/ChestAPI/type/graphic/MultiBlockInvMenuGraphic.php (5)

18-20: LGTM: Constructor is simple and appropriate.

Takes an array of graphics and stores them as readonly, which is correct for a composite.


22-29: LGTM: Good error handling for empty graphics array.

The first() method properly validates that graphics exist and provides a clear error message.


31-35: LGTM: Correct delegation for send operation.

Broadcasting the send operation to all graphics is the right approach for a composite.


37-39: LGTM: Using first graphic for inventory operations is logical.

Since all graphics in the composite represent the same logical inventory, using the first one for inventory operations makes sense.


55-64: LGTM: Animation duration calculation is well-implemented.

Taking the maximum duration from all graphics ensures the composite animation completes properly.

src/form/ChestAPI/type/graphic/ActorInvMenuGraphic.php (4)

26-32: LGTM: Constructor parameters are well-designed.

The readonly properties provide good encapsulation, and the optional parameters have sensible defaults.


34-54: LGTM: Actor packet creation is comprehensive.

The AddActorPacket::create() call includes all necessary parameters, and the conditional nametag override logic is well-implemented.


56-58: LGTM: Inventory delegation is appropriate.

Delegating to setCurrentWindow() is the correct way to handle inventory display.


60-62: LGTM: Actor removal is straightforward.

Sending RemoveActorPacket with the runtime identifier is the correct cleanup approach.

src/form/ChestAPI/type/util/builder/BlockActorFixedInvMenuTypeBuilder.php (5)

11-16: LGTM: Good use of traits for shared builder functionality.

The trait composition provides reusable building blocks for different menu type builders.


19-21: LGTM: Constructor properly initializes default network translator.

Adding the BlockInvMenuGraphicNetworkTranslator::instance() by default is a sensible default for block actor menus.


23-26: LGTM: Fluent interface implementation is correct.

The method properly sets the property and returns $this for method chaining.


28-30: LGTM: Good validation with clear error message.

The null coalescing with exception provides clear feedback when required properties aren't set.


32-34: LGTM: Build method correctly assembles all components.

All parameters from traits and the local property are properly passed to create the BlockActorFixedInvMenuType instance.

src/form/ChestAPI/InvMenu.php (3)

56-67: Constructor and destructor implementation looks good!

The constructor properly validates handler registration, initializes the inventory, and the destructor ensures proper cleanup.


122-131: Excellent synchronizer lifecycle management!

The setInventory() method properly destroys the old synchronizer before creating a new one, preventing resource leaks.


188-191: Clean transaction handling implementation!

The method elegantly handles both cases - with and without a listener - using the null coalescing pattern.

}
return $clientData;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix trailing formatting issue on closing brace.

The closing brace appears to have formatting issues flagged by PHP CS Fixer.

Apply this diff to fix the formatting:

-	}	
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
}
}
🤖 Prompt for AI Agents
In src/network/mcpe/handler/LoginPacketHandler.php at line 213, there is a
trailing formatting issue on the closing brace. Remove any extra spaces or
characters after the closing brace to ensure it conforms to PHP CS Fixer
standards, making sure the brace is properly aligned and followed by a newline
if required.

Comment on lines +196 to +201

// Ensure GraphicsMode exists
if (!isset($clientDataClaims["GraphicsMode"])) {
$clientDataClaims["GraphicsMode"] = 1;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix formatting issues flagged by PHP CS Fixer.

The logic change to ensure GraphicsMode exists is good defensive programming for backward compatibility with Minecraft: Bedrock Edition 1.21.80. However, there are formatting issues that need to be addressed.

Apply this diff to fix the formatting issues:

-	
-	// Ensure GraphicsMode exists
-	if (!isset($clientDataClaims["GraphicsMode"])) {
-		$clientDataClaims["GraphicsMode"] = 1;
-	}
-	
+
+		// Ensure GraphicsMode exists
+		if (!isset($clientDataClaims["GraphicsMode"])) {
+			$clientDataClaims["GraphicsMode"] = 1;
+		}
+

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/network/mcpe/handler/LoginPacketHandler.php around lines 196 to 201, the
code block ensuring GraphicsMode exists has formatting issues. Adjust the
indentation and spacing to comply with PHP CS Fixer standards, ensuring
consistent use of spaces, proper alignment of braces, and no extra blank lines
within the block.


interface PlayerNetworkHandler{

public function createNetworkStackLatencyEntry(Closure $then) : NetworkStackLatencyEntry;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Specify the Closure parameter signature for better type safety.

PHPStan is flagging that the $then Closure parameter lacks a signature specification. Based on the NetworkStackLatencyEntry usage pattern, consider specifying the Closure signature to improve type safety and documentation.

-	public function createNetworkStackLatencyEntry(Closure $then) : NetworkStackLatencyEntry;
+	/**
+	 * @param Closure(): void $then
+	 */
+	public function createNetworkStackLatencyEntry(Closure $then) : NetworkStackLatencyEntry;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function createNetworkStackLatencyEntry(Closure $then) : NetworkStackLatencyEntry;
/**
* @param Closure(): void $then
*/
public function createNetworkStackLatencyEntry(Closure $then) : NetworkStackLatencyEntry;
🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 12-12:
Method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 12-12:
Method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 12-12:
Method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

🪛 PHPStan (2.1.15)

12-12: Method pocketmine\form\ChestAPI\session\network\handler\PlayerNetworkHandler::createNetworkStackLatencyEntry() has parameter $then with no signature specified for Closure.

(missingType.callable)

🤖 Prompt for AI Agents
In src/form/ChestAPI/session/network/handler/PlayerNetworkHandler.php at line
12, the createNetworkStackLatencyEntry method's Closure parameter $then lacks a
specified signature, causing PHPStan to flag it. Update the method declaration
to specify the Closure's parameter and return types according to how $then is
used with NetworkStackLatencyEntry, enhancing type safety and clarity.

Comment on lines +1 to +14
<?php

declare(strict_types=1);

namespace pocketmine\form\ChestAPI\type\graphic\network;

use pocketmine\form\ChestAPI\session\InvMenuInfo;
use pocketmine\form\ChestAPI\session\PlayerSession;
use pocketmine\network\mcpe\protocol\ContainerOpenPacket;

interface InvMenuGraphicNetworkTranslator{

public function translate(PlayerSession $session, InvMenuInfo $current, ContainerOpenPacket $packet) : void;
} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Address the pipeline failure: missing license header.

The PHP CS Fixer detected that a license header is missing from this file.


🏁 Script executed:

#!/bin/bash
# Description: Check if other files in the project have license headers to understand the expected format.

# Find existing PHP files and check for license headers
fd -e php -x head -n 20 {} \; | grep -A 15 -B 5 "license\|copyright\|author" || echo "No license headers found in sample files"

# Check if there's a specific license header template in the project
fd -t f -e txt -e md | xargs grep -l "license\|copyright" | head -5

Length of output: 65667


Add missing PocketMine license header

The interface file must include the standard PocketMine LGPL header (with ASCII art and copyright notice) before the declare(strict_types=1); line, matching other PHP files in the project.

• File to update:

  • src/form/ChestAPI/type/graphic/network/InvMenuGraphicNetworkTranslator.php

• Apply this diff (license block copied from any existing PocketMine PHP file):

 <?php

+/*
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link   http://www.pocketmine.net/
+ *
+ */
 declare(strict_types=1);

After adding this header, the pipeline should pass.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-34: PHP CS Fixer formatting issues detected. Added license header.

🤖 Prompt for AI Agents
In src/form/ChestAPI/type/graphic/network/InvMenuGraphicNetworkTranslator.php at
lines 1 to 14, the file is missing the standard PocketMine LGPL license header.
Add the full PocketMine license header block, including the ASCII art and
copyright notice, at the very top of the file before the
declare(strict_types=1); statement to comply with project licensing standards
and fix the pipeline failure.

Comment on lines +15 to +17
public function __construct(
readonly private Closure $creator
){}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add explicit closure signature for better type safety.

The constructor parameter $creator has a PHPDoc signature but PHPStan correctly flags that more explicit typing would improve type safety. Consider using more specific typing if supported by your PHP version.

🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 15-15:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::__construct() has parameter $creator with no signature specified for Closure.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 15-15:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::__construct() has parameter $creator with no signature specified for Closure.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 15-15:
Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::__construct() has parameter $creator with no signature specified for Closure.

🪛 PHPStan (2.1.15)

15-15: Method pocketmine\form\ChestAPI\session\network\handler\ClosurePlayerNetworkHandler::__construct() has parameter $creator with no signature specified for Closure.

(missingType.callable)

🤖 Prompt for AI Agents
In src/form/ChestAPI/session/network/handler/ClosurePlayerNetworkHandler.php at
lines 15 to 17, the constructor's Closure parameter $creator lacks an explicit
closure signature, reducing type safety. Update the constructor to specify the
exact closure signature expected, such as parameter and return types, using
PHP's callable type hinting or a more specific Closure type declaration
supported by your PHP version to improve static analysis and type safety.

Comment on lines +47 to +53
public function send(Player $player, ?string $name) : void{
$this->block_graphic->send($player, $name);
if($name !== null){
$this->tile->setString(Nameable::TAG_CUSTOM_NAME, $name);
}
$player->getNetworkSession()->sendDataPacket(BlockActorDataPacket::create(BlockPosition::fromVector3($this->position), new CacheableNbt($this->tile)));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Potential mutation of shared CompoundTag instance.

The method modifies the tile property's content, which could cause issues if the same CompoundTag instance is shared across multiple BlockActorInvMenuGraphic instances. Consider cloning the tag before modification to preserve immutability.

 public function send(Player $player, ?string $name) : void{
     $this->block_graphic->send($player, $name);
+    $tile_to_send = clone $this->tile;
     if($name !== null){
-        $this->tile->setString(Nameable::TAG_CUSTOM_NAME, $name);
+        $tile_to_send->setString(Nameable::TAG_CUSTOM_NAME, $name);
     }
-    $player->getNetworkSession()->sendDataPacket(BlockActorDataPacket::create(BlockPosition::fromVector3($this->position), new CacheableNbt($this->tile)));
+    $player->getNetworkSession()->sendDataPacket(BlockActorDataPacket::create(BlockPosition::fromVector3($this->position), new CacheableNbt($tile_to_send)));
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function send(Player $player, ?string $name) : void{
$this->block_graphic->send($player, $name);
if($name !== null){
$this->tile->setString(Nameable::TAG_CUSTOM_NAME, $name);
}
$player->getNetworkSession()->sendDataPacket(BlockActorDataPacket::create(BlockPosition::fromVector3($this->position), new CacheableNbt($this->tile)));
}
public function send(Player $player, ?string $name) : void{
$this->block_graphic->send($player, $name);
$tile_to_send = clone $this->tile;
if($name !== null){
$tile_to_send->setString(Nameable::TAG_CUSTOM_NAME, $name);
}
$player->getNetworkSession()->sendDataPacket(BlockActorDataPacket::create(BlockPosition::fromVector3($this->position), new CacheableNbt($tile_to_send)));
}
🤖 Prompt for AI Agents
In src/form/ChestAPI/type/graphic/BlockActorInvMenuGraphic.php around lines 47
to 53, the send method modifies the shared tile CompoundTag directly, which
risks unintended side effects if the tag is shared among instances. To fix this,
clone the tile CompoundTag before setting the custom name to ensure immutability
and prevent mutation of the shared instance.

Comment on lines +51 to +52
protected ?Closure $listener = null;
protected ?Closure $inventory_close_listener = null;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add proper type signatures for Closure properties.

The Closure properties lack type signatures, which reduces type safety and causes static analysis warnings.

Apply this diff to add proper type signatures:

-protected ?Closure $listener = null;
-protected ?Closure $inventory_close_listener = null;
+/** @var (Closure(InvMenuTransaction): InvMenuTransactionResult)|null */
+protected ?Closure $listener = null;
+/** @var (Closure(Player, Inventory): void)|null */
+protected ?Closure $inventory_close_listener = null;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected ?Closure $listener = null;
protected ?Closure $inventory_close_listener = null;
/** @var (Closure(InvMenuTransaction): InvMenuTransactionResult)|null */
protected ?Closure $listener = null;
/** @var (Closure(Player, Inventory): void)|null */
protected ?Closure $inventory_close_listener = null;
🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 52-52:
Property pocketmine\form\ChestAPI\InvMenu::$inventory_close_listener type has no signature specified for Closure.


[failure] 51-51:
Property pocketmine\form\ChestAPI\InvMenu::$listener type has no signature specified for Closure.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 52-52:
Property pocketmine\form\ChestAPI\InvMenu::$inventory_close_listener type has no signature specified for Closure.


[failure] 51-51:
Property pocketmine\form\ChestAPI\InvMenu::$listener type has no signature specified for Closure.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 52-52:
Property pocketmine\form\ChestAPI\InvMenu::$inventory_close_listener type has no signature specified for Closure.


[failure] 51-51:
Property pocketmine\form\ChestAPI\InvMenu::$listener type has no signature specified for Closure.

🪛 PHPStan (2.1.15)

51-51: Property pocketmine\form\ChestAPI\InvMenu::$listener type has no signature specified for Closure.

(missingType.callable)


52-52: Property pocketmine\form\ChestAPI\InvMenu::$inventory_close_listener type has no signature specified for Closure.

(missingType.callable)

🤖 Prompt for AI Agents
In src/form/ChestAPI/InvMenu.php around lines 51 to 52, the Closure properties
$listener and $inventory_close_listener lack explicit type signatures, reducing
type safety and causing static analysis warnings. Update the property
declarations to include proper type signatures by specifying the expected
parameter and return types for each Closure, ensuring they are accurately typed
to improve code safety and analysis.

Comment on lines +31 to +33
public static function create(string $identifier, ...$args) : InvMenu{
return new InvMenu(InvMenuHandler::getTypeRegistry()->get($identifier), ...$args);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix the type mismatch in the create() method.

The create() method passes variadic arguments to the constructor, but the constructor expects a specific signature (InvMenuType $type, ?Inventory $custom_inventory = null). This could lead to runtime errors if incorrect arguments are passed.

Apply this diff to fix the type safety:

-public static function create(string $identifier, ...$args) : InvMenu{
-	return new InvMenu(InvMenuHandler::getTypeRegistry()->get($identifier), ...$args);
+public static function create(string $identifier, ?Inventory $custom_inventory = null) : InvMenu{
+	return new InvMenu(InvMenuHandler::getTypeRegistry()->get($identifier), $custom_inventory);
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public static function create(string $identifier, ...$args) : InvMenu{
return new InvMenu(InvMenuHandler::getTypeRegistry()->get($identifier), ...$args);
}
public static function create(string $identifier, ?Inventory $custom_inventory = null) : InvMenu {
return new InvMenu(
InvMenuHandler::getTypeRegistry()->get($identifier),
$custom_inventory
);
}
🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 32-32:
Parameter #2 $custom_inventory of class pocketmine\form\ChestAPI\InvMenu constructor expects pocketmine\inventory\Inventory|null, mixed given.

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 32-32:
Parameter #2 $custom_inventory of class pocketmine\form\ChestAPI\InvMenu constructor expects pocketmine\inventory\Inventory|null, mixed given.

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 32-32:
Parameter #2 $custom_inventory of class pocketmine\form\ChestAPI\InvMenu constructor expects pocketmine\inventory\Inventory|null, mixed given.

🪛 PHPStan (2.1.15)

32-32: Parameter #2 $custom_inventory of class pocketmine\form\ChestAPI\InvMenu constructor expects pocketmine\inventory\Inventory|null, mixed given.

(argument.type)

🤖 Prompt for AI Agents
In src/form/ChestAPI/InvMenu.php around lines 31 to 33, the create() method
incorrectly passes variadic arguments to the InvMenu constructor, which expects
a specific signature with an InvMenuType and an optional Inventory. To fix this,
change the method to accept only the expected parameters and pass them
explicitly to the constructor, ensuring the first argument is an InvMenuType
obtained from the registry and the second is an optional Inventory, removing the
use of variadic arguments to maintain type safety.

Comment on lines +1 to +4
<?php

declare(strict_types=1);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Add the missing license header.

The pipeline failure indicates that a license header is required for this file.


🏁 Script executed:

#!/bin/bash
# Check if other PHP files in the project have license headers
fd -e php -x head -n 10 {} \; | grep -A 5 "<?php" | head -50

Length of output: 1184


🏁 Script executed:

#!/bin/bash
# Display the top lines of a representative PHP file to capture the standard license header
fd -e php | grep -v 'src/form/ChestAPI/InvMenu.php' | head -n1 | xargs head -n20

Length of output: 788


Add the standard PocketMine LGPL header to src/form/ChestAPI/InvMenu.php

The new file is missing the project’s required license header. At the top of InvMenu.php, insert the following (before declare(strict_types=1);):

 <?php
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ */
+
 declare(strict_types=1);
  • File: src/form/ChestAPI/InvMenu.php
  • Insert lines 2–16 as shown above.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<?php
declare(strict_types=1);
<?php
/*
*
* ____ _ _ __ __ _ __ __ ____
* | _ \ ___ ___| | _____| |_| \/ (_)_ __ ___ | \/ | _ \
* | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
* | __/ (_) | (__| < __/ |_| | | | | | | | __/_____| | | | __/
* |_| \___/ \___|_|\_\___|\__|_| |_|_|_| |_|\___| |_| |_|_|
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* @author PocketMine Team
* @link http://www.pocketmine.net/
*
*/
declare(strict_types=1);
🧰 Tools
🪛 GitHub Actions: CI

[error] 1-44: PHP CS Fixer formatting issues detected. Added license header and removed redundant return type annotations.

🤖 Prompt for AI Agents
In src/form/ChestAPI/InvMenu.php at the top of the file before line 4, add the
standard PocketMine LGPL license header as found in other PHP files in the
project. Insert the license header lines between the opening <?php tag and the
declare(strict_types=1); statement, covering lines 2 to 16, to comply with
project licensing requirements and fix the pipeline failure.

Comment on lines +58 to +60
public function getIdentifier(InvMenuType $type) : string{
return $this->identifiers[spl_object_id($type)];
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add bounds checking to prevent undefined array key errors.

The getIdentifier() method should verify that the type exists in the reverse mapping before accessing the array.

 public function getIdentifier(InvMenuType $type) : string{
+	$objectId = spl_object_id($type);
+	if (!isset($this->identifiers[$objectId])) {
+		throw new \InvalidArgumentException("Type is not registered in this registry");
+	}
-	return $this->identifiers[spl_object_id($type)];
+	return $this->identifiers[$objectId];
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function getIdentifier(InvMenuType $type) : string{
return $this->identifiers[spl_object_id($type)];
}
public function getIdentifier(InvMenuType $type) : string{
$objectId = spl_object_id($type);
if (!isset($this->identifiers[$objectId])) {
throw new \InvalidArgumentException("Type is not registered in this registry");
}
return $this->identifiers[$objectId];
}
🤖 Prompt for AI Agents
In src/form/ChestAPI/type/InvMenuTypeRegistry.php around lines 58 to 60, the
getIdentifier() method accesses the identifiers array using spl_object_id($type)
without checking if the key exists, which can cause undefined array key errors.
Add a check to verify that the key exists in the identifiers array before
returning it, and handle the case where it does not exist, for example by
throwing an exception or returning a default value.

KanadeBlue and others added 2 commits June 6, 2025 18:37
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/scheduler/TaskScheduler.php (1)

10-10: Remove unused import.

The TextFormat class is imported but never used in this file.

 use pocketmine\player\Player;
-use pocketmine\utils\TextFormat;
 use pocketmine\utils\UUID;
🧹 Nitpick comments (1)
src/scheduler/TaskScheduler.php (1)

191-193: Add type safety for queue current() call.

While the logic is correct (isEmpty check precedes current() call), consider adding explicit type assertion to address static analysis concerns and improve code clarity.

 private function isReady(int $currentTick): bool {
-    return !$this->queue->isEmpty() && $this->queue->current()->getNextRun() <= $currentTick;
+    if ($this->queue->isEmpty()) {
+        return false;
+    }
+    $current = $this->queue->current();
+    assert($current instanceof TaskHandler);
+    return $current->getNextRun() <= $currentTick;
 }
🧰 Tools
🪛 PHPStan (2.1.15)

192-192: Cannot call method getNextRun() on array<string, int|pocketmine\scheduler\TaskHandler>|int|pocketmine\scheduler\TaskHandler.

(method.nonObject)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67a9164 and ca3c452.

📒 Files selected for processing (2)
  • src/form/FormAPI/Form.php (1 hunks)
  • src/scheduler/TaskScheduler.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/form/FormAPI/Form.php
🧰 Additional context used
🪛 PHPStan (2.1.15)
src/scheduler/TaskScheduler.php

17-17: Property pocketmine\scheduler\TaskScheduler::$queue with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


23-23: Property pocketmine\scheduler\TaskScheduler::$tasks with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


28-28: Property pocketmine\scheduler\TaskScheduler::$playerTasks with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


38-38: Method pocketmine\scheduler\TaskScheduler::scheduleTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


45-45: Method pocketmine\scheduler\TaskScheduler::scheduleDelayedTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


52-52: Method pocketmine\scheduler\TaskScheduler::scheduleRepeatingTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


59-59: Method pocketmine\scheduler\TaskScheduler::scheduleDelayedRepeatingTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


76-76: Method pocketmine\scheduler\TaskScheduler::isQueued() has parameter $task with generic class pocketmine\scheduler\TaskHandler but does not specify its types: TTask

(missingType.generics)


90-90: Method pocketmine\scheduler\TaskScheduler::scheduleForPlayer() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


118-118: Method pocketmine\scheduler\TaskScheduler::addTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


132-132: Method pocketmine\scheduler\TaskScheduler::handle() has parameter $handler with generic class pocketmine\scheduler\TaskHandler but does not specify its types: TTask

(missingType.generics)


132-132: Method pocketmine\scheduler\TaskScheduler::handle() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


168-168: PHPDoc tag @var for variable $task contains generic class pocketmine\scheduler\TaskHandler but does not specify its types: TTask

(missingType.generics)


192-192: Cannot call method getNextRun() on array<string, int|pocketmine\scheduler\TaskHandler>|int|pocketmine\scheduler\TaskHandler.

(method.nonObject)

🪛 GitHub Actions: CI
src/scheduler/TaskScheduler.php

[error] 1-212: PHP CS Fixer formatting issues detected. Indentation and spacing inconsistencies throughout the file. Run 'php-cs-fixer fix' to correct.

🔇 Additional comments (1)
src/scheduler/TaskScheduler.php (1)

1-194:

Details

❓ Verification inconclusive

Fix formatting issues to comply with code standards.

The CI pipeline detected PHP CS Fixer formatting issues including indentation and spacing inconsistencies throughout the file.

Please run the following command to fix formatting issues:


🏁 Script executed:

#!/bin/bash
# Fix PHP CS Fixer formatting issues
php-cs-fixer fix src/scheduler/TaskScheduler.php --rules=@PSR12

Length of output: 168


Ensure PSR-12 compliance with PHP CS Fixer
The CI pipeline flagged formatting issues in src/scheduler/TaskScheduler.php (indentation, spacing, etc.), but php-cs-fixer wasn’t found in the environment. Please verify and fix formatting by one of the following approaches:

• Install or add PHP CS Fixer (e.g., composer global require friendsofphp/php-cs-fixer or include it in your project’s dev dependencies).
• Run locally:

php-cs-fixer fix src/scheduler/TaskScheduler.php --rules=@PSR12

• If you cannot run the fixer here, manually adjust the file to adhere to PSR-12 (consistent indentation, spacing, blank lines) and commit the changes.

🧰 Tools
🪛 PHPStan (2.1.15)

17-17: Property pocketmine\scheduler\TaskScheduler::$queue with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


23-23: Property pocketmine\scheduler\TaskScheduler::$tasks with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


28-28: Property pocketmine\scheduler\TaskScheduler::$playerTasks with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


38-38: Method pocketmine\scheduler\TaskScheduler::scheduleTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


45-45: Method pocketmine\scheduler\TaskScheduler::scheduleDelayedTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


52-52: Method pocketmine\scheduler\TaskScheduler::scheduleRepeatingTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


59-59: Method pocketmine\scheduler\TaskScheduler::scheduleDelayedRepeatingTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


76-76: Method pocketmine\scheduler\TaskScheduler::isQueued() has parameter $task with generic class pocketmine\scheduler\TaskHandler but does not specify its types: TTask

(missingType.generics)


90-90: Method pocketmine\scheduler\TaskScheduler::scheduleForPlayer() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


118-118: Method pocketmine\scheduler\TaskScheduler::addTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


132-132: Method pocketmine\scheduler\TaskScheduler::handle() has parameter $handler with generic class pocketmine\scheduler\TaskHandler but does not specify its types: TTask

(missingType.generics)


132-132: Method pocketmine\scheduler\TaskScheduler::handle() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


168-168: PHPDoc tag @var for variable $task contains generic class pocketmine\scheduler\TaskHandler but does not specify its types: TTask

(missingType.generics)


192-192: Cannot call method getNextRun() on array<string, int|pocketmine\scheduler\TaskHandler>|int|pocketmine\scheduler\TaskHandler.

(method.nonObject)

🪛 GitHub Actions: CI

[error] 1-212: PHP CS Fixer formatting issues detected. Indentation and spacing inconsistencies throughout the file. Run 'php-cs-fixer fix' to correct.

Comment on lines +90 to +101
public function scheduleForPlayer(Player $player, Task $task, int $ticks = 20): TaskHandler {
$uniqueId = $player->getUniqueId()->toString();

if ($this->isScheduledFor($player)) {
return $this->playerTasks[$uniqueId];
}

$taskHandler = $this->scheduleRepeatingTask($task, $ticks);
$this->playerTasks[$uniqueId] = $taskHandler;

return $taskHandler;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Clarify behavior when task already exists for player.

The current logic returns the existing TaskHandler when a task is already scheduled for a player, potentially ignoring the new task being requested. Consider if this is the intended behavior or if you should:

  1. Cancel the existing task and schedule the new one
  2. Throw an exception to prevent duplicate scheduling
  3. Document this behavior clearly

If the intention is to replace existing tasks, apply this fix:

 public function scheduleForPlayer(Player $player, Task $task, int $ticks = 20): TaskHandler {
     $uniqueId = $player->getUniqueId()->toString();

     if ($this->isScheduledFor($player)) {
-        return $this->playerTasks[$uniqueId];
+        $this->cancelForPlayer($player);
     }

     $taskHandler = $this->scheduleRepeatingTask($task, $ticks);
     $this->playerTasks[$uniqueId] = $taskHandler;

     return $taskHandler;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function scheduleForPlayer(Player $player, Task $task, int $ticks = 20): TaskHandler {
$uniqueId = $player->getUniqueId()->toString();
if ($this->isScheduledFor($player)) {
return $this->playerTasks[$uniqueId];
}
$taskHandler = $this->scheduleRepeatingTask($task, $ticks);
$this->playerTasks[$uniqueId] = $taskHandler;
return $taskHandler;
}
public function scheduleForPlayer(Player $player, Task $task, int $ticks = 20): TaskHandler {
$uniqueId = $player->getUniqueId()->toString();
if ($this->isScheduledFor($player)) {
$this->cancelForPlayer($player);
}
$taskHandler = $this->scheduleRepeatingTask($task, $ticks);
$this->playerTasks[$uniqueId] = $taskHandler;
return $taskHandler;
}
🧰 Tools
🪛 PHPStan (2.1.15)

90-90: Method pocketmine\scheduler\TaskScheduler::scheduleForPlayer() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)

🤖 Prompt for AI Agents
In src/scheduler/TaskScheduler.php around lines 90 to 101, the method
scheduleForPlayer returns the existing TaskHandler if a task is already
scheduled for the player, which may ignore the new task request. To fix this,
modify the method to first check if a task exists for the player, cancel that
existing task, then schedule the new task and update the playerTasks array
accordingly. This ensures the new task replaces the old one as intended.

Comment on lines +170 to +184
if ($task->isCancelled()) {
$this->tasks->remove($task);
continue;
}

$task->run();

if (!$task->isCancelled() && $task->isRepeating()) {
$nextRun = $this->currentTick + $task->getPeriod();
$task->setNextRun($nextRun);
$this->queue->insert($task, $nextRun);
} else {
$task->remove();
$this->tasks->remove($task);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add cleanup for completed player tasks to prevent memory leaks.

When tasks complete naturally (non-repeating tasks or cancelled repeating tasks), they should be removed from $playerTasks to prevent memory leaks.

             if (!$task->isCancelled() && $task->isRepeating()) {
                 $nextRun = $this->currentTick + $task->getPeriod();
                 $task->setNextRun($nextRun);
                 $this->queue->insert($task, $nextRun);
             } else {
                 $task->remove();
                 $this->tasks->remove($task);
+                // Clean up player task references
+                foreach ($this->playerTasks as $uuid => $playerTask) {
+                    if ($playerTask === $task) {
+                        unset($this->playerTasks[$uuid]);
+                        break;
+                    }
+                }
             }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if ($task->isCancelled()) {
$this->tasks->remove($task);
continue;
}
$task->run();
if (!$task->isCancelled() && $task->isRepeating()) {
$nextRun = $this->currentTick + $task->getPeriod();
$task->setNextRun($nextRun);
$this->queue->insert($task, $nextRun);
} else {
$task->remove();
$this->tasks->remove($task);
}
if ($task->isCancelled()) {
$this->tasks->remove($task);
continue;
}
$task->run();
if (!$task->isCancelled() && $task->isRepeating()) {
$nextRun = $this->currentTick + $task->getPeriod();
$task->setNextRun($nextRun);
$this->queue->insert($task, $nextRun);
} else {
$task->remove();
$this->tasks->remove($task);
// Clean up player task references
foreach ($this->playerTasks as $uuid => $playerTask) {
if ($playerTask === $task) {
unset($this->playerTasks[$uuid]);
break;
}
}
}
🤖 Prompt for AI Agents
In src/scheduler/TaskScheduler.php around lines 170 to 184, completed tasks are
removed from $this->tasks but not from $playerTasks, which can cause memory
leaks. Add code to also remove the task from $playerTasks when it is completed
or cancelled, ensuring all references to finished tasks are cleaned up properly.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (1)
src/block/VanillaBlocks.php (1)

868-876: Consider extracting anonymous class for clarity
The inline new class hack customizes bamboo break time but can hinder readability. Extracting it into a named subclass of BreakInfo would improve maintainability.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ca3c452 and 8e9f494.

📒 Files selected for processing (5)
  • src/block/BlockTypeIds.php (1 hunks)
  • src/block/VanillaBlocks.php (23 hunks)
  • src/data/bedrock/block/convert/BlockObjectToStateSerializer.php (1 hunks)
  • src/data/bedrock/block/convert/BlockStateToObjectDeserializer.php (1 hunks)
  • src/item/StringToItemParser.php (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/block/BlockTypeIds.php
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/data/bedrock/block/convert/BlockStateToObjectDeserializer.php (1)
src/data/bedrock/block/convert/BlockObjectToStateSerializer.php (1)
  • mapSimple (240-242)
🪛 GitHub Check: PHP 8.3 / PHPStan analysis
src/block/VanillaBlocks.php

[failure] 116-116:
PHPDoc tag @method has invalid value (static Transparent STRUCTURE_VOID name()): Unexpected token "name", expected '(' at offset 1724 on line 41

src/data/bedrock/block/convert/BlockObjectToStateSerializer.php

[failure] 973-973:
Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

src/data/bedrock/block/convert/BlockStateToObjectDeserializer.php

[failure] 890-890:
Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

🪛 GitHub Check: PHP 8.2 / PHPStan analysis
src/block/VanillaBlocks.php

[failure] 116-116:
PHPDoc tag @method has invalid value (static Transparent STRUCTURE_VOID name()): Unexpected token "name", expected '(' at offset 1724 on line 41

src/data/bedrock/block/convert/BlockObjectToStateSerializer.php

[failure] 973-973:
Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

src/data/bedrock/block/convert/BlockStateToObjectDeserializer.php

[failure] 890-890:
Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

🪛 GitHub Check: PHP 8.1 / PHPStan analysis
src/block/VanillaBlocks.php

[failure] 116-116:
PHPDoc tag @method has invalid value (static Transparent STRUCTURE_VOID name()): Unexpected token "name", expected '(' at offset 1724 on line 41

src/data/bedrock/block/convert/BlockObjectToStateSerializer.php

[failure] 973-973:
Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

src/data/bedrock/block/convert/BlockStateToObjectDeserializer.php

[failure] 890-890:
Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

🪛 PHPStan (2.1.15)
src/block/VanillaBlocks.php

116-116: PHPDoc tag @method has invalid value (static Transparent STRUCTURE_VOID name()): Unexpected token "name", expected '(' at offset 1724 on line 41

(phpDoc.parseError)

src/data/bedrock/block/convert/BlockObjectToStateSerializer.php

973-973: Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

(staticMethod.notFound)

src/data/bedrock/block/convert/BlockStateToObjectDeserializer.php

890-890: Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

(staticMethod.notFound)

src/item/StringToItemParser.php

158-158: Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

(staticMethod.notFound)

🪛 GitHub Actions: CI
src/block/VanillaBlocks.php

[error] 1-1: PHP CS Fixer formatting issues detected. Multiple function declarations missing space before colon in return type declarations.

🔇 Additional comments (8)
src/data/bedrock/block/convert/BlockObjectToStateSerializer.php (1)

973-973:

Details

❌ Incorrect review comment

Undefined static method: Blocks::STRUCTURE_VOID()
Static analysis reports that VanillaBlocks::STRUCTURE_VOID() does not exist. Please verify the correct factory method or add the missing method/constant in VanillaBlocks.


I’ll check how Blocks is imported and whether STRUCTURE_VOID is defined anywhere.


🏁 Script executed:

#!/bin/bash
# Show the top of the serializer to see its imports/aliases
sed -n '1,200p' src/data/bedrock/block/convert/BlockObjectToStateSerializer.php

# Locate any class definitions named Blocks or VanillaBlocks
rg "class\s+\w*(Blocks|VanillaBlocks)" -n src

# Search for STRUCTURE_VOID definitions or references
rg "STRUCTURE_VOID" -n src

Length of output: 7864


Ignore the undefined‐method warning for STRUCTURE_VOID()
The call to Blocks::STRUCTURE_VOID() is valid—Blocks is an alias for VanillaBlocks, which implements __callStatic and includes a @method static Transparent STRUCTURE_VOID() annotation to generate that factory. You can safely disregard the static-analysis error here.

Likely an incorrect or invalid review comment.

🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 973-973:
Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 973-973:
Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 973-973:
Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

🪛 PHPStan (2.1.15)

973-973: Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

(staticMethod.notFound)

src/block/VanillaBlocks.php (7)

815-815: Final class declaration looks good
Using final for VanillaBlocks is appropriate to prevent subclassing registry.


884-884: Registration of structure_void is correct
Your new Transparent block with indestructible break info aligns with other void-like blocks.


1050-1050: Obsidian hardness override is accurate
Using 35.0 to match Bedrock while commenting on Java parity is clear.


1114-1124: Anonymous class override for stone drops is well implemented
The inlined class correctly handles Silk Touch and drop logic consistent with Minecraft behavior.


1231-1238: Custom Leaves break info looks good
Overriding getBreakTime for shears is concise and correctly uses the ToolType check.


1242-1249: Dynamic registration of saplings and leaves is a strong DRY improvement
Using cases() on enums reduces boilerplate and eases future block additions.


1337-1342: Reinforced Deepslate drop override is correct
Returning an empty array aligns with its unbreakable nature.

$result->registerBlock("banner", fn() => Blocks::BANNER());
$result->registerBlock("barrel", fn() => Blocks::BARREL());
$result->registerBlock("barrier", fn() => Blocks::BARRIER());
$result->registerBlock("structure_void", fn() => Blocks::STRUCTURE_VOID());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

❓ Verification inconclusive

Verify that Blocks::STRUCTURE_VOID() is implemented
Static analysis flags STRUCTURE_VOID() as undefined. Confirm that the STRUCTURE_VOID() factory method exists in VanillaBlocks (and is spelled correctly) to prevent runtime errors.

Run this script to verify the declaration:


🏁 Script executed:

#!/bin/bash
# Verify STRUCTURE_VOID() exists in VanillaBlocks
rg -n 'function STRUCTURE_VOID' src/block/VanillaBlocks.php

Length of output: 61


Implement missing STRUCTURE_VOID() factory method
Static analysis confirms no STRUCTURE_VOID() declaration in src/block/VanillaBlocks.php, so the call in StringToItemParser will fail at runtime. Please add or correct this factory method.

Locations to address:

  • src/block/VanillaBlocks.php: implement
    public static function STRUCTURE_VOID(): Block
    {
        // return a Structure Void block instance
    }
  • src/item/StringToItemParser.php (line 158): ensure you’re registering the correct factory method for “structure_void”
🧰 Tools
🪛 PHPStan (2.1.15)

158-158: Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

(staticMethod.notFound)

🤖 Prompt for AI Agents
In src/block/VanillaBlocks.php, add a public static method named
STRUCTURE_VOID() that returns an instance of the Structure Void block to fix the
missing factory method. Then, in src/item/StringToItemParser.php at line 158,
confirm that the registerBlock call uses this newly implemented STRUCTURE_VOID()
method to correctly register the "structure_void" block type.

$this->mapSimple(Ids::ANCIENT_DEBRIS, fn() => Blocks::ANCIENT_DEBRIS());
$this->mapSimple(Ids::ANDESITE, fn() => Blocks::ANDESITE());
$this->mapSimple(Ids::BARRIER, fn() => Blocks::BARRIER());
$this->mapSimple(Ids::STRUCTURE_VOID, fn() => Blocks::STRUCTURE_VOID());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Undefined static method STRUCTURE_VOID

Static analysis flags pocketmine\block\VanillaBlocks::STRUCTURE_VOID() as undefined, so this deserializer mapping will fail at runtime. Ensure the VanillaBlocks class defines a corresponding STRUCTURE_VOID() factory method or adjust to the correct method name.

Suggested diff to add the missing method in src/block/VanillaBlocks.php:

+ // Register the new STRUCTURE_VOID block
+ public static function STRUCTURE_VOID() : Block {
+     return new StructureVoid(); // replace with the actual block class
+ }

🏁 Script executed:

#!/bin/bash
set -e

# Locate the VanillaBlocks class definition
rg "class VanillaBlocks" -A3 src

# Search for any existing STRUCTURE_VOID method declaration in the block factories
rg "FUNCTION\s+STRUCTURE_VOID" -n src

# Search globally for STRUCTURE_VOID references
rg "STRUCTURE_VOID" -n src

Length of output: 251


🏁 Script executed:

#!/bin/bash
set -e

# Find the CloningRegistryTrait to inspect magic method support
rg "trait CloningRegistryTrait" -A20 src

# Search for __callStatic implementations to verify dynamic factory behavior
rg "__callStatic" -n src

Length of output: 850


🏁 Script executed:

#!/bin/bash
set -e

# List all constants defined in VanillaBlocks to check for STRUCTURE_VOID
rg -n "const " src/block/VanillaBlocks.php

Length of output: 53


🏁 Script executed:

#!/bin/bash
set -e

# List all public static methods in VanillaBlocks to check for STRUCTURE_VOID
rg "public static function" -n src/block/VanillaBlocks.php

# Display the __callStatic implementation in RegistryTrait for dynamic factory support
sed -n '90,160p' src/utils/RegistryTrait.php

Length of output: 1683


Add the missing STRUCTURE_VOID factory and registry entry

The call

$this->mapSimple(Ids::STRUCTURE_VOID, fn() => Blocks::STRUCTURE_VOID());

will fail because VanillaBlocks only defines getAll() and leans on __callStatic() for existing registry members—and no STRUCTURE_VOID entry exists.

To fix:

• In src/data/bedrock/block/Ids.php (namespace pocketmine\data\bedrock\block), add the new constant:

+    /** Block ID for Structure Void */
+    public const STRUCTURE_VOID = <correct numeric ID>;

• In src/block/VanillaBlocks.php, register and expose your new block:

+ use pocketmine\block\StructureVoid;
+
+    /**
+     * @return StructureVoid
+     */
+    public static function STRUCTURE_VOID(): Block {
+        return new StructureVoid();
+    }

Ensure you import StructureVoid at the top of the file.

After these additions, the deserializer mapping will resolve at runtime.

🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 890-890:
Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 890-890:
Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 890-890:
Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

🪛 PHPStan (2.1.15)

890-890: Call to an undefined static method pocketmine\block\VanillaBlocks::STRUCTURE_VOID().

(staticMethod.notFound)

🤖 Prompt for AI Agents
In src/data/bedrock/block/convert/BlockStateToObjectDeserializer.php at line
890, the call to Blocks::STRUCTURE_VOID() fails because the STRUCTURE_VOID
factory method does not exist in the VanillaBlocks class. To fix this, add a new
constant STRUCTURE_VOID in src/data/bedrock/block/Ids.php, then in
src/block/VanillaBlocks.php, import the StructureVoid class, register the
STRUCTURE_VOID block in the registry, and implement a corresponding static
factory method STRUCTURE_VOID() that returns an instance of StructureVoid. This
will ensure the deserializer mapping works correctly at runtime.

}

protected static function setup() : void{
protected static function setup(): void
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add space before return type colon
Fix indentation and return type formatting for CI.

- protected static function setup(): void
+ protected static function setup() : void
🤖 Prompt for AI Agents
In src/block/VanillaBlocks.php at line 861, add a space before the colon in the
return type declaration of the setup method to fix the formatting. Ensure the
method signature reads "protected static function setup() : void" with proper
spacing to comply with coding standards and pass CI checks.

* @phpstan-return array<string, Block>
*/
public static function getAll() : array{
public static function getAll(): array
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add space before return type colon
Align with project style in return type declarations.

- public static function getAll(): array
+ public static function getAll() : array
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public static function getAll(): array
public static function getAll() : array
🤖 Prompt for AI Agents
In src/block/VanillaBlocks.php at line 853, add a space before the colon in the
return type declaration of the getAll() method to align with the project's
coding style for return type declarations.

* @method static FloorBanner BANNER()
* @method static Barrel BARREL()
* @method static Transparent BARRIER()
* @method static Transparent STRUCTURE_VOID name()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix @method PHPDoc for STRUCTURE_VOID
The annotation is malformed and triggers PHPStan errors. Update to match other block entries.
Apply this diff:

- * @method static Transparent STRUCTURE_VOID name()
+ * @method static Transparent STRUCTURE_VOID()
🧰 Tools
🪛 GitHub Check: PHP 8.3 / PHPStan analysis

[failure] 116-116:
PHPDoc tag @method has invalid value (static Transparent STRUCTURE_VOID name()): Unexpected token "name", expected '(' at offset 1724 on line 41

🪛 GitHub Check: PHP 8.2 / PHPStan analysis

[failure] 116-116:
PHPDoc tag @method has invalid value (static Transparent STRUCTURE_VOID name()): Unexpected token "name", expected '(' at offset 1724 on line 41

🪛 GitHub Check: PHP 8.1 / PHPStan analysis

[failure] 116-116:
PHPDoc tag @method has invalid value (static Transparent STRUCTURE_VOID name()): Unexpected token "name", expected '(' at offset 1724 on line 41

🪛 PHPStan (2.1.15)

116-116: PHPDoc tag @method has invalid value (static Transparent STRUCTURE_VOID name()): Unexpected token "name", expected '(' at offset 1724 on line 41

(phpDoc.parseError)

🤖 Prompt for AI Agents
In src/block/VanillaBlocks.php at line 116, the @method PHPDoc annotation for
STRUCTURE_VOID is malformed and causes PHPStan errors. Update the annotation to
follow the same format as other block entries by correcting the syntax and
ensuring it properly declares the static method returning Transparent with the
correct method name.

Comment on lines +1276 to +1286
self::register("wool", fn(BID $id) => new Wool($id, "Wool", new Info(new class (0.8, ToolType::SHEARS) extends BreakInfo {
public function getBreakTime(Item $item): float
{
$time = parent::getBreakTime($item);
if ($item->getBlockToolType() === ToolType::SHEARS) {
$time *= 3; //shears break compatible blocks 15x faster, but wool 5x
}

return $time;
}

return $time;
}
})));
})));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix break time calculation for wool with shears
Multiplying by 3 reverses the intended faster break. It should divide for a 5× speed-up:

- if ($item->getBlockToolType() === ToolType::SHEARS) {
-     $time *= 3; // shears break compatible blocks 15x faster, but wool 5x
+ if ($item->getBlockToolType() === ToolType::SHEARS) {
+     $time /= 5; // wool breaks 5× faster with shears
🤖 Prompt for AI Agents
In src/block/VanillaBlocks.php around lines 1276 to 1286, the break time
calculation for wool when using shears incorrectly multiplies the break time by
3, which slows down breaking instead of speeding it up. To fix this, change the
multiplication by 3 to a division by 5 to correctly reflect the 5× faster break
speed with shears.

* @phpstan-return TBlock
*/
protected static function register(string $name, \Closure $createBlock, ?string $tileClass = null) : Block{
protected static function register(string $name, \Closure $createBlock, ?string $tileClass = null): Block
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add space before return type colon
CI formatting requires a space before : in return type declarations.
Apply:

- protected static function register(string $name, \Closure $createBlock, ?string $tileClass = null): Block
+ protected static function register(string $name, \Closure $createBlock, ?string $tileClass = null) : Block
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected static function register(string $name, \Closure $createBlock, ?string $tileClass = null): Block
protected static function register(string $name, \Closure $createBlock, ?string $tileClass = null) : Block
🤖 Prompt for AI Agents
In src/block/VanillaBlocks.php at line 830, add a space before the colon in the
return type declaration of the register method. Change the method signature from
"register(string $name, \Closure $createBlock, ?string $tileClass = null):
Block" to include a space before the colon, like "register(string $name,
\Closure $createBlock, ?string $tileClass = null) : Block" to comply with CI
formatting rules.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (6)
src/updater/UpdateCheckTask.php (3)

30-41: Check HTTP status code before decoding JSON

Internet::getURL() can return a response object even for non-200 status codes (e.g. 404 HTML page).
Attempting to json_decode() such a body will trigger the “Invalid response data” path and hide the real problem.

+        if ($response->getCode() !== 200) {
+            $this->error = "HTTP {$response->getCode()} returned by update server";
+            return;
+        }

48-54: Avoid re-using a static JsonMapper instance across worker threads

A static variable persists for the lifetime of the PHP worker thread.
If another task (possibly from a different plugin) mutates the JsonMapper configuration, it will affect all subsequent runs and can introduce hard-to-reproduce bugs.
Instantiating a fresh mapper per task keeps the async worker side-effect-free.
The small allocation cost is negligible compared to the network call.


1-24: Address PHP-CS-Fixer complaints

CI flagged style violations in this file (lines 1-24).
Running vendor/bin/php-cs-fixer fix src/updater/UpdateCheckTask.php locally and committing the result will unblock the pipeline.

🧰 Tools
🪛 GitHub Actions: CI

[error] 1-24: PHP CS Fixer formatting issues detected. File requires code style fixes.

src/network/mcpe/ChunkRequestTask.php (3)

25-25: Mark encoded buffer as immutable

$encodedData is written once in the constructor and never mutated afterwards.
Declaring it readonly documents this intent and prevents accidental writes in future refactors.

-	protected string $encodedData;
+	protected readonly string $encodedData;

55-56: Handle compression edge-cases

If the compressor fails or produces an empty string, the current code will still prefix
the network-ID and propagate a broken payload to the promise.
Defensive check avoids hard-to-trace network corruption:

-		$compressed = $compressor->compress($this->encodedData);
-		$this->setResult(chr($compressor->getNetworkId()) . $compressed);
+		$compressed = $compressor->compress($this->encodedData);
+		if($compressed === '' || $compressed === false){
+			throw new \RuntimeException('Chunk compression failed');
+		}
+		$this->setResult(chr($compressor->getNetworkId()) . $compressed);

59-59: Fix trailing whitespace & run php-cs-fixer

Line 59 has a stray tab; CI is already flagging CS issues for this file.
Running your standard fixer will resolve this and any other minor style violations.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e9f494 and 8730c27.

📒 Files selected for processing (2)
  • src/network/mcpe/ChunkRequestTask.php (1 hunks)
  • src/updater/UpdateCheckTask.php (1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
src/updater/UpdateCheckTask.php

[error] 1-24: PHP CS Fixer formatting issues detected. File requires code style fixes.

src/network/mcpe/ChunkRequestTask.php

[error] 1-24: PHP CS Fixer formatting issues detected. File requires code style fixes.

🪛 PHPStan (2.1.15)
src/network/mcpe/ChunkRequestTask.php

34-34: Parameter #2 $dimensionId of static method pocketmine\network\mcpe\serializer\ChunkSerializer::getSubChunkCount() expects 0|1|2, int given.

(argument.type)


35-35: Parameter #2 $dimensionId of static method pocketmine\network\mcpe\serializer\ChunkSerializer::serializeFullChunk() expects 0|1|2, int given.

(argument.type)


41-41: Parameter #2 $dimensionId of static method pocketmine\network\mcpe\protocol\LevelChunkPacket::create() expects 0|1|2, int given.

(argument.type)

Comment on lines +26 to +35
public function onRun() : void {
$error = "";
$url = sprintf("%s?channel=%s", $this->endpoint, $this->channel);

$response = Internet::getURL($url, 4, [], $error);
$this->error = $error;

if ($response === null) {
return;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add a fallback error message before early–return

Internet::getURL() populates $error only on failure.
When the request fails silently (e.g. timeout) $error stays an empty string, but you immediately overwrite the default "Unknown error" with that empty string.
Consequently the caller receives a blank error message.

-        $this->error = $error;
-
-        if ($response === null) {
-            return;
-        }
+        // Preserve a meaningful message if the request failed silently
+        $this->error = $error !== '' ? $error : 'Failed to contact update server';
+
+        if ($response === null) {
+            return;
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function onRun() : void {
$error = "";
$url = sprintf("%s?channel=%s", $this->endpoint, $this->channel);
$response = Internet::getURL($url, 4, [], $error);
$this->error = $error;
if ($response === null) {
return;
}
public function onRun() : void {
$error = "";
$url = sprintf("%s?channel=%s", $this->endpoint, $this->channel);
$response = Internet::getURL($url, 4, [], $error);
// Preserve a meaningful message if the request failed silently
$this->error = $error !== ''
? $error
: 'Failed to contact update server';
if ($response === null) {
return;
}
// …rest of method…
}
🤖 Prompt for AI Agents
In src/updater/UpdateCheckTask.php around lines 26 to 35, the error variable is
overwritten with an empty string when Internet::getURL() fails silently,
resulting in a blank error message. To fix this, set a default error message
like "Unknown error" before calling getURL(), then only overwrite it if getURL()
populates an actual error. This ensures the caller always receives a meaningful
error message even on silent failures.

Comment on lines +32 to 36
$tiles = ChunkSerializer::serializeTiles($chunk);
$blockTranslator = TypeConverter::getInstance()->getBlockTranslator();
$subCount = ChunkSerializer::getSubChunkCount($chunk, $dimensionId);
$payload = ChunkSerializer::serializeFullChunk($chunk, $dimensionId, $blockTranslator, $tiles);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Add runtime guard (or stricter type) for $dimensionId

Static analysis warns because the callee expects 0|1|2 (see DimensionIds::*).
Either assert or narrow the parameter type to those literals to prevent accidental invalid IDs and silence PHPStan.

-		$subCount = ChunkSerializer::getSubChunkCount($chunk, $dimensionId);
+		assert(in_array($dimensionId, [
+			DimensionIds::OVERWORLD,
+			DimensionIds::NETHER,
+			DimensionIds::THE_END,
+		], true), 'Invalid dimension ID');
+
+		$subCount = ChunkSerializer::getSubChunkCount($chunk, $dimensionId);

🏁 Script executed:

#!/bin/bash
# Display the ChunkRequestTask file and its header
TASK_FILE=$(rg -l "class ChunkRequestTask")
echo "---- $TASK_FILE ----"
sed -n '1,200p' "$TASK_FILE"

# Display the ChunkSerializer class, its header, and relevant method signatures
SERIALIZER_FILE=$(rg -l "class ChunkSerializer")
echo "---- $SERIALIZER_FILE ----"
sed -n '1,200p' "$SERIALIZER_FILE"
rg -n "function getSubChunkCount" "$SERIALIZER_FILE"
rg -n "function serializeFullChunk" "$SERIALIZER_FILE"

# Display the DimensionIds constants
DIM_FILE=$(rg -l "class DimensionIds")
echo "---- $DIM_FILE ----"
sed -n '1,200p' "$DIM_FILE"

Length of output: 10065


#!/bin/bash
# Locate and display the DimensionIds file content
FOUND_FILES=$(find src -type f -name "DimensionIds.php")
if [ -z "$FOUND_FILES" ]; then
  echo "No DimensionIds.php found."
  exit 0
fi

for FILE in $FOUND_FILES; do
  echo "---- $FILE ----"
  sed -n '1,200p' "$FILE"
done

#!/bin/bash
# Locate and display the DimensionIds constants
FILE=$(find src -type f -iname "DimensionIds.php" | head -n1)
if [ -z "$FILE" ]; then
  echo "No DimensionIds.php found."
  exit 1
fi

echo "---- $FILE ----"
sed -n '1,200p' "$FILE"
echo
echo "---- Constants in $FILE ----"
grep -n "const " "$FILE"

Constrain $dimensionId in ChunkRequestTask constructor

PHPStan is right: you’re passing a plain int into methods annotated to expect DimensionIds::*. You can either:

  • Add a phpdoc on the constructor to narrow the type for static analysis
  • Or add an early runtime assert(...) to catch bad IDs at runtime

Locations to update:

• File: src/network/mcpe/ChunkRequestTask.php
– Above the constructor signature, add:

/**
 * @phpstan-param DimensionIds::* $dimensionId
 */
public function __construct(int $chunkX, int $chunkZ, int $dimensionId, Chunk $chunk, CompressBatchPromise $promise, Compressor $compressor){

– (Optional) At the top of __construct, add:

assert(in_array($dimensionId, [
    DimensionIds::OVERWORLD,
    DimensionIds::NETHER,
    DimensionIds::THE_END,
], true), "Invalid dimension ID $dimensionId");

This will satisfy PHPStan’s literal‐type requirement and guard against invalid IDs at runtime.

🧰 Tools
🪛 PHPStan (2.1.15)

34-34: Parameter #2 $dimensionId of static method pocketmine\network\mcpe\serializer\ChunkSerializer::getSubChunkCount() expects 0|1|2, int given.

(argument.type)


35-35: Parameter #2 $dimensionId of static method pocketmine\network\mcpe\serializer\ChunkSerializer::serializeFullChunk() expects 0|1|2, int given.

(argument.type)

🤖 Prompt for AI Agents
In src/network/mcpe/ChunkRequestTask.php around lines 32 to 36, the $dimensionId
parameter is typed as int but should be constrained to the specific literals
defined in DimensionIds (0, 1, or 2) to satisfy static analysis. Fix this by
adding a phpdoc annotation above the constructor to specify the allowed
DimensionIds::* values for $dimensionId, and optionally add a runtime assert at
the start of the constructor to verify $dimensionId is one of the valid
constants, throwing an error if not. This ensures type safety and silences
PHPStan warnings.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

♻️ Duplicate comments (2)
src/scheduler/TaskScheduler.php (2)

95-103: Re-scheduling logic silently ignores the new task
If a task is already scheduled for the player, the method returns the existing TaskHandler and discards the new $task. This behaviour surprised reviewers previously and is still here.

-        if ($this->isScheduledFor($player)) {
-            return $this->playerTasks[$uniqueId];
-        }
+        if ($this->isScheduledFor($player)) {
+            $this->cancelForPlayer($player);           // replace instead of ignore
+        }

186-189: Clean up $playerTasks to avoid memory leaks

Completed or cancelled tasks are removed from $tasks but the per-player mapping keeps dangling references.

             } else {
                 $task->remove();
                 $this->tasks->remove($task);
+                // drop player reference
+                foreach ($this->playerTasks as $uuid => $playerTask) {
+                    if ($playerTask === $task) {
+                        unset($this->playerTasks[$uuid]);
+                        break;
+                    }
+                }
             }
🧹 Nitpick comments (8)
src/scheduler/TaskScheduler.php (1)

10-12: Remove unused use statements

TextFormat and UUID are never referenced in this class. Keeping unused imports violates CS rules (see pipeline failure) and adds noise.

-use pocketmine\utils\TextFormat;
-use pocketmine\utils\UUID;
src/world/format/io/FastChunkSerializer.php (2)

36-39: Ensure consistent endianness in pack/unpack
The current use of pack('L*', …) and unpack('L*', …) relies on machine byte order. To guarantee cross-platform consistency, switch to an explicit endian specifier ('V*' for little-endian or 'N*' for big-endian).

Also applies to: 80-82


80-82: Reindex unpacked palette array
unpack('L*', $paletteData) returns a 1-indexed array; wrap the result in array_values() to restore zero-based indexing before passing it to PalettedBlockArray::fromData().

src/world/light/LightPopulationTask.php (4)

26-28: Simplify PHPDoc for closure parameter
The generic signature in @param \Closure(array<int, LightArray>, array<int, LightArray>, non-empty-list<int>) isn’t supported by standard PHPDoc parsers. Replace it with @param callable $onCompletion or a simple @param \Closure $onCompletion to eliminate parse errors.


21-24: Initialize serialized-data properties
Declaring $heightMapData, $skyLightData, and $blockLightData as string without defaults can trigger null-assignment warnings. Either assign them an initial empty string or change their types to ?string.


42-53: Extract light-update closures into methods
Embedding two anonymous functions in an array obscures intent. Consider moving the block- and sky-light update logic into private methods (e.g., performBlockLightUpdate(), performSkyLightUpdate()) to improve readability and testability.


59-61: Align serialization calls
The multi-line assignments for $skyLightData and $blockLightData are misaligned. Align the array_map invocations or extract them into a helper for consistent formatting.

src/world/World.php (1)

1366-1394: Minor CS / readability nits

CI is currently failing PHP-CS-Fixer on this hunk (see pipeline log). A quick run of php-cs-fixer fix will re-align indentation and brace placement to the project standard.

No functional change, just styling.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8730c27 and e98e242.

📒 Files selected for processing (5)
  • src/scheduler/TaskScheduler.php (1 hunks)
  • src/world/World.php (2 hunks)
  • src/world/format/io/FastChunkSerializer.php (1 hunks)
  • src/world/generator/PopulationTask.php (1 hunks)
  • src/world/light/LightPopulationTask.php (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/world/generator/PopulationTask.php (4)
src/scheduler/AsyncTask.php (3)
  • AsyncTask (62-265)
  • storeLocal (231-233)
  • fetchLocal (245-252)
src/world/format/io/FastChunkSerializer.php (4)
  • __construct (23-25)
  • serializeTerrain (44-69)
  • FastChunkSerializer (20-114)
  • deserializeTerrain (88-113)
src/world/World.php (5)
  • __construct (481-548)
  • World (148-3631)
  • getXZ (448-450)
  • getChunk (2552-2554)
  • setChunk (2641-2707)
src/world/SimpleChunkManager.php (1)
  • SimpleChunkManager (32-84)
src/world/World.php (5)
src/world/format/Chunk.php (4)
  • isLightPopulated (180-182)
  • setLightPopulated (184-186)
  • setHeightMapArray (254-256)
  • getSubChunk (286-291)
src/scheduler/AsyncPool.php (1)
  • submitTask (196-204)
src/world/light/LightPopulationTask.php (1)
  • LightPopulationTask (18-72)
src/world/SimpleChunkManager.php (1)
  • getChunk (57-59)
src/world/format/SubChunk.php (2)
  • setBlockLightArray (121-123)
  • setBlockSkyLightArray (113-115)
🪛 PHPStan (2.1.15)
src/world/generator/PopulationTask.php

22-22: Method pocketmine\world\generator\PopulationTask::__construct() has parameter $adjacentChunks with no value type specified in iterable type array.
See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type

(missingType.iterableValue)


22-22: Method pocketmine\world\generator\PopulationTask::__construct() has parameter $onCompletion with no signature specified for Closure.

(missingType.callable)


30-30: Call to an undefined method pocketmine\world\format\Chunk::serializeTerrain().

(method.notFound)


31-31: Property pocketmine\world\generator\PopulationTask::$adjacentChunksData (string) does not accept string|null.

(assign.propertyType)


32-32: Call to an undefined method pocketmine\world\format\Chunk::serializeTerrain().

(method.notFound)


49-49: Argument of an invalid type mixed supplied for foreach, only iterables are supported.

(foreach.nonIterable)


51-51: Parameter #1 $data of static method pocketmine\world\format\io\FastChunkSerializer::deserializeTerrain() expects string, mixed given.

(argument.type)


51-51: Result of method pocketmine\world\format\Chunk::clearTerrainDirtyFlags() (void) is used.

(method.void)


57-57: Possible unreported errors in foreach on array with unspecified key type.
PHPStan might not be reporting some type errors if the key type is not specified.
Declare a key type using @phpstan-var or @phpstan-param, or use pocketmine\utils\Utils::promoteKeys() to force PHPStan to report proper errors.

(pocketmine.foreach.implicitKeys)


58-58: Cannot use array destructuring on null.

(offsetAccess.nonArray)


58-58: Result of static method pocketmine\world\World::getXZ() (void) is used.

(staticMethod.void)


58-58: Static method pocketmine\world\World::getXZ() invoked with 1 parameter, 3 required.

(arguments.count)


69-69: Property pocketmine\world\generator\PopulationTask::$adjacentChunksData (string) does not accept string|null.

(assign.propertyType)


70-70: Parameter #1 $callback of function array_map expects (callable(pocketmine\world\format\Chunk|null): mixed)|null, Closure(pocketmine\world\format\Chunk): (string|null) given.
Type #1 from the union: Type pocketmine\world\format\Chunk of parameter #1 $c of passed callable needs to be same or wider than parameter type pocketmine\world\format\Chunk|null of accepting callable.

(argument.type)


75-75: Method pocketmine\world\generator\PopulationTask::setOrGenerate() has parameter $gen with no type specified.

(missingType.parameter)


87-87: Parameter #1 $data of static method pocketmine\world\format\io\FastChunkSerializer::deserializeTerrain() expects string, string|null given.

(argument.type)


91-91: Argument of an invalid type mixed supplied for foreach, only iterables are supported.

(foreach.nonIterable)


93-93: Parameter #1 $data of static method pocketmine\world\format\io\FastChunkSerializer::deserializeTerrain() expects string, mixed given.

(argument.type)


97-97: Trying to invoke mixed but it's not a callable.

(callable.nonCallable)

src/scheduler/TaskScheduler.php

17-17: Property pocketmine\scheduler\TaskScheduler::$queue with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


23-23: Property pocketmine\scheduler\TaskScheduler::$tasks with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


28-28: Property pocketmine\scheduler\TaskScheduler::$playerTasks with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


38-38: Method pocketmine\scheduler\TaskScheduler::scheduleTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


45-45: Method pocketmine\scheduler\TaskScheduler::scheduleDelayedTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


52-52: Method pocketmine\scheduler\TaskScheduler::scheduleRepeatingTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


59-59: Method pocketmine\scheduler\TaskScheduler::scheduleDelayedRepeatingTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


63-63: Method pocketmine\scheduler\TaskScheduler::getTasks() return type has no value type specified in iterable type array.
See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type

(missingType.iterableValue)


81-81: Method pocketmine\scheduler\TaskScheduler::isQueued() has parameter $task with generic class pocketmine\scheduler\TaskHandler but does not specify its types: TTask

(missingType.generics)


95-95: Method pocketmine\scheduler\TaskScheduler::scheduleForPlayer() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


123-123: Method pocketmine\scheduler\TaskScheduler::addTask() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


137-137: Method pocketmine\scheduler\TaskScheduler::handle() has parameter $handler with generic class pocketmine\scheduler\TaskHandler but does not specify its types: TTask

(missingType.generics)


137-137: Method pocketmine\scheduler\TaskScheduler::handle() return type with generic class pocketmine\scheduler\TaskHandler does not specify its types: TTask

(missingType.generics)


173-173: PHPDoc tag @var for variable $task contains generic class pocketmine\scheduler\TaskHandler but does not specify its types: TTask

(missingType.generics)


197-197: Cannot call method getNextRun() on array<string, int|pocketmine\scheduler\TaskHandler>|int|pocketmine\scheduler\TaskHandler.

(method.nonObject)

src/world/World.php

1380-1380: Parameter #1 $values of method pocketmine\world\format\Chunk::setHeightMapArray() expects non-empty-list, array given.
• array might not be a list.
• array might be empty.

(argument.type)


1382-1382: Possible unreported errors in foreach on array with unspecified key type.
PHPStan might not be reporting some type errors if the key type is not specified.
Declare a key type using @phpstan-var or @phpstan-param, or use pocketmine\utils\Utils::promoteKeys() to force PHPStan to report proper errors.

(pocketmine.foreach.implicitKeys)


1383-1383: Using nullsafe method call on non-nullable type pocketmine\world\format\SubChunk. Use -> instead.

(nullsafe.neverNull)


1386-1386: Possible unreported errors in foreach on array with unspecified key type.
PHPStan might not be reporting some type errors if the key type is not specified.
Declare a key type using @phpstan-var or @phpstan-param, or use pocketmine\utils\Utils::promoteKeys() to force PHPStan to report proper errors.

(pocketmine.foreach.implicitKeys)


1387-1387: Using nullsafe method call on non-nullable type pocketmine\world\format\SubChunk. Use -> instead.

(nullsafe.neverNull)

src/world/format/io/FastChunkSerializer.php

27-27: Parameter $array of method pocketmine\world\format\io\FastChunkSerializer::serializePalettedArray() has invalid type pocketmine\world\format\PalettedBlockArray.

(class.notFound)


28-28: Call to method getBitsPerBlock() on an unknown class pocketmine\world\format\PalettedBlockArray.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


29-29: Call to method getWordArray() on an unknown class pocketmine\world\format\PalettedBlockArray.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


30-30: Call to method getPalette() on an unknown class pocketmine\world\format\PalettedBlockArray.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


71-71: Method pocketmine\world\format\io\FastChunkSerializer::deserializePalettedArray() has invalid return type pocketmine\world\format\PalettedBlockArray.

(class.notFound)


73-73: Call to static method getExpectedWordArraySize() on an unknown class pocketmine\world\format\PalettedBlockArray.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


82-82: Call to static method fromData() on an unknown class pocketmine\world\format\PalettedBlockArray.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)

src/world/light/LightPopulationTask.php

27-27: PHPDoc tag @param has invalid value (\Closure(array<int, LightArray>, array<int, LightArray>, non-empty-list) $onCompletion): Unexpected token "(", expected variable at offset 26 on line 2

(phpDoc.parseError)


29-29: Method pocketmine\world\light\LightPopulationTask::__construct() has parameter $onCompletion with no signature specified for Closure.

(missingType.callable)


59-59: Property pocketmine\world\light\LightPopulationTask::$heightMapData (string) does not accept string|null.

(assign.propertyType)


60-60: Property pocketmine\world\light\LightPopulationTask::$skyLightData (string) does not accept string|null.

(assign.propertyType)


61-61: Property pocketmine\world\light\LightPopulationTask::$blockLightData (string) does not accept string|null.

(assign.propertyType)


66-66: Trying to invoke mixed but it's not a callable.

(callable.nonCallable)

🪛 GitHub Actions: CI
src/world/generator/PopulationTask.php

[error] 1-91: PHP CS Fixer formatting issues detected. Please run 'php-cs-fixer fix' to fix code style issues.

src/scheduler/TaskScheduler.php

[error] 1-216: PHP CS Fixer formatting issues detected. Please run 'php-cs-fixer fix' to fix code style issues.

src/world/World.php

[error] 1352-1372: PHP CS Fixer formatting issues detected. Please run 'php-cs-fixer fix' to fix code style issues.

src/world/format/io/FastChunkSerializer.php

[error] 1-97: PHP CS Fixer formatting issues detected. Please run 'php-cs-fixer fix' to fix code style issues.

src/world/light/LightPopulationTask.php

[error] 1-62: PHP CS Fixer formatting issues detected. Please run 'php-cs-fixer fix' to fix code style issues.

🔇 Additional comments (4)
src/scheduler/TaskScheduler.php (1)

1-199: Run php-cs-fixer to satisfy CI

CI reports CS issues. Running php-cs-fixer fix on the repository will automatically correct formatting and prevent the pipeline from failing.

src/world/format/io/FastChunkSerializer.php (1)

1-4: Run PHP CS Fixer
CI logs indicate formatting issues in this file. Please run php-cs-fixer fix to resolve code style violations.

src/world/light/LightPopulationTask.php (2)

1-4: Run PHP CS Fixer
Formatting violations were flagged by CI. Execute php-cs-fixer fix to correct indentation and spacing.


65-66: Verify callback is callable before invocation
$this->fetchLocal() returns mixed. Add an is_callable() check on $callback prior to invoking it to avoid potential runtime errors.

Comment thread src/world/World.php
Comment on lines +1380 to +1388
$chunk->setHeightMapArray($heightMap);

foreach ($blockLight as $y => $lightArray) {
$chunk->getSubChunk($y)?->setBlockLightArray($lightArray);
}

foreach ($skyLight as $y => $lightArray) {
$chunk->getSubChunk($y)?->setBlockSkyLightArray($lightArray);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Remove null-safe operator – getSubChunk() never returns null

Chunk::getSubChunk() throws on an invalid Y but never returns null, therefore the null-safe operator (?->) is dead-code and produces the PHPStan nullsafe.neverNull warning.

-                $chunk->getSubChunk($y)?->setBlockLightArray($lightArray);
+                $chunk->getSubChunk($y)->setBlockLightArray($lightArray);
...
-                $chunk->getSubChunk($y)?->setBlockSkyLightArray($lightArray);
+                $chunk->getSubChunk($y)->setBlockSkyLightArray($lightArray);

This both satisfies static analysis and avoids masking logic errors where the returned sub-chunk would unexpectedly be null.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$chunk->setHeightMapArray($heightMap);
foreach ($blockLight as $y => $lightArray) {
$chunk->getSubChunk($y)?->setBlockLightArray($lightArray);
}
foreach ($skyLight as $y => $lightArray) {
$chunk->getSubChunk($y)?->setBlockSkyLightArray($lightArray);
}
$chunk->setHeightMapArray($heightMap);
foreach ($blockLight as $y => $lightArray) {
$chunk->getSubChunk($y)->setBlockLightArray($lightArray);
}
foreach ($skyLight as $y => $lightArray) {
$chunk->getSubChunk($y)->setBlockSkyLightArray($lightArray);
}
🧰 Tools
🪛 PHPStan (2.1.15)

1380-1380: Parameter #1 $values of method pocketmine\world\format\Chunk::setHeightMapArray() expects non-empty-list, array given.
• array might not be a list.
• array might be empty.

(argument.type)


1382-1382: Possible unreported errors in foreach on array with unspecified key type.
PHPStan might not be reporting some type errors if the key type is not specified.
Declare a key type using @phpstan-var or @phpstan-param, or use pocketmine\utils\Utils::promoteKeys() to force PHPStan to report proper errors.

(pocketmine.foreach.implicitKeys)


1383-1383: Using nullsafe method call on non-nullable type pocketmine\world\format\SubChunk. Use -> instead.

(nullsafe.neverNull)


1386-1386: Possible unreported errors in foreach on array with unspecified key type.
PHPStan might not be reporting some type errors if the key type is not specified.
Declare a key type using @phpstan-var or @phpstan-param, or use pocketmine\utils\Utils::promoteKeys() to force PHPStan to report proper errors.

(pocketmine.foreach.implicitKeys)


1387-1387: Using nullsafe method call on non-nullable type pocketmine\world\format\SubChunk. Use -> instead.

(nullsafe.neverNull)

🤖 Prompt for AI Agents
In src/world/World.php around lines 1380 to 1388, remove the null-safe operator
(`?->`) from the calls to `getSubChunk()` because this method never returns null
and throws on invalid input. Replace `?->` with the regular object operator `->`
to avoid dead code and PHPStan warnings, ensuring any unexpected null cases are
not silently ignored.

Comment thread src/world/World.php
Comment on lines +1355 to +1365
private function orderLightPopulation(int $chunkX, int $chunkZ): void {
$chunkHash = World::chunkHash($chunkX, $chunkZ);
$chunk = $this->chunks[$chunkHash] ?? null;

if ($chunk === null || $chunk->isLightPopulated() !== false) {
return;
}

$chunk->setLightPopulated(null);
$this->markTickingChunkForRecheck($chunkX, $chunkZ);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Height-map list must be enforced & task may silently stall

$chunk->setLightPopulated(null); is good, but the callback later passes $heightMap straight into setHeightMapArray().
That method is annotated non-empty-list<int> whereas LightPopulationTask returns a plain array that might (a) have string keys, (b) be empty when the chunk is void.
Static analysis is already yelling about this.

-            $chunk->setHeightMapArray($heightMap);
+            // make sure we provide a real non-empty 0-based list
+            $chunk->setHeightMapArray(array_values($heightMap));
+            if ($heightMap === []) {
+                $this->logger->warning("Received empty height-map for chunk $chunkX,$chunkZ – skipping light-population");
+                return;
+            }

Without this the callback may throw at runtime in strict builds, or worse, leave the chunk permanently in the “light-calculating” (null) state.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/world/World.php around lines 1355 to 1365, the callback passes $heightMap
directly to setHeightMapArray(), which requires a non-empty list of integers,
but LightPopulationTask may return an empty or associative array. To fix this,
ensure the $heightMap is validated and converted to a non-empty list of integers
before calling setHeightMapArray(), and handle the case where the height map is
empty to avoid leaving the chunk in a null light-populated state or causing
runtime errors.

Comment on lines +58 to +60
[$dx, $dz] = World::getXZ($hash);
$adjChunks[$hash] = self::setOrGenerate($mgr, $gen, $this->chunkX + $dx, $this->chunkZ + $dz, $chunkObj);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Incorrect call to World::getXZ() and double-offset

World::getXZ() is a void function that fills two out-parameters; it doesn’t return anything.
Additionally, the hash already contains absolute coordinates, so adding $this->chunkX/$this->chunkZ a second time shifts the position.

-            [$dx, $dz] = World::getXZ($hash);
-            $adjChunks[$hash] = self::setOrGenerate($mgr, $gen, $this->chunkX + $dx, $this->chunkZ + $dz, $chunkObj);
+            World::getXZ($hash, $dx, $dz);
+            $adjChunks[$hash] = self::setOrGenerate($mgr, $gen, $dx, $dz, $chunkObj);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[$dx, $dz] = World::getXZ($hash);
$adjChunks[$hash] = self::setOrGenerate($mgr, $gen, $this->chunkX + $dx, $this->chunkZ + $dz, $chunkObj);
}
World::getXZ($hash, $dx, $dz);
$adjChunks[$hash] = self::setOrGenerate($mgr, $gen, $dx, $dz, $chunkObj);
🧰 Tools
🪛 PHPStan (2.1.15)

58-58: Cannot use array destructuring on null.

(offsetAccess.nonArray)


58-58: Result of static method pocketmine\world\World::getXZ() (void) is used.

(staticMethod.void)


58-58: Static method pocketmine\world\World::getXZ() invoked with 1 parameter, 3 required.

(arguments.count)

🤖 Prompt for AI Agents
In src/world/generator/PopulationTask.php around lines 58 to 60, the call to
World::getXZ() incorrectly expects a return value, but it is a void function
that sets two out-parameters. Modify the code to declare variables for $dx and
$dz, then pass them by reference to World::getXZ() to be filled. Also, remove
the addition of $this->chunkX and $this->chunkZ when calling setOrGenerate(),
since the hash already contains absolute coordinates and adding these offsets
again causes incorrect positioning.

Comment on lines +49 to +53
foreach ($adjacent as $hash => $data) {
$adjChunks[$hash] = $data !== null
? FastChunkSerializer::deserializeTerrain($data)->clearTerrainDirtyFlags()
: null;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

void result assigned – adjacent chunks are lost

clearTerrainDirtyFlags() returns void, so the ternary assigns null instead of a Chunk instance. That forces unnecessary regeneration later.

-            $adjChunks[$hash] = $data !== null
-                ? FastChunkSerializer::deserializeTerrain($data)->clearTerrainDirtyFlags()
-                : null;
+            if ($data !== null) {
+                $chunkObj = FastChunkSerializer::deserializeTerrain($data);
+                $chunkObj->clearTerrainDirtyFlags();
+                $adjChunks[$hash] = $chunkObj;
+            } else {
+                $adjChunks[$hash] = null;
+            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
foreach ($adjacent as $hash => $data) {
$adjChunks[$hash] = $data !== null
? FastChunkSerializer::deserializeTerrain($data)->clearTerrainDirtyFlags()
: null;
}
foreach ($adjacent as $hash => $data) {
if ($data !== null) {
$chunkObj = FastChunkSerializer::deserializeTerrain($data);
$chunkObj->clearTerrainDirtyFlags();
$adjChunks[$hash] = $chunkObj;
} else {
$adjChunks[$hash] = null;
}
}
🧰 Tools
🪛 PHPStan (2.1.15)

49-49: Argument of an invalid type mixed supplied for foreach, only iterables are supported.

(foreach.nonIterable)


51-51: Parameter #1 $data of static method pocketmine\world\format\io\FastChunkSerializer::deserializeTerrain() expects string, mixed given.

(argument.type)


51-51: Result of method pocketmine\world\format\Chunk::clearTerrainDirtyFlags() (void) is used.

(method.void)

🤖 Prompt for AI Agents
In src/world/generator/PopulationTask.php around lines 49 to 53, the method
clearTerrainDirtyFlags() returns void, but its result is assigned in the ternary
expression, causing adjacent chunks to be set to null incorrectly. To fix this,
call clearTerrainDirtyFlags() on the deserialized chunk without assigning its
result; instead, assign the deserialized chunk to $adjChunks[$hash] and then
call clearTerrainDirtyFlags() on it in a separate statement.

Comment on lines +68 to +72
$this->centerChunk = FastChunkSerializer::serializeTerrain($popChunk);
$this->adjacentChunksData = igbinary_serialize(array_map(
fn(Chunk $c) => $c->isTerrainDirty() ? FastChunkSerializer::serializeTerrain($c) : null,
$adjChunks
));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Arrow function must accept null to satisfy array_map

$adjChunks contains null entries. Restricting the parameter to Chunk triggers a type error.

-            fn(Chunk $c) => $c->isTerrainDirty() ? FastChunkSerializer::serializeTerrain($c) : null,
+            fn(?Chunk $c) => $c !== null && $c->isTerrainDirty()
+                ? FastChunkSerializer::serializeTerrain($c)
+                : null,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$this->centerChunk = FastChunkSerializer::serializeTerrain($popChunk);
$this->adjacentChunksData = igbinary_serialize(array_map(
fn(Chunk $c) => $c->isTerrainDirty() ? FastChunkSerializer::serializeTerrain($c) : null,
$adjChunks
));
$this->centerChunk = FastChunkSerializer::serializeTerrain($popChunk);
$this->adjacentChunksData = igbinary_serialize(array_map(
fn(?Chunk $c) => $c !== null && $c->isTerrainDirty()
? FastChunkSerializer::serializeTerrain($c)
: null,
$adjChunks
));
🧰 Tools
🪛 PHPStan (2.1.15)

69-69: Property pocketmine\world\generator\PopulationTask::$adjacentChunksData (string) does not accept string|null.

(assign.propertyType)


70-70: Parameter #1 $callback of function array_map expects (callable(pocketmine\world\format\Chunk|null): mixed)|null, Closure(pocketmine\world\format\Chunk): (string|null) given.
Type #1 from the union: Type pocketmine\world\format\Chunk of parameter #1 $c of passed callable needs to be same or wider than parameter type pocketmine\world\format\Chunk|null of accepting callable.

(argument.type)

🤖 Prompt for AI Agents
In src/world/generator/PopulationTask.php around lines 68 to 72, the arrow
function passed to array_map is typed to accept only Chunk objects, but
$adjChunks contains null entries, causing a type error. Modify the arrow
function parameter to accept both Chunk and null types, and add a check inside
the function to handle null values appropriately before calling isTerrainDirty
or serializeTerrain.

Comment on lines +75 to +83
private static function setOrGenerate(SimpleChunkManager $mgr, $gen, int $x, int $z, ?Chunk $chunk): Chunk {
$mgr->setChunk($x, $z, $chunk ?? new Chunk([], false));
if ($chunk === null) {
$gen->generateChunk($mgr, $x, $z);
return $mgr->getChunk($x, $z)
?? throw new AssumptionFailedError("Generated chunk missing");
}
return $chunk;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add proper type-hints and remove redundant construction in setOrGenerate()

Explicitly type the generator, avoid creating a dummy Chunk when one is already supplied, and simplify the control flow.

-    private static function setOrGenerate(SimpleChunkManager $mgr, $gen, int $x, int $z, ?Chunk $chunk): Chunk {
-        $mgr->setChunk($x, $z, $chunk ?? new Chunk([], false));
-        if ($chunk === null) {
-            $gen->generateChunk($mgr, $x, $z);
-            return $mgr->getChunk($x, $z)
-                ?? throw new AssumptionFailedError("Generated chunk missing");
-        }
-        return $chunk;
+    private static function setOrGenerate(
+        SimpleChunkManager $mgr,
+        ChunkGenerator $gen,
+        int $x,
+        int $z,
+        ?Chunk $chunk
+    ): Chunk {
+        if ($chunk !== null) {
+            $mgr->setChunk($x, $z, $chunk);
+            return $chunk;
+        }
+
+        $mgr->setChunk($x, $z, new Chunk([], false));
+        $gen->generateChunk($mgr, $x, $z);
+
+        return $mgr->getChunk($x, $z)
+            ?? throw new AssumptionFailedError("Generated chunk missing");
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private static function setOrGenerate(SimpleChunkManager $mgr, $gen, int $x, int $z, ?Chunk $chunk): Chunk {
$mgr->setChunk($x, $z, $chunk ?? new Chunk([], false));
if ($chunk === null) {
$gen->generateChunk($mgr, $x, $z);
return $mgr->getChunk($x, $z)
?? throw new AssumptionFailedError("Generated chunk missing");
}
return $chunk;
}
private static function setOrGenerate(
SimpleChunkManager $mgr,
ChunkGenerator $gen,
int $x,
int $z,
?Chunk $chunk
): Chunk {
if ($chunk !== null) {
$mgr->setChunk($x, $z, $chunk);
return $chunk;
}
$mgr->setChunk($x, $z, new Chunk([], false));
$gen->generateChunk($mgr, $x, $z);
return $mgr->getChunk($x, $z)
?? throw new AssumptionFailedError("Generated chunk missing");
}
🧰 Tools
🪛 PHPStan (2.1.15)

75-75: Method pocketmine\world\generator\PopulationTask::setOrGenerate() has parameter $gen with no type specified.

(missingType.parameter)

🤖 Prompt for AI Agents
In src/world/generator/PopulationTask.php around lines 75 to 83, add explicit
type-hints for the $gen parameter to specify its expected class or interface.
Modify the method to avoid creating a new dummy Chunk when a valid $chunk is
already provided by only setting the chunk if $chunk is not null. Simplify the
control flow by restructuring the logic to eliminate redundant operations and
make the method clearer and more efficient.

Comment on lines +30 to +34
$this->centerChunk = $chunk?->serializeTerrain();
$this->adjacentChunksData = igbinary_serialize(array_map(
fn(?Chunk $c) => $c?->serializeTerrain(),
$adjacentChunks
));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Undefined serializeTerrain() ‑ use FastChunkSerializer instead

Chunk does not expose a serializeTerrain() method anywhere in the codebase, resulting in a fatal error and the PHPStan method.notFound warning.

-        $this->centerChunk = $chunk?->serializeTerrain();
-        $this->adjacentChunksData = igbinary_serialize(array_map(
-            fn(?Chunk $c) => $c?->serializeTerrain(),
+        $this->centerChunk = $chunk !== null
+            ? FastChunkSerializer::serializeTerrain($chunk)
+            : null;
+        $this->adjacentChunksData = igbinary_serialize(array_map(
+            fn(?Chunk $c) => $c !== null
+                ? FastChunkSerializer::serializeTerrain($c)
+                : null,
             $adjacentChunks
         ));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$this->centerChunk = $chunk?->serializeTerrain();
$this->adjacentChunksData = igbinary_serialize(array_map(
fn(?Chunk $c) => $c?->serializeTerrain(),
$adjacentChunks
));
$this->centerChunk = $chunk !== null
? FastChunkSerializer::serializeTerrain($chunk)
: null;
$this->adjacentChunksData = igbinary_serialize(array_map(
fn(?Chunk $c) => $c !== null
? FastChunkSerializer::serializeTerrain($c)
: null,
$adjacentChunks
));
🧰 Tools
🪛 PHPStan (2.1.15)

30-30: Call to an undefined method pocketmine\world\format\Chunk::serializeTerrain().

(method.notFound)


31-31: Property pocketmine\world\generator\PopulationTask::$adjacentChunksData (string) does not accept string|null.

(assign.propertyType)


32-32: Call to an undefined method pocketmine\world\format\Chunk::serializeTerrain().

(method.notFound)

🤖 Prompt for AI Agents
In src/world/generator/PopulationTask.php around lines 30 to 34, the code calls
an undefined method serializeTerrain() on Chunk objects, causing errors. Replace
these calls by using the FastChunkSerializer class to serialize the terrain data
of each chunk instead. Instantiate or use FastChunkSerializer to serialize the
center chunk and adjacent chunks properly, ensuring no direct call to
serializeTerrain() on Chunk instances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants