Player can no longer interact with anything during spectator mode #854

Smarticles101 · 2017-04-19T21:10:41Z

Introduction

Fixes problems with spectator mode functionality.

Relevant issues

Fixes Players in spectator mode can shoot bows #852

Changes

Added custom isAlive() function to Player that returns false if the player is in spectator mode. Removed some places where that causes issues that were not needed in the first place, found one or two calls to isAlive that had two pairs of parentheses and removed the extra pairs.

Smarticles101 · 2017-04-19T21:38:18Z

src/pocketmine/Player.php

-							if($this->inventory->getItemInHand()->getId() === Item::BOW){
-								$bow = $this->inventory->getItemInHand();
+							if($this->inventory->getItemInHand()->getId() === Item::BOW
+                                and !$this->isSpectator()){


I wasn't quite sure whether to put the and !$this->isSpectator() onto a new line or not because it would have made the header of the if statement longer, but I also realize that some people don't like long headers, so I put it on the next line and then inserted another line after for readability's sake.

Same line. Look at the statement just before it, its on the same line.

SOF3 · 2017-04-20T01:57:22Z

src/pocketmine/Player.php

-							if($this->inventory->getItemInHand()->getId() === Item::BOW){
-								$bow = $this->inventory->getItemInHand();
+							if($this->inventory->getItemInHand()->getId() === Item::BOW and !$this->isSpectator()){
+							    $bow = $this->inventory->getItemInHand();


You see the spaces ;)

sigh this should be the last time I have this problem

dktapps

This is a general issue which does not just apply to bows. A specific check for bows is more of a hack.

Muqsit · 2017-04-20T20:16:00Z

How about you just...

if($this->isSpectator()){
    return;
}

Smarticles101 · 2017-04-20T21:26:27Z

There you go @dktapps

ghost · 2017-04-21T00:19:07Z

I think it would look cleaner to check $this->spawned === true first, to match up with the rest of the file.

$this->spawned and !$this->isSpectator()

Smarticles101 · 2017-04-21T00:20:33Z

@madecode15 That's a great idea. Didn't think of it when I was changing stuff

… sake

dktapps · 2017-04-21T08:28:43Z

Oh god no. All that boilerplate code. >_<

Muqsit · 2017-04-21T09:56:09Z

@dktapps same, not a fan of that.

Smarticles101 · 2017-04-21T10:19:13Z

#blameshoghip. Should I make a method to check for all those?

dktapps · 2017-04-21T12:05:08Z

This is ugly. I don't consider boilerplating this up to be an effective solution to the problem.

This will also conflict with changes made on the api3/network branch. api3/network should for the time being be considered the primary branch.

Muqsit · 2017-04-21T13:09:06Z

Should spectator players even be considered alive? If not, this will be unneeded.
IMO, they shouldn't be considered alive as it's not even a question. Spectators cannot die..

ghost · 2017-04-21T13:15:55Z

@Muqsit There not dead, so yes, they should be considered alive. Should the server have to tick them, probably not. Though making them "dead" would be a hack.

Smarticles101 · 2017-04-21T13:17:21Z

@madecode15 I'm pretty sure that the server already doesn't check for collisions on them as spectator mode players have no-clip on. Also, I think that setting a spectator mode player to be dead would be correct because it is more of a "ghost" mode where they should have no effect on anything in the world just as dead players.

dktapps · 2017-04-21T14:24:19Z

@madecode15 for most intents and purposes, they are dead. They are essentially ghosts.

Smarticles101 · 2017-04-21T14:44:12Z

@Muqsit @dktapps Making a spectator mode player dead would call for less changes, but I think it also might cause some confusion.

Muqsit · 2017-04-21T17:49:02Z

@Smarticles101 Can you give examples of confusions that could happen? I think it's okay for the most part. This would also eliminate other isSpectator() checks.
Plus, spectator mode players are ghosts, as you and @dktapps had said.

PC treats spectators as ghosts..

Smarticles101 · 2017-04-21T18:01:13Z

@Muqsit Well, at first, I was thinking it could confuse plugin developers, but after more consideration, I don't think there are any cases in which it would matter, so I'll go ahead and make the changes.

Smarticles101 · 2017-04-21T21:54:40Z

Wait a second. So I tried setting health to 0, which just kills the player. So then I tried making an isAlive method in the Player class that adds an extra check to always return false if the player is a spectator. Then I found out that that doesn't even let you move. Then I looked around and found places where it causes problems in the Player class:

Smarticles101 · 2017-04-24T21:27:33Z

Hope this is a better fix 😀 I updated the original post with new details under changes.

jasonw4331 · 2017-04-24T21:48:00Z

src/pocketmine/Player.php

@@ -2310,7 +2321,7 @@ public function handleDataPacket(DataPacket $packet){
 				break;

 			case ProtocolInfo::REMOVE_BLOCK_PACKET:
-				if($this->spawned === false or !$this->isAlive()){
+				if($this->spawned === false or !$this->isAlive()()){


Yep. I thought that those weren't mine but it turns out they were. I searched and replaced the method without parenthesis and so it left them

dktapps · 2017-05-05T10:27:22Z

src/pocketmine/Player.php

@@ -1378,7 +1385,7 @@ protected function checkNearEntities($tickDiff){
 	}

 	protected function processMovement($tickDiff){
-		if(!$this->isAlive() or !$this->spawned or $this->newPosition === null or $this->teleportPosition !== null or $this->isSleeping()){


I contest the decision that this is not needed. This will allow hacked clients to move around while dead. It also doesn't make sense to allow this regardless.

Wouldn't this be considered anticheat?

If there is a better way to implement this PR, I would be glad to. I tried implementing what was previously suggested. I might need someone to give me a push in the right direction.

It also doesn't make sense to allow this regardless.

Wouldn't this be considered anticheat?

It's a basic sanity check. There's no valid context in which dead players will move.

That project is targeted at removing the irritating parts of the anti-cheat and placing them in plugins. However it makes zero sense to have a plugin for the sake of stopping dead players moving.

There's no valid context in which dead players will move.
for most intents and purposes, they are dead. They are essentially ghosts.

Spectator mode players need to be able to move

for most intents and purposes

That does not mean that they are literally dead.

Spectators share many behavioural characteristics with dead players. Movement is not one of them.

Smarticles101 · 2017-05-05T17:37:57Z

src/pocketmine/Player.php

@@ -1385,7 +1385,7 @@ protected function checkNearEntities($tickDiff){
 	}

 	protected function processMovement($tickDiff){
-		if(!$this->spawned or $this->newPosition === null or $this->teleportPosition !== null or $this->isSleeping()){
+		if(!$this->isAlive() and !$this->isSpectator() or !$this->spawned or $this->newPosition === null or $this->teleportPosition !== null or $this->isSleeping()){


@dktapps Better?

dktapps · 2017-05-06T17:34:22Z

This is not an effective solution and will cause more problems than it solves.

Fix #852

add6064

Smarticles101 commented Apr 19, 2017

View reviewed changes

Smarticles101 added 2 commits April 19, 2017 16:41

Just realized my ide keeps putting spaces instead of tabs

28e1024

if statement is one one line

54ccf65

SOF3 reviewed Apr 20, 2017

View reviewed changes

Spaces are my worst enemy

c1dfdcb

dktapps added Category: Gameplay Related to Minecraft gameplay experience Type: Fix Bug fix, typo fix, or any other fix labels Apr 20, 2017

dktapps requested changes Apr 20, 2017

View reviewed changes

Spectator mode functionality should be mostly correct now

514bcab

Smarticles101 changed the title ~~Fix #852~~ Player can no longer interact with anything during spectator mode Apr 20, 2017

Spectator mode checks moved to the end of each line for consistency's…

069c825

… sake

Smarticles101 added 2 commits April 24, 2017 16:20

Should be a better fix now

722550c

No clue how that got there

d0c3604

jasonw4331 reviewed Apr 24, 2017

View reviewed changes

Fixed two typos

d5f5f37

dktapps force-pushed the master branch from a4adaec to f889bf9 Compare May 2, 2017 11:29

dktapps requested changes May 5, 2017

View reviewed changes

Hope this fixes everything

e2f478f

Smarticles101 commented May 5, 2017

View reviewed changes

dktapps closed this May 6, 2017

dktapps added the Resolution: Declined PR has been declined by maintainers label May 6, 2017

Smarticles101 deleted the Patch-852 branch May 6, 2017 21:12

matcracker mentioned this pull request May 8, 2017

Fixed #852 #901

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Player can no longer interact with anything during spectator mode #854

Player can no longer interact with anything during spectator mode #854

Smarticles101 commented Apr 19, 2017 •

edited

Smarticles101 Apr 19, 2017

TheDiamondYT1 Apr 19, 2017

Smarticles101 Apr 19, 2017

SOF3 Apr 20, 2017

Smarticles101 Apr 20, 2017

dktapps left a comment

Muqsit commented Apr 20, 2017

Smarticles101 commented Apr 20, 2017

ghost commented Apr 21, 2017

Smarticles101 commented Apr 21, 2017

dktapps commented Apr 21, 2017

Muqsit commented Apr 21, 2017

Smarticles101 commented Apr 21, 2017 •

edited

dktapps commented Apr 21, 2017

Muqsit commented Apr 21, 2017

ghost commented Apr 21, 2017 •

edited by ghost

Smarticles101 commented Apr 21, 2017 •

edited

dktapps commented Apr 21, 2017

Smarticles101 commented Apr 21, 2017

Muqsit commented Apr 21, 2017 •

edited

Smarticles101 commented Apr 21, 2017

Smarticles101 commented Apr 21, 2017 •

edited

Smarticles101 commented Apr 24, 2017

jasonw4331 Apr 24, 2017

Smarticles101 Apr 25, 2017

dktapps May 5, 2017

Smarticles101 May 5, 2017

dktapps May 5, 2017

tnytown May 5, 2017 •

edited

dktapps May 5, 2017

Smarticles101 May 5, 2017 •

edited

dktapps May 5, 2017 •

edited

Smarticles101 May 5, 2017

dktapps commented May 6, 2017

Player can no longer interact with anything during spectator mode #854

Player can no longer interact with anything during spectator mode #854

Conversation

Smarticles101 commented Apr 19, 2017 • edited

Introduction

Relevant issues

Changes

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

Muqsit commented Apr 20, 2017

Smarticles101 commented Apr 20, 2017

ghost commented Apr 21, 2017

Smarticles101 commented Apr 21, 2017

dktapps commented Apr 21, 2017

Muqsit commented Apr 21, 2017

Smarticles101 commented Apr 21, 2017 • edited

dktapps commented Apr 21, 2017

Muqsit commented Apr 21, 2017

ghost commented Apr 21, 2017 • edited by ghost

Smarticles101 commented Apr 21, 2017 • edited

dktapps commented Apr 21, 2017

Smarticles101 commented Apr 21, 2017

Muqsit commented Apr 21, 2017 • edited

Smarticles101 commented Apr 21, 2017

Smarticles101 commented Apr 21, 2017 • edited

Smarticles101 commented Apr 24, 2017

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

tnytown May 5, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Smarticles101 May 5, 2017 • edited

Choose a reason for hiding this comment

dktapps May 5, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dktapps commented May 6, 2017

Smarticles101 commented Apr 19, 2017 •

edited

Smarticles101 commented Apr 21, 2017 •

edited

ghost commented Apr 21, 2017 •

edited by ghost

Smarticles101 commented Apr 21, 2017 •

edited

Muqsit commented Apr 21, 2017 •

edited

Smarticles101 commented Apr 21, 2017 •

edited

tnytown May 5, 2017 •

edited

Smarticles101 May 5, 2017 •

edited

dktapps May 5, 2017 •

edited