Implemented Hopper pushing, pulling and picking up items logic

[ColinHDev]

Introduction

In pm4 hopper blocks were implemented to have an inventory. But the logic for pushing, pulling and picking up items was missing nonetheless.
This PR aims to add this logic to the hopper.
This PR contains many comments because the hopper logic is very complex. To prevent future developers from getting confused about how certain things were done, most parts were commented to explain what was done and why.

Relevant issues

• Fixes https://github.com/pmmp/PocketMine-MP/blob/master/src/block/Hopper.php#L98-L102

Sources

• https://minecraft.fandom.com/wiki/Hopper
  • https://static.wikia.nocookie.net/minecraft_gamepedia/images/3/38/Hopper_logic_flowchart.png/revision/latest?cb=20210518094503

Changes

API changes

• Added private function in block class Hopper::push(HopperInventory) : bool which handles pushing items from the hopper to a tile in the direction the hopper is facing.
• Added private function in block class Hopper::pull(HopperInventory, Container) : bool which handles pulling items by the hopper from a container above.
• Added private function in block class Hopper::pickup(HopperInventory) : bool which handles picking up items by the hopper.
• Added getter in tile class Hopper::getTransferCooldown() : int to get the transfer cooldown of that hopper.
• Added setter in tile class Hopper::setTransferCooldown(int) to set the transfer cooldown of that hopper.

Behavioural changes

• Because hoppers need to be updated every tick, it could impact the server's performance if a large amount of hoppers is loaded.

TODOs

• Pushing to specific blocks couldn't be implemented because these blocks aren't implemented at the moment. This includes the Composter, the Brewing Stand and the Jukebox (not working as in vanilla).
• Pulling from specific blocks couldn't be implemented because these blocks aren't implemented at the moment. This includes the Composter, the Brewing Stand and the Jukebox.
• Calling an event when a hopper moves an item between its own and another inventory.
• Calling an event when a hopper picks up an item.

Backwards compatibility

• This PR is fully backwards compatible. Already placed hoppers should also work without any problems when updating to a version of pm4 which merged these changes.

Tests

• Functionality tests

  • Every block a hopper can push items into was tested if it works as explained in the sources.
    https://www.youtube.com/watch?v=4gSyuViaPaU
  • Every block a hopper can pull items from was tested if it works as explained in the sources.
    https://www.youtube.com/watch?v=6NWvr6Kv88E
  • The collision box of the area where the hopper should pick up items was checked and works as explained in the sources.
    https://www.youtube.com/watch?v=hVEPiK9KWkA

• "Performance" tests

  • 128 hoppers pushing 27 * 64 dirt from one chest to another:
    https://timings.pmmp.io/?id=158627
    

[dktapps]

I don't have the time to do an in-depth review right now, but one quick note: setting an isNull item into an inventory does the same thing as setting an air item. You don't need to check for it.

[ColinHDev]

Oh, yeah, I see that now.
Thanks, I changed it

[sylvrs]

The PR overall seems good, but there are some nitpicks that I thought I'd point out 😄

[dktapps]

Not a depth review, but I have quite a few PRs to go over. Overall doesn't look bad, but I do find a lot of the comments unnecessary and there are a few other things I'm not too stoked about. I'll come back to this.

[ColinHDev]

Changed everything except the bitwise operator for now.
If I removed it, the code would get a bit bigger from (for example)

$success |= $this->pickup($inventory);

to

if($success){
    $this->pickup($inventory);
}else{
    $success = $this->pickup($inventory);
}

[dktapps]

It can be written as $success = $success && doSomething(...) to stay purely within the bounds of boolean behaviour.

[ColinHDev]

It can be written as $success = $success && doSomething(...) to stay purely within the bounds of boolean behaviour.

It shouldn't at least according to both my knowledge and the wiki (I looked it up for that https://www.php.net/manual/en/language.operators.logical.php).
If pushing failed and $success would be false, the pull or pickup functions wouldn't be called

[dktapps]

Oops. I meant ||, not &&.

[ColinHDev]

But then the functions wouldn't be called if the pushing succeeded and $success would be true

[dktapps]

You're right. I guess the condition order would have to be flipped.

[ColinHDev]

Yes

$success = $this->pickup($inventory) || $success;

should do it.
It looks a little bit awkward in my opinion, that's why I wanted to use bitwise operators. But if it works and you think it suits better, I will change it

[ColinHDev]

Changed everything, also bitwise operators. I would leave this PR open until #4413 is merged (or rejected) because its implementations of the BrewingStand and the improvements of the BrewingStandInventory would help m4e to also implement pushing and pulling logic for BrewingStands

[ColinHDev]

I still don't have a good idea how to ditch dependency on Container files in the pickup logic.. It works for now, but how stated in the TODO in the future, not all blocks a hopper can pull from are containers