Skip to content

Commit 7552ae9

Browse files
MuqsitMuqsit
committed
v1.21.90: rework delay strategy by resending ContainerOpenPacket until client acknowledges the request
Mojang now handles duplicate ContainerOpenPackets gracefully by not resending the window. There is no proper way to know whether the client has indeed rendered a container, but we can infer that by (ab)using PacketViolationWarningPacket. It is important to 'know' on our end if a container was successfully sent so we know when to terminate the retry logic. Since a few versions ago, you can no longer view a container whilst having chat window opened. In these cases not only do you not get to see your inventory, but all your inventories get bricked. You can reproduce this bug in vanilla PocketMine as well - press T soon as you right-click a chest. You can now no longer use E to access your inventory. This commit also takes care of it situation - InvMenu inventories will not be a source of the issue.
1 parent 2b2eb51 commit 7552ae9

6 files changed

Lines changed: 264 additions & 223 deletions

File tree

‎src/muqsit/invmenu/InvMenu.php‎

Lines changed: 14 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
use LogicException;
99
use muqsit\invmenu\inventory\SharedInvMenuSynchronizer;
1010
use muqsit\invmenu\session\InvMenuInfo;
11-
use muqsit\invmenu\session\network\PlayerNetwork;
11+
use muqsit\invmenu\session\PlayerWindowDispatcher;
1212
use muqsit\invmenu\transaction\DeterministicInvMenuTransaction;
1313
use muqsit\invmenu\transaction\InvMenuTransaction;
1414
use muqsit\invmenu\transaction\InvMenuTransactionResult;
@@ -139,50 +139,23 @@ final public function send(Player $player, ?string $name = null, ?Closure $callb
139139
$player->removeCurrentWindow();
140140

141141
$session = InvMenuHandler::getPlayerManager()->get($player);
142-
$network = $session->network;
143-
144-
// Avoid players from spamming InvMenu::send() and other similar
145-
// requests and filling up queued tasks in memory.
146-
// It would be better if this check were implemented by plugins,
147-
// however I suppose it is more convenient if done within InvMenu...
148-
if($network->getPending() >= 8){
149-
$network->dropPending();
150-
}else{
151-
$network->dropPendingOfType(PlayerNetwork::DELAY_TYPE_OPERATION);
142+
if($session->dispatcher !== null){
143+
$session->dispatcher->then($this, $name, $callback);
144+
return;
152145
}
153146

154-
$network->waitUntil(PlayerNetwork::DELAY_TYPE_OPERATION, 50 * 8, function(bool $success) use($player, $session, $name, $callback) : bool{
155-
if(!$success){
156-
if($callback !== null){
157-
$callback(false);
158-
}
159-
return false;
147+
$graphic = $this->type->createGraphic($this, $player);
148+
if($graphic === null){
149+
if($callback !== null){
150+
$callback(false);
160151
}
152+
return;
153+
}
161154

162-
$graphic = $this->type->createGraphic($this, $player);
163-
if($graphic !== null){
164-
$session->setCurrentMenu(new InvMenuInfo($this, $graphic, $name), static function(bool $success) use($callback) : void{
165-
if($callback !== null){
166-
$callback($success);
167-
}
168-
});
169-
}else{
170-
if($callback !== null){
171-
$callback(false);
172-
}
173-
}
174-
return false;
175-
});
176-
}
177-
178-
/**
179-
* @internal use InvMenu::send() instead.
180-
*
181-
* @param Player $player
182-
* @return bool
183-
*/
184-
public function sendInventory(Player $player) : bool{
185-
return $player->setCurrentWindow($this->getInventory());
155+
$session->dispatcher = new PlayerWindowDispatcher($session, new InvMenuInfo($this, $graphic, $name));
156+
if($callback !== null){
157+
$session->dispatcher->addCallback($callback);
158+
}
186159
}
187160

188161
public function handleInventoryTransaction(Player $player, Item $out, Item $in, SlotChangeAction $action, InventoryTransaction $transaction) : InvMenuTransactionResult{
@@ -194,7 +167,5 @@ public function onClose(Player $player) : void{
194167
if($this->inventory_close_listener !== null){
195168
($this->inventory_close_listener)($player, $this->getInventory());
196169
}
197-
198-
InvMenuHandler::getPlayerManager()->get($player)->removeCurrentMenu();
199170
}
200171
}

‎src/muqsit/invmenu/InvMenuEventHandler.php‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,15 @@
66

77
use muqsit\invmenu\session\network\PlayerNetwork;
88
use muqsit\invmenu\session\PlayerManager;
9+
use muqsit\invmenu\session\PlayerWindowDispatcher;
910
use pocketmine\event\inventory\InventoryCloseEvent;
1011
use pocketmine\event\inventory\InventoryTransactionEvent;
1112
use pocketmine\event\Listener;
1213
use pocketmine\event\server\DataPacketReceiveEvent;
1314
use pocketmine\inventory\transaction\action\SlotChangeAction;
15+
use pocketmine\network\mcpe\protocol\ContainerClosePacket;
1416
use pocketmine\network\mcpe\protocol\NetworkStackLatencyPacket;
17+
use pocketmine\network\mcpe\protocol\PacketViolationWarningPacket;
1518

1619
final class InvMenuEventHandler implements Listener{
1720

@@ -30,6 +33,31 @@ public function onDataPacketReceive(DataPacketReceiveEvent $event) : void{
3033
if($player !== null){
3134
$this->player_manager->getNullable($player)?->network->notify($packet->timestamp);
3235
}
36+
}elseif($packet instanceof ContainerClosePacket){
37+
if(!$packet->server && $packet->windowId === 255 && $packet->windowType === 247){
38+
$player = $event->getOrigin()->getPlayer();
39+
if($player !== null && $this->player_manager->getNullable($player)?->dispatcher !== null){
40+
$event->cancel();
41+
}
42+
}
43+
}elseif($packet instanceof PacketViolationWarningPacket){
44+
if($packet->getPacketId() === PacketViolationWarningPacket::NETWORK_ID && $packet->getType() === -1 && $packet->getSeverity() === PacketViolationWarningPacket::SEVERITY_WARNING){
45+
$player = $event->getOrigin()->getPlayer();
46+
if($player !== null){
47+
$dispatcher = $this->player_manager->getNullable($player)?->dispatcher;
48+
if($dispatcher !== null){
49+
if($dispatcher->state === PlayerWindowDispatcher::STATE_SENDING){
50+
$dispatcher->setResult(true);
51+
$event->cancel();
52+
}elseif($dispatcher->state === PlayerWindowDispatcher::STATE_FINALIZING){
53+
if(--$dispatcher->n_finalization_acks <= 0){
54+
$dispatcher->finalize();
55+
}
56+
$event->cancel();
57+
}
58+
}
59+
}
60+
}
3361
}
3462
}
3563

@@ -44,11 +72,12 @@ public function onInventoryClose(InventoryCloseEvent $event) : void{
4472
return;
4573
}
4674

47-
$current = $session->getCurrent();
75+
$current = $session->current;
4876
if($current !== null && $event->getInventory() === $current->menu->getInventory()){
49-
$current->menu->onClose($player);
77+
$current?->graphic->remove($player);
78+
$session->current = null;
5079
}
51-
$session->network->waitUntil(PlayerNetwork::DELAY_TYPE_ANIMATION_WAIT, 325, static fn(bool $success) : bool => false);
80+
$session->network->wait(PlayerNetwork::DELAY_TYPE_ANIMATION_WAIT, static fn($success) => false);
5281
}
5382

5483
/**
@@ -60,7 +89,19 @@ public function onInventoryTransaction(InventoryTransactionEvent $event) : void{
6089
$player = $transaction->getSource();
6190

6291
$player_instance = $this->player_manager->get($player);
63-
$current = $player_instance->getCurrent();
92+
93+
// cancel transaction if menu is still being sent
94+
if($player_instance->dispatcher !== null && $player_instance->dispatcher->state !== PlayerWindowDispatcher::STATE_FINALIZING){
95+
$inventory = $player_instance->dispatcher->info->menu->getInventory();
96+
foreach($transaction->getActions() as $action){
97+
if($action instanceof SlotChangeAction && $action->getInventory() === $inventory){
98+
$event->cancel();
99+
return;
100+
}
101+
}
102+
}
103+
104+
$current = $player_instance->current;
64105
if($current === null){
65106
return;
66107
}

‎src/muqsit/invmenu/session/PlayerSession.php‎

Lines changed: 4 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,13 @@
44

55
namespace muqsit\invmenu\session;
66

7-
use Closure;
87
use muqsit\invmenu\session\network\PlayerNetwork;
98
use pocketmine\player\Player;
10-
use function spl_object_id;
119

1210
final class PlayerSession{
1311

14-
private ?InvMenuInfo $current = null;
12+
public ?PlayerWindowDispatcher $dispatcher = null;
13+
public ?InvMenuInfo $current = null;
1514

1615
public function __construct(
1716
readonly public Player $player,
@@ -27,76 +26,19 @@ public function finalize() : void{
2726
$this->player->removeCurrentWindow();
2827
}
2928
$this->network->finalize();
29+
$this->dispatcher?->finalize();
30+
$this->dispatcher = null;
3031
}
3132

3233
public function getCurrent() : ?InvMenuInfo{
3334
return $this->current;
3435
}
3536

36-
/**
37-
* @internal use InvMenu::send() instead.
38-
*
39-
* @param InvMenuInfo|null $current
40-
* @param (Closure(bool) : void)|null $callback
41-
*/
42-
public function setCurrentMenu(?InvMenuInfo $current, ?Closure $callback = null) : void{
43-
if($this->current !== null){
44-
$this->current->graphic->remove($this->player);
45-
}
46-
47-
$this->current = $current;
48-
49-
if($this->current !== null){
50-
$current_id = spl_object_id($this->current);
51-
$this->current->graphic->send($this->player, $this->current->graphic_name);
52-
$this->network->waitUntil(PlayerNetwork::DELAY_TYPE_OPERATION, $this->current->graphic->getAnimationDuration(), function(bool $success) use($callback, $current_id) : bool{
53-
$current = $this->current;
54-
if($current !== null && spl_object_id($current) === $current_id){
55-
if($success){
56-
$this->network->onBeforeSendMenu($this, $current);
57-
$result = $current->graphic->sendInventory($this->player, $current->menu->getInventory());
58-
if($result){
59-
if($callback !== null){
60-
$callback(true);
61-
}
62-
return false;
63-
}
64-
}
65-
66-
$this->removeCurrentMenu();
67-
}
68-
if($callback !== null){
69-
$callback(false);
70-
}
71-
return false;
72-
});
73-
}else{
74-
$this->network->wait(PlayerNetwork::DELAY_TYPE_ANIMATION_WAIT, static function(bool $success) use($callback) : bool{
75-
if($callback !== null){
76-
$callback($success);
77-
}
78-
return false;
79-
});
80-
}
81-
}
82-
8337
/**
8438
* @deprecated Access {@see PlayerSession::$network} directly
8539
* @return PlayerNetwork
8640
*/
8741
public function getNetwork() : PlayerNetwork{
8842
return $this->network;
8943
}
90-
91-
/**
92-
* @internal use Player::removeCurrentWindow() instead
93-
* @return bool
94-
*/
95-
public function removeCurrentMenu() : bool{
96-
if($this->current !== null){
97-
$this->setCurrentMenu(null);
98-
return true;
99-
}
100-
return false;
101-
}
10244
}

0 commit comments

Comments
 (0)