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.

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

feat: add Crimson Nylium and Warped Nylium blocks #6481

Open
wants to merge 19 commits into
base: stable
Choose a base branch
from

Conversation

AzaleeX
Copy link

@AzaleeX AzaleeX commented Nov 3, 2024

Introduction

Crimson Nylium and Warped Nylium blocks added
image

Tests

I tested this PR by doing the following (tick all that apply):

  • Writing PHPUnit tests (commit these in the tests/phpunit folder)
  • Playtesting using a Minecraft client (provide screenshots or a video)
  • Writing a test plugin (provide the code and sample output)
  • Other (provide details)

src/block/GrassNylium.php Outdated Show resolved Hide resolved
src/block/NetherRoots.php Outdated Show resolved Hide resolved

class RedMushroom extends Flowable{
use StaticSupportTrait;
Copy link
Member

Choose a reason for hiding this comment

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

Apply only minimal change, this is unrelated to the PR "add Crimson Nylium".

Open an other PR if you want to change that.

Copy link
Member

Choose a reason for hiding this comment

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

This isn't resolved, you still add the use of StaticSupportTrait

src/block/VanillaBlocks.php Outdated Show resolved Hide resolved
src/block/GrassNylium.php Outdated Show resolved Hide resolved
@ipad54
Copy link
Member

ipad54 commented Nov 4, 2024

Bonemeal logic is missing.

Also, this PR should target minor-next branch, not stable.

@ShockedPlot7560
Copy link
Member

There is no block available for bone meal spreading, roots and fungus is missing

@ipad54
Copy link
Member

ipad54 commented Nov 4, 2024

roots and fungus is missing

Roots are implemented. https://github.com/pmmp/PocketMine-MP/blob/stable/src/block/NetherRoots.php

@jasonw4331 jasonw4331 added Category: Gameplay Related to Minecraft gameplay experience Status: Waiting on Author Type: Enhancement Contributes features or other improvements to PocketMine-MP labels Nov 4, 2024
@AzaleeX AzaleeX requested a review from a team as a code owner November 17, 2024 09:47
@AzaleeX AzaleeX requested a review from ipad54 November 17, 2024 10:00
Comment on lines +33 to +34
class NetherGrass{
public static function growGrass(ChunkManager $world, Vector3 $pos, Random $random, int $count = 8, int $radius = 5) : void{
Copy link
Member

Choose a reason for hiding this comment

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

As there is no nether generator, I don't see the point of including a single function in a new class in the generator objects.


class RedMushroom extends Flowable{
use StaticSupportTrait;
Copy link
Member

Choose a reason for hiding this comment

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

This isn't resolved, you still add the use of StaticSupportTrait

$world = $this->position->getWorld();
if($item instanceof Fertilizer){
$item->pop();
NetherGrass::growGrass($world, $this->position, new Random(mt_rand()), 8, 2);
Copy link
Member

Choose a reason for hiding this comment

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

What's the point of integrating all the logic into a specific function that's only used once?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Gameplay Related to Minecraft gameplay experience Status: Waiting on Author Type: Enhancement Contributes features or other improvements to PocketMine-MP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants