Skip to content

Commit 5711619

Browse files
dries-cdries-c
committed
Merge remote-tracking branch 'upstream/pm5-delay-rework' into pm5
2 parents 9d35c75 + 3e98dfc commit 5711619

6 files changed

Lines changed: 276 additions & 232 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() >= 1){
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: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,15 @@
77
use muqsit\invmenu\inventory\InvMenuInventory;
88
use muqsit\invmenu\session\network\PlayerNetwork;
99
use muqsit\invmenu\session\PlayerManager;
10+
use muqsit\invmenu\session\PlayerWindowDispatcher;
1011
use pocketmine\event\inventory\InventoryCloseEvent;
1112
use pocketmine\event\inventory\InventoryTransactionEvent;
1213
use pocketmine\event\Listener;
1314
use pocketmine\event\server\DataPacketReceiveEvent;
1415
use pocketmine\inventory\transaction\action\SlotChangeAction;
16+
use pocketmine\network\mcpe\protocol\ContainerClosePacket;
1517
use pocketmine\network\mcpe\protocol\NetworkStackLatencyPacket;
18+
use pocketmine\network\mcpe\protocol\PacketViolationWarningPacket;
1619

1720
final class InvMenuEventHandler implements Listener{
1821

@@ -31,6 +34,38 @@ public function onDataPacketReceive(DataPacketReceiveEvent $event) : void{
3134
if($player !== null){
3235
$this->player_manager->getNullable($player)?->network->notify($packet->timestamp);
3336
}
37+
}elseif($packet instanceof ContainerClosePacket){
38+
// these are not magic numbers. 255 (windowId) is supposed to be ContainerIds::NONE (-1) but it appears
39+
// either pocketmine or mojang wrongly encodes/decodes the packet. the same applies to 247 (windowType)
40+
// which actually is WindowTypes::NONE (-9).
41+
if(!$packet->server && $packet->windowId === 255 && $packet->windowType === 247){
42+
$player = $event->getOrigin()->getPlayer();
43+
if($player !== null && $this->player_manager->getNullable($player)?->dispatcher !== null){
44+
$event->cancel();
45+
}
46+
}
47+
}elseif($packet instanceof PacketViolationWarningPacket){
48+
// we (ab)use a packet violation as an ACK the inventory was successfully sent to the player. we expect to
49+
// receive the same number of violation packets as the number of excess ContainerOpenPackets that we sent.
50+
// digesting these excess violation packets is not necessary, but in this way we can intercept violations
51+
// from propagating further if existing plugins print these violations for debugging purposes.
52+
if($packet->getPacketId() === PacketViolationWarningPacket::NETWORK_ID && $packet->getType() === -1 && $packet->getSeverity() === PacketViolationWarningPacket::SEVERITY_WARNING){
53+
$player = $event->getOrigin()->getPlayer();
54+
if($player !== null){
55+
$dispatcher = $this->player_manager->getNullable($player)?->dispatcher;
56+
if($dispatcher !== null){
57+
if($dispatcher->state === PlayerWindowDispatcher::STATE_SENDING){
58+
$dispatcher->setResult(true);
59+
$event->cancel();
60+
}elseif($dispatcher->state === PlayerWindowDispatcher::STATE_FINALIZING){
61+
if(--$dispatcher->n_finalization_acks <= 0){
62+
$dispatcher->finalize();
63+
}
64+
$event->cancel();
65+
}
66+
}
67+
}
68+
}
3469
}
3570
}
3671

@@ -45,11 +80,13 @@ public function onInventoryClose(InventoryCloseEvent $event) : void{
4580
return;
4681
}
4782

48-
$current = $session->getCurrent();
83+
$current = $session->current;
4984
if($current !== null && $event->getInventory() === $current->menu->getInventory()){
50-
$current->menu->onClose($player);
85+
$current?->graphic->remove($player);
86+
$session->current = null;
5187
}
52-
$session->network->waitUntil(PlayerNetwork::DELAY_TYPE_ANIMATION_WAIT, 325, static fn(bool $success) : bool => false);
88+
$session->network->wait(PlayerNetwork::DELAY_TYPE_ANIMATION_WAIT, static fn($success) => false);
89+
$current?->menu->onClose($player);
5390
}
5491

5592
/**
@@ -61,19 +98,27 @@ public function onInventoryTransaction(InventoryTransactionEvent $event) : void{
6198
$player = $transaction->getSource();
6299

63100
$player_instance = $this->player_manager->get($player);
64-
$current = $player_instance->getCurrent();
65-
$inventory = $current?->menu->getInventory();
66-
$network_stack_callbacks = [];
67-
foreach($transaction->getActions() as $action){
68-
if(!($action instanceof SlotChangeAction)){
69-
continue;
70-
}
71101

72-
if($action->getInventory() !== $inventory){
73-
if($action->getInventory() instanceof InvMenuInventory){
102+
// cancel transaction if menu is still being sent
103+
if($player_instance->dispatcher !== null && $player_instance->dispatcher->state !== PlayerWindowDispatcher::STATE_FINALIZING){
104+
$inventory = $player_instance->dispatcher->info->menu->getInventory();
105+
foreach($transaction->getActions() as $action){
106+
if($action instanceof SlotChangeAction && $action->getInventory() === $inventory){
74107
$event->cancel();
75108
return;
76109
}
110+
}
111+
}
112+
113+
$current = $player_instance->current;
114+
if($current === null){
115+
return;
116+
}
117+
118+
$inventory = $current->menu->getInventory();
119+
$network_stack_callbacks = [];
120+
foreach($transaction->getActions() as $action){
121+
if(!($action instanceof SlotChangeAction) || $action->getInventory() !== $inventory){
77122
continue;
78123
}
79124

‎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)