Implement beacon #4687

ShockedPlot7560 · 2022-01-01T17:23:16Z

Introduction

Beacons are currently available but completely useless as the functionality has not been added.

Changes

API changes

Added BeaconInventory and BeaconAction classes
Completion of the beacon classe

Tests

Build beacon structure as vanilla
Activate it with the ore of your choice

dktapps · 2022-01-16T18:52:09Z

a

ghost · 2022-01-16T22:46:23Z

I'm not sure that dktapps will merged it.

ShockedPlot7560 · 2022-01-16T22:52:13Z

I am in the process of making changes to improve performance

dktapps · 2022-01-16T22:55:09Z

I'm not sure that dktapps will merged it.

I haven't had time to review it yet.

ghost · 2022-01-17T02:39:46Z

I'm not sure that dktapps will merged it.

I haven't had time to review it yet.

I've meant that it's useless for most users of PocketMine-MP. This functionality can be move to plugin.

I thought pmmp was following the way of minimalism. No?

dktapps · 2022-01-17T03:41:45Z

If that were the case, we could strip almost all the code in the name of "minimalism"...

* Make use of `World->getNearbyEntities()` * Added checks for primary and secondary effect

src/block/Beacon.php

src/block/inventory/Beaconinventory.php

src/network/mcpe/convert/TypeConverter.php

src/network/mcpe/handler/InGamePacketHandler.php

src/block/Beacon.php

dktapps · 2022-01-18T21:31:48Z

If the beacon level reduces the secondary effect should be removed. This is an unlikely case so it doesn't bear much thinking about.

ShockedPlot7560 · 2022-01-18T21:35:19Z

If the beacon level reduces the secondary effect should be removed. This is an unlikely case so it doesn't bear much thinking about.

good

Crasher508

Works nice, tested
idk. who need a event/PHPDoc seems a little bit old

src/block/Beacon.php

src/network/mcpe/handler/InGamePacketHandler.php

IvanCraft623 · 2024-06-20T19:38:20Z

src/block/Beacon.php

