Muqsit / InvMenu

A PocketMine-MP virion to create and manage virtual inventories!
https://poggit.pmmp.io/ci/Muqsit/InvMenu/~
GNU General Public License v3.0
201 stars 75 forks source link

PS4 players don't get sent the menu #73

Closed unickorn closed 4 years ago

unickorn commented 4 years ago

People using ps4 to connect to servers have been reporting that the inventory doesn't pop up to them at all.

https://github.com/DaPigGuy/PiggyAuctions/issues/11 https://github.com/DaPigGuy/PiggyCrates/issues/65

I'm also having the same issue. Is this a client bug or is there anything InvMenu can fix?

Edit: master branch, used on piggyauctions

TwistedAsylumMC commented 4 years ago

Can confirm this happens with any controller

Muqsit commented 4 years ago

I am kind of lazy to setup my PS4 currently. I think what's causing this bug is the minimalism in specifying tile properties in NBT.

InvMenu sends nothing more than the Tile::TAG_ID and Nameable::TAG_CUSTOM_NAME of the container in the tile NBT (and pairx, pairz for double chests). PMMP specifies the Tile::TAG_X, Tile::TAG_Y and Tile::TAG_Z as well which is probably why physically placed chests open whereas InvMenu menus don't. https://github.com/Muqsit/InvMenu/blob/master/src/muqsit/invmenu/metadata/SingleBlockMenuMetadata.php#L87-#L94

Muqsit commented 4 years ago

Can you check if this test plugin works for PS4 player? https://transfer.sh/vinYl/IMPS4Test_v0.0.1.phar Run /imps4test, it should send a chest inventory.

unickorn commented 4 years ago

I tested with a ps4 player, still doesn't seem to work.

AndreasHGK commented 4 years ago

I can confirm that the same issue occurs on my server

Joshiiiii commented 4 years ago

The link is not active anymore. But the issue still appears. Anyone got a solution?

Joshiiiii commented 4 years ago

Ps4 hasn't implemented servers yet, so it doesn't need a ping. So you have to remove the hack in https://github.com/Muqsit/InvMenu/blob/master/src/muqsit/invmenu/session/PlayerNetwork.php#L59-L64

Muqsit commented 4 years ago

Ps4 hasn't implemented servers yet, so it doesn't need a ping. So you have to remove the hack in https://github.com/Muqsit/InvMenu/blob/master/src/muqsit/invmenu/session/PlayerNetwork.php#L59-L64

By that argument, isn't this issue unnecessary to fix? PS4 hasn't implemented servers yet, so any hackarounds to access third party servers via PS4 are likely gonna be unstable.

Also, the NetworkStackLatencyPacket handling is very needed. It's a huge step-up from the unreliable even more than hacky scheduleDelayedTask method.

Joshiiiii commented 4 years ago

Oh, sorry for the wrong solution. Thanks for letting me know :)

Muqsit commented 4 years ago

I'll have to close this issue. This isn't really a bug with InvMenu but an inconsistency found within the PS4 client. The PS4 client sends a different response in the NetworkStackLatencyPacket compared to other clients. Besides, joining 3P servers via PS4 isn't officially supported anyway and any software or workaround doing so should also account for such inconsistencies — Instead of mopping the hazardous water every day, fix the pipes instead.

xxAROX commented 4 years ago

I found a method, its very unstable but it works: PlayerNetwork.php

    public function notify(int $timestamp) : void{
        if ($this->session instanceof MyCustomPlayer) {
            if ($this->session->getOS() /*returns the device-os, you can find them at 'LoginPacket::$clientData["DeviceOS"]'*/ == 11) {
                if ($this->current !== null && $timestamp !== $this->current->timestamp) {
                    $this->processCurrent(true);
                }
            } else {
                if ($this->current !== null && $timestamp === $this->current->timestamp) {
                    $this->processCurrent(true);
                }
            }
        }
    }
Muqsit commented 4 years ago

As much as I don't want to fix the network side of MCPE in a virion, this one is a special case as at the moment, pocketmine plugins and virions have to handle NetworkStackLatencyPacket on their own.

Muqsit commented 4 years ago

Didn't mean to close, this change needs testing but I can't test at the moment due to a known crash on PS4 during xbox sign in.

xxAROX commented 4 years ago

Here are some Discord-Tags:(PS4 users)

SxnDxrkh#2995 KimEd2r#2154 DM them, if you need testers.

Muqsit commented 4 years ago

Here are some Discord-Tags:(PS4 users)

SxnDxrkh#2995 KimEd2r#2154 DM them, if you need testers.

@xxAROX reproducing this issue does not require multiple clients to coordinate with one another

dries-c commented 4 years ago

I got reports it still doesn't work on PS4

Muqsit commented 4 years ago

I got reports it still doesn't work on PS4

@Driesboy I've tested it and works fine on PS4. The issue was due to incorrect timestamp values being sent by playstation client in response to NetworkStackLatencyPacket. All I can think of that could still cause this issue is: