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

Setting player's pitch not working #821

Closed
mad-hon opened this issue Apr 13, 2017 · 19 comments
Closed

Setting player's pitch not working #821

mad-hon opened this issue Apr 13, 2017 · 19 comments
Labels
Category: Client Related to Mojang client (usually client-caused bugs) Resolution: Fixed

Comments

@mad-hon
Copy link

mad-hon commented Apr 13, 2017

Issue description

Setting player's position ignores whatever is supplied as pitch and resets player's pitch to 0.

Steps to reproduce the issue

$player->teleport(new Vector3($x, $y, $z), 0, 45);

This should set player's pitch to look 45 degrees down but it does not.

Via some trial and error I discovered that adding the pitch float value once more into the move player packet makes this work:

	public function encode(){
		$this->reset();
		$this->putEntityRuntimeId($this->eid);
		$this->putVector3f($this->x, $this->y, $this->z);
		$this->putLFloat($this->pitch);
		$this->putLFloat($this->yaw);
		$this->putLFloat($this->bodyYaw); //TODO
		$this->putLFloat($this->pitch); // <---- Added to make PMMP able to set player's pitch
		$this->putByte($this->mode);
		$this->putBool($this->onGround);
	}

Not sure though that this is 100% correct.
I suggest you verify what exactly has changed in the protocol, and then update the MovePlayerPosition class accordingly.

@moskadev
Copy link
Contributor

I can confirm. I have the same problem when I'm doing this.

@dktapps
Copy link
Member

dktapps commented Apr 13, 2017

That encoding is incorrect. I'm sorry, but we don't do guesswork. I can say for 100% certain that there is no extra float in MovePlayerPacket. It's more likely that you accidentally hit the MODE_ROTATION movement type. The current encoding is correct.

@dktapps dktapps added Category: Client Related to Mojang client (usually client-caused bugs) Status: Reproduced Bug has been reproduced, but cause has not yet been identified labels Apr 13, 2017
@dktapps
Copy link
Member

dktapps commented Apr 13, 2017

iTXTech/Genisys#1997

@dries-c
Copy link
Member

dries-c commented Apr 13, 2017

can not be found

@mad-hon
Copy link
Author

mad-hon commented Apr 16, 2017

Well, but when I tried to use the current encoding, I was not able to change the player's pitch, whatever mode I passed in. I mentioned that I am not sure my fix is correct, but it was the only way I actually made it work.

@mad-hon
Copy link
Author

mad-hon commented Apr 16, 2017

BTW, don't forget that when you only reverse-engineer communication between Minecraft PE client and server, you don't actually get a functional packet where server would be setting the player's pitch, so how can you be sure there's no extra float expected in such case?

@dktapps
Copy link
Member

dktapps commented Apr 16, 2017

It's more likely that you accidentally hit the MODE_ROTATION movement type. The current encoding is correct.

We know far more about MCPE internals than you give us credit for.

@mad-hon
Copy link
Author

mad-hon commented Apr 16, 2017

Well, that's good then! I was actually hoping that you do have access to more info.
But how do you explain that setting mode to rotation does not work and adding that extra float actually does work? That for me at least proves that the current encoding is still missing something.

@dktapps
Copy link
Member

dktapps commented Apr 16, 2017

Once again:

I can say for 100% certain that there is no extra float in MovePlayerPacket.

The encoding is definitely not missing anything. As I said, you've guessed and hit something that works by accident.

In the long run of things, packets are a sequence of bytes. By giving it a float you happened to give it a byte that makes it behave differently.

@mad-hon
Copy link
Author

mad-hon commented Apr 16, 2017

I confirm I guessed and made something work by accident. I have never said otherwise.
The primary reason I was trying that is that something is not right. At least the $player->teleport does not work properly because it ignores the pitch even though it accepts it as parameter.
If the packet encoding is right then the bug is somewhere between the Player::teleport method and the MovePlayerPacket::encode method.
Sorry for guessing wrong where the bug is. Will keep searching...

