refact(syncable): clarify interface

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<network::protocol::FieldsDiff>();
 				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;

OSS13 Server/Sources/World/Tile.cpp

@@ -93,10 +93,10 @@ bool Tile::RemoveObject(Object *obj) {
 	if (obj->IsChanged()) {
 		auto fieldsDiff = std::make_shared<network::protocol::FieldsDiff>();
 		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<network::protocol::RemoveDiff>();
 		diff->objId = obj->ID();
@@ -193,15 +193,17 @@ void Tile::PlaceTo(Object *obj) {
 		if (obj->IsChanged()) {
 			auto fieldsDiff = std::make_shared<network::protocol::FieldsDiff>();
 			fieldsDiff->objId = obj->ID();
-			fieldsDiff->fieldsChanges = obj->GetChanges();
+			fieldsDiff->fieldsChanges = obj->PopChanges();
 			lastTile->AddDiff(std::move(fieldsDiff), obj);
 		}
 		auto relocateAwayDiff = std::make_shared<network::protocol::RelocateAwayDiff>();
 		relocateAwayDiff->objId = obj->ID();
 		relocateAwayDiff->newCoords = pos;
 		lastTile->AddDiff(relocateAwayDiff, obj);
+	} else {
+		obj->ResetChanges();
 	}
-	obj->DropUpdateState();
+
 	addObject(obj);
 
 	auto relocateDiff = std::make_shared<network::protocol::RelocateDiff>();

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<network::protocol::FieldsDiff>();
 			diff->objId = object->ID();
-			diff->fieldsChanges = object->GetChanges();
+			diff->fieldsChanges = object->PopChanges();
 			object->GetTile()->AddDiff(std::move(diff), object.get());
-			object->DropUpdateState();
 		}
 	}
 }

SharedLibrary/Sources/Shared/Network/Syncable.cpp

@@ -9,51 +9,46 @@ 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;
 	for (std::size_t i = 0; i < fieldsOffsets.size(); i++) {
 		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;
 }
 
 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) {

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;

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