Implemented #3836: Replace setCancelled() in events with cancel() and uncancel()

The motivation for this is to prevent passing a dynamic argument to cancellation, which in almost all cases is a bug in user code. This same mistake also appears in a few places in the PM core (as seen in this commit), but in those cases the mistakes were mostly harmless since they were taking place before the event was actually called.
closes #3836
This commit is contained in:
Dylan K. Taylor 2020-09-26 14:31:56 +01:00
parent 75e3a0aa0f
commit 7b02cc3efd
12 changed files with 41 additions and 27 deletions

View File

@ -549,7 +549,9 @@ class Server{
public function saveOfflinePlayerData(string $name, CompoundTag $nbtTag) : void{
$ev = new PlayerDataSaveEvent($nbtTag, $name);
$ev->setCancelled(!$this->shouldSavePlayerData());
if(!$this->shouldSavePlayerData()){
$ev->cancel();
}
$ev->call();

View File

@ -74,7 +74,7 @@ class Fire extends Flowable{
$ev = new EntityCombustByBlockEvent($this, $entity, 8);
if($entity instanceof Arrow){
$ev->setCancelled();
$ev->cancel();
}
$ev->call();
if(!$ev->isCancelled()){

View File

@ -356,7 +356,7 @@ abstract class Living extends Entity{
public function applyDamageModifiers(EntityDamageEvent $source) : void{
if($this->lastDamageCause !== null and $this->attackTime > 0){
if($this->lastDamageCause->getBaseDamage() >= $source->getBaseDamage()){
$source->setCancelled();
$source->cancel();
}
$source->setModifier(-$this->lastDamageCause->getBaseDamage(), EntityDamageEvent::MODIFIER_PREVIOUS_DAMAGE_COOLDOWN);
}
@ -437,7 +437,7 @@ abstract class Living extends Entity{
public function attack(EntityDamageEvent $source) : void{
if($this->noDamageTicks > 0){
$source->setCancelled();
$source->cancel();
}
if($this->effectManager->has(VanillaEffects::FIRE_RESISTANCE()) and (
@ -446,7 +446,7 @@ abstract class Living extends Entity{
or $source->getCause() === EntityDamageEvent::CAUSE_LAVA
)
){
$source->setCancelled();
$source->cancel();
}
$this->applyDamageModifiers($source);

View File

@ -146,7 +146,9 @@ class EffectManager{
}
$ev = new EntityEffectAddEvent($this->entity, $effect, $oldEffect);
$ev->setCancelled($cancelled);
if($cancelled){
$ev->cancel();
}
$ev->call();
if($ev->isCancelled()){

View File

@ -178,7 +178,7 @@ class Arrow extends Projectile{
$ev = new InventoryPickupArrowEvent($playerInventory, $this);
if($this->pickupMode === self::PICKUP_NONE or ($this->pickupMode === self::PICKUP_CREATIVE and !$player->isCreative())){
$ev->setCancelled();
$ev->cancel();
}
$ev->call();

View File

@ -39,7 +39,11 @@ trait CancellableTrait{
return $this->isCancelled;
}
public function setCancelled(bool $value = true) : void{
$this->isCancelled = $value;
public function cancel() : void{
$this->isCancelled = true;
}
public function uncancel() : void{
$this->isCancelled = false;
}
}

View File

@ -27,10 +27,10 @@ namespace pocketmine\event\entity;
* Called when an effect is removed from an entity.
*/
class EntityEffectRemoveEvent extends EntityEffectEvent{
public function setCancelled(bool $value = true) : void{
public function cancel() : void{
if($this->getEffect()->getDuration() <= 0){
throw new \InvalidStateException("Removal of expired effects cannot be cancelled");
}
parent::setCancelled($value);
parent::cancel();
}
}

View File

@ -80,7 +80,7 @@ class Bow extends Tool implements Releasable{
$ev = new EntityShootBowEvent($player, $this, $entity, $baseForce * 3);
if($baseForce < 0.1 or $diff < 5 or $player->isSpectator()){
$ev->setCancelled();
$ev->cancel();
}
$ev->call();

View File

@ -1428,7 +1428,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
$ev = new PlayerItemUseEvent($this, $item, $directionVector);
if($this->hasItemCooldown($item) or $this->isSpectator()){
$ev->setCancelled();
$ev->cancel();
}
$ev->call();
@ -1462,7 +1462,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
if($slot instanceof ConsumableItem){
$ev = new PlayerItemConsumeEvent($this, $slot);
if($this->hasItemCooldown($slot)){
$ev->setCancelled();
$ev->cancel();
}
$ev->call();
@ -1521,7 +1521,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
$ev = new PlayerBlockPickEvent($this, $block, $item);
$existingSlot = $this->inventory->first($item);
if($existingSlot === -1 and $this->hasFiniteResources()){
$ev->setCancelled();
$ev->cancel();
}
$ev->call();
@ -1563,7 +1563,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
$ev = new PlayerInteractEvent($this, $this->inventory->getItemInHand(), $target, null, $face, PlayerInteractEvent::LEFT_CLICK_BLOCK);
if($this->isSpectator()){
$ev->setCancelled();
$ev->cancel();
}
$ev->call();
if($ev->isCancelled()){
@ -1669,7 +1669,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
$ev = new EntityDamageByEntityEvent($this, $entity, EntityDamageEvent::CAUSE_ENTITY_ATTACK, $heldItem->getAttackPoints());
if($this->isSpectator() or !$this->canInteract($entity->getLocation(), 8) or ($entity instanceof Player and !$this->server->getConfigGroup()->getConfigBool("pvp"))){
$ev->setCancelled();
$ev->cancel();
}
$meleeEnchantmentDamage = 0;
@ -1751,7 +1751,9 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
public function toggleFlight(bool $fly) : bool{
$ev = new PlayerToggleFlightEvent($this, $fly);
$ev->setCancelled(!$this->allowFlight);
if(!$this->allowFlight){
$ev->cancel();
}
$ev->call();
if($ev->isCancelled()){
return false;
@ -2175,9 +2177,9 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{
and $source->getCause() !== EntityDamageEvent::CAUSE_SUICIDE
and $source->getCause() !== EntityDamageEvent::CAUSE_VOID
){
$source->setCancelled();
$source->cancel();
}elseif($this->allowFlight and $source->getCause() === EntityDamageEvent::CAUSE_FALL){
$source->setCancelled();
$source->cancel();
}
parent::attack($source);

View File

@ -1448,7 +1448,7 @@ class World implements ChunkManager{
$ev = new BlockBreakEvent($player, $target, $item, $player->isCreative(), $drops, $xpDrop);
if($target instanceof Air or ($player->isSurvival() and !$target->getBreakInfo()->isBreakable()) or $player->isSpectator()){
$ev->setCancelled();
$ev->cancel();
}
if($player->isAdventure(true) and !$ev->isCancelled()){
@ -1462,7 +1462,9 @@ class World implements ChunkManager{
}
}
$ev->setCancelled(!$canBreak);
if(!$canBreak){
$ev->cancel();
}
}
$ev->call();
@ -1538,7 +1540,7 @@ class World implements ChunkManager{
if($player !== null){
$ev = new PlayerInteractEvent($player, $item, $blockClicked, $clickVector, $face, PlayerInteractEvent::RIGHT_CLICK_BLOCK);
if($player->isSpectator()){
$ev->setCancelled(); //set it to cancelled so plugins can bypass this
$ev->cancel(); //set it to cancelled so plugins can bypass this
}
$ev->call();
@ -1581,7 +1583,7 @@ class World implements ChunkManager{
if($player !== null){
$ev = new BlockPlaceEvent($player, $hand, $blockReplace, $blockClicked, $item);
if($player->isSpectator()){
$ev->setCancelled();
$ev->cancel();
}
if($player->isAdventure(true) and !$ev->isCancelled()){
@ -1595,7 +1597,9 @@ class World implements ChunkManager{
}
}
$ev->setCancelled(!$canPlace);
if(!$canPlace){
$ev->cancel();
}
}
$ev->call();

View File

@ -141,7 +141,7 @@ class WorldManager{
$ev = new WorldUnloadEvent($world);
if($world === $this->defaultWorld and !$forceUnload){
$ev->setCancelled(true);
$ev->cancel();
}
$ev->call();

View File

@ -51,7 +51,7 @@ class Main extends PluginBase implements Listener{
//be asynchronous tests running. Instead we cancel this and stop the server of our own accord once all tests
//have completed.
if($event->getCommand() === "stop"){
$event->setCancelled();
$event->cancel();
}
}