+	}
+
+	public function onScheduledUpdate() : void{
+		$this->position->getWorld()->scheduleDelayedBlockUpdate($this->position, 20 * 3);


According to the wiki, updates are every 4 seconds

IvanCraft623 · 2024-06-20T19:41:05Z

src/block/Beacon.php

+	public const MIN_LEVEL_BEACON = 1;
+	public const MAX_LEVEL_BEACON = 4;


Inclunding BEACON in the constant name seems redundant since it is on the Beacon class

IvanCraft623 · 2024-06-20T19:43:42Z

src/block/Beacon.php

+		ItemTypeIds::NETHERITE_INGOT => true
+	];
+
+	private const ALLOWED_BLOCK_IDS = [


It would be nice to allow plugins to be able to modify this

Type tags would be the best thing

IvanCraft623 · 2024-06-20T19:46:09Z

src/block/Beacon.php

+			if(!$this->viewSky()){
+				return;
+			}
+


We need some validation here, it is possible that players might destroy blocks on any level and some effects will be no longer avaible

IvanCraft623 · 2024-06-20T19:47:21Z

src/block/Beacon.php

+		}
+	}
+
+	public function isBeaconLevelValid(int $level) : bool{


Please document this function, i feel that the name isn't clear enough

IvanCraft623 · 2024-06-20T19:49:30Z

src/block/Beacon.php

+		return true;
+	}
+
+	public function viewSky() : bool{


The function name is not the most appropriate.

IvanCraft623 · 2024-06-20T19:50:50Z

src/block/Beacon.php

+	}
+
+	/** @return Effect[] */
+	public function getAllowedEffect(int $beaconLevel) : array {


Suggested change

public function getAllowedEffect(int $beaconLevel) : array {

public function getAllowedEffects(int $beaconLevel) : array {

IvanCraft623 · 2024-06-20T19:54:36Z

src/inventory/transaction/action/BeaconPaymentAction.php

+		if(!in_array($this->primaryEffect, $allowedEffects, true)){
+			throw new TransactionValidationException("Primary effect provided is not allowed");
+		}
+		if($this->secondaryEffect !== null && !in_array($this->secondaryEffect, $allowedEffects, true)){


This validation is incomplete. Secondary effect can only be Regeneration or the same as primary effect. Current code allows that a hacked-cleint to send a primary effect as secondary.

IvanCraft623 · 2024-06-20T19:55:01Z

src/inventory/transaction/action/BeaconPaymentAction.php

+		}
+
+		$allowedEffects = $block->getAllowedEffect($block->getBeaconLevel());
+		if(!in_array($this->primaryEffect, $allowedEffects, true)){


Avoid using in_array when possible

dktapps

I stalled to review this because the performance will suck, but there currently isn't a great alternative way to detect changes in the pyramid.

dktapps · 2024-11-14T14:32:34Z

src/block/Beacon.php

+			for($z = -$level; $z <= $level; $z++){
+				$block = $world->getBlock($pos->add($x, 0, $z));
+				if(!isset(self::ALLOWED_BLOCK_IDS[$block->getTypeId()])){
+					return false;


I really don't like this. I know that's the most obvious way to do it, but it's going to lag like shit.

dktapps · 2024-11-14T14:33:01Z

src/block/Beacon.php

+		ItemTypeIds::NETHERITE_INGOT => true
+	];
+
+	private const ALLOWED_BLOCK_IDS = [


Type tags would be the best thing

dktapps · 2024-11-23T20:29:54Z

As discussed internally, it would be best to move forward with a ChunkListener-based system for triggering pyramid rechecks as per @Muqsit's beacons plugin.

github-actions · 2024-12-07T02:05:00Z

This PR has been marked as "Waiting on Author", but we haven't seen any activity in 7 days.

If there is no further activity, it will be closed in 28 days.

Note for maintainers: Adding an assignee to the PR will prevent it from being marked as stale.

ShockedPlot7560 added 6 commits January 1, 2022 17:44

implement beacon

709cbd5

fix styling

bcae409

prevent beacon desactivation

27979c1

fix: enchantment level 3

991334e

fix parameter lint

cb33b5d

fix style

4a3c7cf

JavierLeon9966 and others added 10 commits January 17, 2022 10:12

Made improvements to the beacon PR (#1)

2ea22b4

* Make use of `World->getNearbyEntities()` * Added checks for primary and secondary effect

implement beacon

a61aace

fix styling

5bc5d63

prevent beacon desactivation

4d71b9e

fix: enchantment level 3

c487433

fix parameter lint

b4ab0c4

fix style

f918f18

Made improvements to the beacon PR (#1)

34b3f27

* Make use of `World->getNearbyEntities()` * Added checks for primary and secondary effect

remove useless phpDoc

357afcc

remove useless phpDoc

e628148

dktapps changed the base branch from stable to next-minor January 17, 2022 15:07

dktapps requested changes Jan 17, 2022

View reviewed changes

dktapps added Category: Gameplay Related to Minecraft gameplay experience Type: Contribution labels Jan 17, 2022

dktapps reviewed Jan 17, 2022

View reviewed changes

src/block/Beacon.php Outdated Show resolved Hide resolved

dktapps reviewed Jan 17, 2022

View reviewed changes

src/block/Beacon.php Show resolved Hide resolved

ShockedPlot7560 added 2 commits January 17, 2022 22:24

delete Beaconinventory

ee06f86

rename Beaconinventory to BeaconInventory

2b348e5

remove mistake

1b3e223

ShockedPlot7560 requested a review from dktapps January 19, 2022 00:11

Remove dead code

7624934

Crasher508 suggested changes Mar 13, 2022

View reviewed changes

ShockedPlot7560 force-pushed the beacon branch from 6838f9c to 7624934 Compare March 13, 2022 10:18

prefer typed variable has PHPdoc

c3bfe3c

jasonw4331 added the Line: Experimental label Sep 20, 2022

dktapps removed the Line: Experimental label Sep 29, 2022

dktapps added Type: Enhancement Contributes features or other improvements to PocketMine-MP and removed Type: Contribution labels Nov 26, 2022

jasonw4331 approved these changes Jul 17, 2023

View reviewed changes

IvanCraft623 added 2 commits June 19, 2024 02:32

Merge remote-tracking branch 'upstream/minor-next' into beacon

c96768d

Update to latest PM changes

df7280c

IvanCraft623 reviewed Jun 20, 2024

View reviewed changes

dktapps requested changes Nov 14, 2024

View reviewed changes

dktapps added Status: Blocked Depends on other changes which are yet to be completed Performance Status: Waiting on Author and removed Status: Waiting on Author labels Nov 14, 2024

Merge remote-tracking branch 'origin/minor-next' into beacon

6e37cd8

ShockedPlot7560 requested a review from a team as a code owner November 20, 2024 07:36

pmmp-admin-bot bot approved these changes Nov 20, 2024

View reviewed changes

dktapps added the Status: Waiting on Author label Nov 23, 2024

dktapps removed the Status: Blocked Depends on other changes which are yet to be completed label Nov 23, 2024

github-actions bot added the Stale label Dec 7, 2024

dktapps assigned ShockedPlot7560 Dec 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement beacon #4687

Implement beacon #4687

ShockedPlot7560 commented Jan 1, 2022

dktapps commented Jan 16, 2022

ghost commented Jan 16, 2022

ShockedPlot7560 commented Jan 16, 2022

dktapps commented Jan 16, 2022

ghost commented Jan 17, 2022

dktapps commented Jan 17, 2022

dktapps commented Jan 18, 2022

ShockedPlot7560 commented Jan 18, 2022

Crasher508 left a comment

IvanCraft623 Jun 20, 2024

IvanCraft623 Jun 20, 2024

IvanCraft623 Jun 20, 2024

dktapps Nov 14, 2024

IvanCraft623 Jun 20, 2024

IvanCraft623 Jun 20, 2024

IvanCraft623 Jun 20, 2024

IvanCraft623 Jun 20, 2024

IvanCraft623 Jun 20, 2024

IvanCraft623 Jun 20, 2024

dktapps left a comment

dktapps Nov 14, 2024

dktapps Nov 14, 2024

dktapps commented Nov 23, 2024

github-actions bot commented Dec 7, 2024

		public const MIN_LEVEL_BEACON = 1;
		public const MAX_LEVEL_BEACON = 4;

	public function getAllowedEffect(int $beaconLevel) : array {
	public function getAllowedEffects(int $beaconLevel) : array {

Implement beacon #4687

Are you sure you want to change the base?

Implement beacon #4687

Conversation

ShockedPlot7560 commented Jan 1, 2022

Introduction

Changes

API changes

Tests

dktapps commented Jan 16, 2022

ghost commented Jan 16, 2022

ShockedPlot7560 commented Jan 16, 2022

dktapps commented Jan 16, 2022

ghost commented Jan 17, 2022

dktapps commented Jan 17, 2022

dktapps commented Jan 18, 2022

ShockedPlot7560 commented Jan 18, 2022

Crasher508 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dktapps left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dktapps commented Nov 23, 2024

github-actions bot commented Dec 7, 2024