@dktapps
Copy link
Member

dktapps commented Apr 16, 2017

Not necessarily. The referenced issue in Genisys above also described this same issue. The general consensus was that it is a client-sided bug, however I never got around to putting the effort into finding out why the issue was occurring.

@mad-hon
Copy link
Author

mad-hon commented Apr 16, 2017

The fact that I succeeded making the player's pitch change is an indication that it can be solved from within PMMP, regardless of whether it would be a bug fix or workaround.

@mad-hon
Copy link
Author

mad-hon commented Apr 16, 2017

Slight progress - it seems the MODE_ROTATION does indeed work, but it is not used anyware.

@mad-hon
Copy link
Author

mad-hon commented Apr 16, 2017

Hmm, seems like overriding the mode in Player::sendPosition helps to achieve what I need although the behavior indicates it's still not a clear solution...

$pk->mode = ($pitch and ($mode == MovePlayerPacket::MODE_RESET)) ? MovePlayerPacket::MODE_ROTATION : $mode;

It works fine when flying but causes little trouble when standing on ground.

It seems MCPE is able to adjust the player's pitch on server's demand but only does it when receiving the MovePlayerPacket with the MODE_ROTATION mode, and produces some unwanted extra movement in response to it.

@mad-hon mad-hon changed the title Setting player's pitch not working - found a solution Setting player's pitch not working Apr 16, 2017
@ghost
Copy link

ghost commented Apr 16, 2017

@mad-hon You did not succeed. Go decompile MCPE yourself instead of guessing as dktapps said. Also, altering packets like that will cause unexpected client bugs. MiNet's encoding of the packet is virtually the same of how PocketMine does it.

It works fine when flying but causes little trouble when standing on ground.

As I said, perfect example of a unintended side effect.

@ghost
Copy link

ghost commented Apr 18, 2017

@Chris-Prime

I never said the encoding was incorrect. I said altering packets has unintended side effects weather good or bad; for example some time ago not using Mode_Reset during teleports caused weird client side glitches. I could care less about anyone's opinions, although most notably I do admire @PEMapModder contributions to PocketMine.

@mad-hon
Copy link
Author

mad-hon commented Apr 21, 2017

Well, I don't have time to decompile and examine MCPE. I thought I was helping as I assumed the main reason @dktapps did not fix this yet was low priority of this issue. Apparently my assumptions were wrong, didn't want to waste anyone's time.
Anyway, it's still helpful to know that server can affect player's pitch, even though there's no guarantee it can be done without breaking something else...

@TheDiamondYT1
Copy link

Related to 716efe2?

@dktapps
Copy link
Member

dktapps commented Apr 27, 2017

@TheDiamondYT1 no.

This is indeed a client-sided bug. They actually hacked around this by adding a separate MovePlayerPacket mode, purely for setting the player's pitch. I'm just... wtf.

@dktapps dktapps closed this as completed in 111fac6 Jul 3, 2017
@dktapps dktapps added Resolution: Fixed and removed Status: Reproduced Bug has been reproduced, but cause has not yet been identified labels Jul 3, 2017
dktapps added a commit that referenced this issue Jul 3, 2017
dktapps added a commit that referenced this issue Jul 4, 2017
Revert "Make use of Mojang's pitch hack, close #821"

This reverts commit c2dfef7.
dktapps added a commit that referenced this issue Jul 5, 2017
…e cleanup (#1153)

* Fixed chunks not loading when respawning and some minor spawn sequence cleanup

* This causes too much unexpected behaviour to be useful

Revert "Make use of Mojang's pitch hack, close #821"

This reverts commit c2dfef7.

* Removed delayed-teleport system and cleaned up movement reset for dead players

* Fixed health resetting to max when quitting and rejoining
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Client Related to Mojang client (usually client-caused bugs) Resolution: Fixed
Projects
None yet
Development

No branches or pull requests

5 participants