Skip to content

Commit 0b4f424

Browse files
SmallJokerSmallJoker
authoredAug 24, 2019
Inventory: Send dirty lists where appropriate (#8742)
This change reduces the amount of sent data towards clients. Inventory lists that are already known to the player are skipped, saving quite some data over time. Raises protocol version to 38 to ensure correct backwards-compatible code.
·
5.11.05.1.0
1 parent 008b80f commit 0b4f424

16 files changed

+193
-160
lines changed
 

Diff for: ‎src/client/client.cpp

+16-14
Original file line numberDiff line numberDiff line change
@@ -557,8 +557,8 @@ void Client::step(float dtime)
557557
if (count_after != count_before) {
558558
// Do this every <interval> seconds after TOCLIENT_INVENTORY
559559
// Reset the locally changed inventory to the authoritative inventory
560-
m_env.getLocalPlayer()->inventory = *m_inventory_from_server;
561-
m_inventory_updated = true;
560+
player->inventory = *m_inventory_from_server;
561+
m_update_wielded_item = true;
562562
}
563563
}
564564

@@ -1331,28 +1331,30 @@ void Client::setPlayerControl(PlayerControl &control)
13311331
void Client::setPlayerItem(u16 item)
13321332
{
13331333
m_env.getLocalPlayer()->setWieldIndex(item);
1334-
m_inventory_updated = true;
1334+
m_update_wielded_item = true;
13351335

13361336
NetworkPacket pkt(TOSERVER_PLAYERITEM, 2);
13371337
pkt << item;
13381338
Send(&pkt);
13391339
}
13401340

1341-
// Returns true if the inventory of the local player has been
1342-
// updated from the server. If it is true, it is set to false.
1343-
bool Client::getLocalInventoryUpdated()
1341+
// Returns true once after the inventory of the local player
1342+
// has been updated from the server.
1343+
bool Client::updateWieldedItem()
13441344
{
1345-
bool updated = m_inventory_updated;
1346-
m_inventory_updated = false;
1347-
return updated;
1348-
}
1345+
if (!m_update_wielded_item)
1346+
return false;
1347+
1348+
m_update_wielded_item = false;
13491349

1350-
// Copies the inventory of the local player to parameter
1351-
void Client::getLocalInventory(Inventory &dst)
1352-
{
13531350
LocalPlayer *player = m_env.getLocalPlayer();
13541351
assert(player);
1355-
dst = player->inventory;
1352+
if (auto *list = player->inventory.getList("main"))
1353+
list->setModified(false);
1354+
if (auto *list = player->inventory.getList("hand"))
1355+
list->setModified(false);
1356+
1357+
return true;
13561358
}
13571359

13581360
Inventory* Client::getInventory(const InventoryLocation &loc)

Diff for: ‎src/client/client.h

+2-4
Original file line numberDiff line numberDiff line change
@@ -274,9 +274,7 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef
274274

275275
// Returns true if the inventory of the local player has been
276276
// updated from the server. If it is true, it is set to false.
277-
bool getLocalInventoryUpdated();
278-
// Copies the inventory of the local player to parameter
279-
void getLocalInventory(Inventory &dst);
277+
bool updateWieldedItem();
280278

281279
/* InventoryManager interface */
282280
Inventory* getInventory(const InventoryLocation &loc) override;
@@ -504,7 +502,7 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef
504502
// If 0, server init hasn't been received yet.
505503
u16 m_proto_ver = 0;
506504

507-
bool m_inventory_updated = false;
505+
bool m_update_wielded_item = false;
508506
Inventory *m_inventory_from_server = nullptr;
509507
float m_inventory_from_server_age = 0.0f;
510508
PacketCounter m_packetcounter;

Diff for: ‎src/client/game.cpp

