From b57035396008a2d881fb81fa56caa5d843323235 Mon Sep 17 00:00:00 2001 From: Insineer Date: Tue, 8 Oct 2019 17:34:07 +0300 Subject: [PATCH] refact(syncable): clarify interface --- OSS13 Server/Sources/World/Objects/Object.cpp | 10 ++++---- OSS13 Server/Sources/World/Tile.cpp | 10 ++++---- OSS13 Server/Sources/World/World.cpp | 3 +-- .../Sources/Shared/Network/Syncable.cpp | 19 ++++++--------- .../Sources/Shared/Network/Syncable.h | 6 ++--- .../Tests/Sources/Syncable_Tests.cpp | 23 +++++++++++++++++-- 6 files changed, 44 insertions(+), 27 deletions(-) diff --git a/OSS13 Server/Sources/World/Objects/Object.cpp b/OSS13 Server/Sources/World/Objects/Object.cpp index c4e1bc5..e050066 100644 --- a/OSS13 Server/Sources/World/Objects/Object.cpp +++ b/OSS13 Server/Sources/World/Objects/Object.cpp @@ -144,9 +144,12 @@ void Object::AddObject(Object *obj) { if (obj->GetHolder()) { if (!obj->GetHolder()->RemoveObject(obj)) return; - } else if (obj->GetTile()) + } else if (obj->GetTile()) { obj->GetTile()->RemoveObject(obj); - obj->DropUpdateState(); + } else { + obj->ResetChanges(); + } + content.push_back(obj); obj->holder = this; @@ -161,10 +164,9 @@ bool Object::RemoveObject(Object *obj) { if (obj->IsChanged()) { auto fieldsDiff = std::make_shared(); fieldsDiff->objId = obj->ID(); - fieldsDiff->fieldsChanges = obj->GetChanges(); + fieldsDiff->fieldsChanges = obj->PopChanges(); GetTile()->AddDiff(std::move(fieldsDiff), obj); } - obj->DropUpdateState(); obj->setTile(nullptr); obj->holder = nullptr; diff --git a/OSS13 Server/Sources/World/Tile.cpp b/OSS13 Server/Sources/World/Tile.cpp index bc86e28..6c2a54c 100644 --- a/OSS13 Server/Sources/World/Tile.cpp +++ b/OSS13 Server/Sources/World/Tile.cpp @@ -93,10 +93,10 @@ bool Tile::RemoveObject(Object *obj) { if (obj->IsChanged()) { auto fieldsDiff = std::make_shared(); fieldsDiff->objId = obj->ID(); - fieldsDiff->fieldsChanges = obj->GetChanges(); + fieldsDiff->fieldsChanges = obj->PopChanges(); AddDiff(std::move(fieldsDiff), obj); } - obj->DropUpdateState(); + if (removeObject(obj)) { auto diff = std::make_shared(); diff->objId = obj->ID(); @@ -193,15 +193,17 @@ void Tile::PlaceTo(Object *obj) { if (obj->IsChanged()) { auto fieldsDiff = std::make_shared(); fieldsDiff->objId = obj->ID(); - fieldsDiff->fieldsChanges = obj->GetChanges(); + fieldsDiff->fieldsChanges = obj->PopChanges(); lastTile->AddDiff(std::move(fieldsDiff), obj); } auto relocateAwayDiff = std::make_shared(); relocateAwayDiff->objId = obj->ID(); relocateAwayDiff->newCoords = pos; lastTile->AddDiff(relocateAwayDiff, obj); + } else { + obj->ResetChanges(); } - obj->DropUpdateState(); + addObject(obj); auto relocateDiff = std::make_shared(); diff --git a/OSS13 Server/Sources/World/World.cpp b/OSS13 Server/Sources/World/World.cpp index ce643fb..deb03cc 100644 --- a/OSS13 Server/Sources/World/World.cpp +++ b/OSS13 Server/Sources/World/World.cpp @@ -58,9 +58,8 @@ void World::Update(std::chrono::microseconds timeElapsed) { if (object && object->GetTile() && object->IsChanged()) { auto diff = std::make_shared(); diff->objId = object->ID(); - diff->fieldsChanges = object->GetChanges(); + diff->fieldsChanges = object->PopChanges(); object->GetTile()->AddDiff(std::move(diff), object.get()); - object->DropUpdateState(); } } } diff --git a/SharedLibrary/Sources/Shared/Network/Syncable.cpp b/SharedLibrary/Sources/Shared/Network/Syncable.cpp index bf502e2..464ace7 100644 --- a/SharedLibrary/Sources/Shared/Network/Syncable.cpp +++ b/SharedLibrary/Sources/Shared/Network/Syncable.cpp @@ -9,7 +9,7 @@ void GeneralSyncField::setChanged() { changed = true; syncable->changed = true; } // namespace detail -SyncableChanges Syncable::GetChanges() { +SyncableChanges Syncable::PopChanges() { SyncableChanges changes; changes.archive.SetMode(Archive::Mode::Input); std::size_t changedCount = 0; @@ -17,15 +17,16 @@ SyncableChanges Syncable::GetChanges() { if (getField(i)->changed) changedCount++; } - EXPECT_WITH_MSG(changedCount, "Empty SyncableChanges are created!"); changes.archive << sf::Int32(changedCount); for (std::size_t i = 0; i < fieldsOffsets.size(); i++) { if (getField(i)->changed) { changes.archive << sf::Int32(i); getField(i)->Serialize(changes.archive); + getField(i)->changed = false; } } + changed = false; return changes; } @@ -33,27 +34,21 @@ void Syncable::AmendChanges(SyncableChanges &&changes) { changes.archive.SetMode(Archive::Mode::Output); sf::Int32 changedCount; changes.archive >> changedCount; - do { + while (changedCount--) { sf::Int32 fieldNumber; changes.archive >> fieldNumber; changes.archive >> *getField(std::size_t(fieldNumber)); - } while (--changedCount); + }; } -void Syncable::DropUpdateState() { +void Syncable::ResetChanges() { for (std::size_t i = 0; i < fieldsOffsets.size(); i++) getField(i)->changed = false; changed = false; } bool Syncable::IsChanged() { - bool t = false; - for (std::size_t i = 0; i < fieldsOffsets.size(); i++) { - if (getField(i)->changed) - t = true; - } - EXPECT(t || !changed); - return changed; + return changed; } void Syncable::Serialize(Archive &ar) { diff --git a/SharedLibrary/Sources/Shared/Network/Syncable.h b/SharedLibrary/Sources/Shared/Network/Syncable.h index e8c22ed..2886d7d 100644 --- a/SharedLibrary/Sources/Shared/Network/Syncable.h +++ b/SharedLibrary/Sources/Shared/Network/Syncable.h @@ -46,7 +46,7 @@ struct SyncField : public detail::GeneralSyncField { SyncField &operator=(const T &value) { this->value = value; setChanged(); return *this; } operator const T &() const { return value; } - T &GetValue() { return value; }; + const T &GetValue() { return value; }; void Serialize(Archive &ar) final { uf::ISerializable::Serialize(ar); @@ -74,10 +74,10 @@ DEFINE_SERIALIZABLE_END struct Syncable : public uf::ISerializable { friend detail::GeneralSyncField; - SyncableChanges GetChanges(); + SyncableChanges PopChanges(); void AmendChanges(SyncableChanges &&changes); + void ResetChanges(); - void DropUpdateState(); bool IsChanged(); void Serialize(Archive &ar) final; diff --git a/SharedLibrary/Tests/Sources/Syncable_Tests.cpp b/SharedLibrary/Tests/Sources/Syncable_Tests.cpp index cc78dc8..c03cfa9 100644 --- a/SharedLibrary/Tests/Sources/Syncable_Tests.cpp +++ b/SharedLibrary/Tests/Sources/Syncable_Tests.cpp @@ -12,12 +12,31 @@ DEFINE_SYNCABLE(TestSyncable) } DEFINE_SYNCABLE_END -TEST(Syncable, test) { +TEST(Syncable, IsChangedReturnsTrueAfterFieldChange) { + TestSyncable testSyncable; + testSyncable.a = 12345; + + CHECK(testSyncable.IsChanged()); +} + +TEST(Syncable, CorrectValuesAfterAmend) { TestSyncable testSyncable; testSyncable.a = 12345; testSyncable.b = "test string"; - auto changes = testSyncable.GetChanges(); + auto changes = testSyncable.PopChanges(); + + TestSyncable otherSyncable; + otherSyncable.AmendChanges(std::move(changes)); + + CHECK(testSyncable.a.GetValue() == otherSyncable.a.GetValue()); + CHECK(testSyncable.b.GetValue() == otherSyncable.b.GetValue()); +} + +TEST(Syncable, AmendDefaultValues) { + TestSyncable testSyncable; + + auto changes = testSyncable.PopChanges(); TestSyncable otherSyncable; otherSyncable.AmendChanges(std::move(changes));