Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Barrels #3672

Closed
wants to merge 4 commits into from
Closed

Add Barrels #3672

wants to merge 4 commits into from

Conversation

Aericio
Copy link
Contributor

@Aericio Aericio commented Jul 2, 2020

Introduction

Adds Barrels

Relevant issues

N/A

Changes

API changes

N/A

Behavioural changes

Adds Barrels

Backwards compatibility

N/A

Follow-up

N/A

Tests

https://youtu.be/o5MJqsvCXKk

@dktapps
Copy link
Member

dktapps commented Jul 2, 2020

hmm, this looks nice!

To fix the tests you need to:

  • Run tests/phpunit/block/regenerate_consistency_check.php
  • Regenerate the PHPStan gc-hacks.neon baseline using vendor/bin/phpstan analyze --generate-baseline tests/phpstan/configs/gc-hacks.neon

and then commit the results.
I can't review this in detail right now because I'm on mobile, but I'll get a closer look at this later.

@dktapps dktapps added Category: Gameplay Related to Minecraft gameplay experience Line: Bleeding Edge labels Jul 2, 2020
dktapps
dktapps reviewed Aug 9, 2020
View reviewed changes
src/block/Barrel.php Outdated Show resolved Hide resolved
dktapps
dktapps reviewed Aug 9, 2020
View reviewed changes
src/block/Barrel.php Outdated Show resolved Hide resolved
src/block/Barrel.php Outdated Show resolved Hide resolved
src/block/Barrel.php Outdated Show resolved Hide resolved
@dktapps
Copy link
Member

dktapps commented Aug 9, 2020

Please integrate the changes from master so I can test this.

@Aericio
Merge branch 'master' of https://github.com/pmmp/PocketMine-MP into b…
e4d72a3 
…arrels

� Conflicts:
�	tests/phpunit/block/block_factory_consistency_check.json
@Aericio
changes from review
0a17cf6
dktapps pushed a commit that referenced this pull request Nov 16, 2020
@Aericio @dktapps
Implemented Barrels, closes #3672
b2765f3
@dktapps
Copy link
Member

dktapps commented Nov 16, 2020

Merged as b2765f3. Thank you.

Sorry for how long this took to merge, I stalled for a while because I was undecided how to deal with the ChestInventory nastiness. Apart from that, this PR was near enough perfect.

@dktapps dktapps closed this Nov 16, 2020
@dktapps dktapps added the Resolution: Merged PR is merged by means other than direct GitHub merge label Nov 16, 2020
@Aericio Aericio deleted the barrels branch November 16, 2020 17:57
@dktapps dktapps added Type: Enhancement Contributes features or other improvements to PocketMine-MP and removed Type: Contribution labels Nov 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dktapps dktapps dktapps left review comments

Assignees
No one assigned
Labels
Category: Gameplay Related to Minecraft gameplay experience Resolution: Merged PR is merged by means other than direct GitHub merge Type: Enhancement Contributes features or other improvements to PocketMine-MP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Aericio @dktapps