+1-11
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,6 @@ struct GameRunData {
599599
bool dig_instantly;
600600
bool digging_blocked;
601601
bool left_punch;
602-
bool update_wielded_item_trigger;
603602
bool reset_jump_timer;
604603
float nodig_delay_timer;
605604
float dig_time;
@@ -1018,7 +1017,6 @@ bool Game::startup(bool *kill,
10181017
// Reinit runData
10191018
runData = GameRunData();
10201019
runData.time_from_last_punch = 10.0;
1021-
runData.update_wielded_item_trigger = true;
10221020

10231021
m_game_ui->initFlags();
10241022

@@ -3736,19 +3734,11 @@ void Game::updateFrame(ProfilerGraph *graph, RunStats *stats, f32 dtime,
37363734
if (player->getWieldIndex() != runData.new_playeritem)
37373735
client->setPlayerItem(runData.new_playeritem);
37383736

3739-
// Update local inventory if it has changed
3740-
if (client->getLocalInventoryUpdated()) {
3741-
//infostream<<"Updating local inventory"<<std::endl;
3742-
runData.update_wielded_item_trigger = true;
3743-
}
3744-
3745-
if (runData.update_wielded_item_trigger) {
3737+
if (client->updateWieldedItem()) {
37463738
// Update wielded tool
37473739
ItemStack selected_item, hand_item;
37483740
ItemStack &tool_item = player->getWieldedItem(&selected_item, &hand_item);
37493741
camera->wield(tool_item);
3750-
3751-
runData.update_wielded_item_trigger = false;
37523742
}
37533743

37543744
/*

Diff for: ‎src/inventory.cpp

+73-41
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
2020
#include "inventory.h"
2121
#include "serialization.h"
2222
#include "debug.h"
23+
#include <algorithm>
2324
#include <sstream>
2425
#include "log.h"
2526
#include "itemdef.h"
@@ -382,27 +383,32 @@ void InventoryList::clearItems()
382383
m_items.emplace_back();
383384
}
384385

385-
//setDirty(true);
386+
setModified();
386387
}
387388

388389
void InventoryList::setSize(u32 newsize)
389390
{
390-
if(newsize != m_items.size())
391-
m_items.resize(newsize);
391+
if (newsize == m_items.size())
392+
return;
393+
394+
m_items.resize(newsize);
392395
m_size = newsize;
396+
setModified();
393397
}
394398

395399
void InventoryList::setWidth(u32 newwidth)
396400
{
397401
m_width = newwidth;
402+
setModified();
398403
}
399404

400405
void InventoryList::setName(const std::string &name)
401406
{
402407
m_name = name;
408+
setModified();
403409
}
404410

405-
void InventoryList::serialize(std::ostream &os) const
411+
void InventoryList::serialize(std::ostream &os, bool incremental) const
406412
{
407413
//os.imbue(std::locale("C"));
408414

@@ -415,6 +421,9 @@ void InventoryList::serialize(std::ostream &os) const
415421
os<<"Item ";
416422
item.serialize(os);
417423
}
424+
// TODO: Implement this:
425+
// if (!incremental || item.checkModified())
426+
// os << "Keep";
418427
os<<"\n";
419428
}
420429

@@ -424,8 +433,8 @@ void InventoryList::serialize(std::ostream &os) const
424433
void InventoryList::deSerialize(std::istream &is)
425434
{
426435
//is.imbue(std::locale("C"));
436+
setModified();
427437

428-
clearItems();
429438
u32 item_i = 0;
430439
m_width = 0;
431440

@@ -439,12 +448,12 @@ void InventoryList::deSerialize(std::istream &is)
439448
std::string name;
440449
std::getline(iss, name, ' ');
441450

442-
if (name == "EndInventoryList")
443-
return;
444-
445-
// This is a temporary backwards compatibility fix
446-
if (name == "end")
451+
if (name == "EndInventoryList" || name == "end") {
452+
// If partial incremental: Clear leftover items (should not happen!)
453+
for (size_t i = item_i; i < m_items.size(); ++i)
454+
m_items[i].clear();
447455
return;
456+
}
448457

449458
if (name == "Width") {
450459
iss >> m_width;
@@ -464,6 +473,8 @@ void InventoryList::deSerialize(std::istream &is)
464473
if(item_i > getSize() - 1)
465474
throw SerializationError("too many items");
466475
m_items[item_i++].clear();
476+
} else if (name == "Keep") {
477+
++item_i; // Unmodified item
467478
}
468479
}
469480

@@ -557,14 +568,15 @@ ItemStack InventoryList::changeItem(u32 i, const ItemStack &newitem)
557568

558569
ItemStack olditem = m_items[i];
559570
m_items[i] = newitem;
560-
//setDirty(true);
571+
setModified();
561572
return olditem;
562573
}
563574

564575
void InventoryList::deleteItem(u32 i)
565576
{
566577
assert(i < m_items.size()); // Pre-condition
567578
m_items[i].clear();
579+
setModified();
568580
}
569581

570582
ItemStack InventoryList::addItem(const ItemStack &newitem_)
@@ -612,8 +624,8 @@ ItemStack InventoryList::addItem(u32 i, const ItemStack &newitem)
612624
return newitem;
613625

614626
ItemStack leftover = m_items[i].addItem(newitem, m_itemdef);
615-
//if(leftover != newitem)
616-
// setDirty(true);
627+
if (leftover != newitem)
628+
setModified();
617629
return leftover;
618630
}
619631

@@ -682,8 +694,8 @@ ItemStack InventoryList::takeItem(u32 i, u32 takecount)
682694
return ItemStack();
683695

684696
ItemStack taken = m_items[i].takeItem(takecount);
685-
//if(!taken.empty())
686-
// setDirty(true);
697+
if (!taken.empty())
698+
setModified();
687699
return taken;
688700
}
689701

@@ -788,16 +800,6 @@ void Inventory::clear()
788800
m_lists.clear();
789801
}
790802

791-
void Inventory::clearContents()
792-
{
793-
m_dirty = true;
794-
for (InventoryList *list : m_lists) {
795-
for (u32 j=0; j<list->getSize(); j++) {
796-
list->deleteItem(j);
797-
}
798-
}
799-
}
800-
801803
Inventory::Inventory(IItemDefManager *itemdef)
802804
{
803805
m_dirty = false;
@@ -807,7 +809,6 @@ Inventory::Inventory(IItemDefManager *itemdef)
807809
Inventory::Inventory(const Inventory &other)
808810
{
809811
*this = other;
810-
m_dirty = false;
811812
}
812813

813814
Inventory & Inventory::operator = (const Inventory &other)
@@ -838,19 +839,24 @@ bool Inventory::operator == (const Inventory &other) const
838839
return true;
839840
}
840841

841-
void Inventory::serialize(std::ostream &os) const
842+
void Inventory::serialize(std::ostream &os, bool incremental) const
842843
{
843-
for (InventoryList *list : m_lists) {
844-
os<<"List "<<list->getName()<<" "<<list->getSize()<<"\n";
845-
list->serialize(os);
844+
for (const InventoryList *list : m_lists) {
845+
if (!incremental || list->checkModified()) {
846+
os << "List " << list->getName() << " " << list->getSize() << "\n";
847+
list->serialize(os, incremental);
848+
} else {
849+
os << "KeepList " << list->getName() << "\n";
850+
}
846851
}
847852

848853
os<<"EndInventory\n";
849854
}
850855

851856
void Inventory::deSerialize(std::istream &is)
852857
{
853-
clear();
858+
std::vector<InventoryList *> new_lists;
859+
new_lists.reserve(m_lists.size());
854860

855861
while (is.good()) {
856862
std::string line;
@@ -861,12 +867,20 @@ void Inventory::deSerialize(std::istream &is)
861867
std::string name;
862868
std::getline(iss, name, ' ');
863869

864-
if (name == "EndInventory")
865-
return;
870+
if (name == "EndInventory" || name == "end") {
871+
// Remove all lists that were not sent
872+
for (auto &list : m_lists) {
873+
if (std::find(new_lists.begin(), new_lists.end(), list) != new_lists.end())
874+
continue;
866875

867-
// This is a temporary backwards compatibility fix
868-
if (name == "end")
876+
delete list;
877+
list = nullptr;
878+
m_dirty = true;
879+
}
880+
m_lists.erase(std::remove(m_lists.begin(), m_lists.end(),
881+
nullptr), m_lists.end());
869882
return;
883+
}
870884

871885
if (name == "List") {
872886
std::string listname;
@@ -875,15 +889,33 @@ void Inventory::deSerialize(std::istream &is)
875889
std::getline(iss, listname, ' ');
876890
iss>>listsize;
877891

878-
InventoryList *list = new InventoryList(listname, listsize, m_itemdef);
892+
InventoryList *list = getList(listname);
893+
bool create_new = !list;
894+
if (create_new)
895+
list = new InventoryList(listname, listsize, m_itemdef);
896+
else
897+
list->setSize(listsize);
879898
list->deSerialize(is);
880899

881-
m_lists.push_back(list);
882-
}
883-
else
884-
{
885-
throw SerializationError("invalid inventory specifier: " + name);
900+
new_lists.push_back(list);
901+
if (create_new)
902+
m_lists.push_back(list);
903+
904+
} else if (name == "KeepList") {
905+
// Incrementally sent list
906+
std::string listname;
907+
std::getline(iss, listname, ' ');
908+
909+
InventoryList *list = getList(listname);
910+
if (list) {
911+
new_lists.push_back(list);
912+
} else {
913+
errorstream << "Inventory::deSerialize(): Tried to keep list '" <<
914+
listname << "' which is non-existent." << std::endl;
915+
}
886916
}
917+
// Any additional fields will throw errors when received by a client
918+
// older than PROTOCOL_VERSION 38
887919
}
888920

889921
// Contents given to deSerialize() were not terminated properly: throw error.

0 commit comments

Comments
 (0)
Please sign in to comment.