fix: house items order being inverted in the stack

[lamonato29]

Description

This will fix the actual behaviour.

Behaviour

Actual

Put some items in the house (stacked), like sword -> mace -> shield.
When the server restarts, the stack will be inverted: shield -> mace -> sword.

fix: #2946

[kaleohanopahala]

Suggestion:
Keep emplace_back instead of insert(items.begin(), item) -> Performance.

With a single reversal after loading, the items are placed in the correct order without the need for multiple insertions that could impact performance. This ensures that you achieve the desired sequence while maintaining efficiency.

Then I added reverse to correct the order obtained by emplace_back.
I would test it like this:

void IOMapSerialize::saveTile(PropWriteStream &stream, std::shared_ptr<Tile> tile) {
	const TileItemVector* tileItems = tile->getItemList();
	if (!tileItems) {
		return;
	}

	std::vector<std::shared_ptr<Item>> items;
	items.reserve(32);

	uint16_t count = 0;
	for (auto &item : *tileItems) {
		if (item->getID() == ITEM_BATHTUB_FILLED_NOTMOVABLE) {
			std::shared_ptr<Item> tub = Item::CreateItem(ITEM_BATHTUB_FILLED);
			items.emplace_back(tub);
			++count;
			continue;
		} else if (!item->isSavedToHouses()) {
			continue;
		}

		items.emplace_back(item);
		++count;
	}

	if (!items.empty()) {
		std::reverse(items.begin(), items.end());

		const Position &tilePosition = tile->getPosition();
		stream.write<uint16_t>(tilePosition.x);
		stream.write<uint16_t>(tilePosition.y);
		stream.write<uint8_t>(tilePosition.z);

		stream.write<uint32_t>(count);
		for (const std::shared_ptr<Item> &item : items) {
			saveItem(stream, item);
		}
	}
}

[sonarcloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[lamonato29]

Suggestion: Keep emplace_back instead of insert(items.begin(), item) -> Performance.

With a single reversal after loading, the items are placed in the correct order without the need for multiple insertions that could impact performance. This ensures that you achieve the desired sequence while maintaining efficiency.

Then I added reverse to correct the order obtained by emplace_back. I would test it like this:

void IOMapSerialize::saveTile(PropWriteStream &stream, std::shared_ptr<Tile> tile) {
	const TileItemVector* tileItems = tile->getItemList();
	if (!tileItems) {
		return;
	}

	std::vector<std::shared_ptr<Item>> items;
	items.reserve(32);

	uint16_t count = 0;
	for (auto &item : *tileItems) {
		if (item->getID() == ITEM_BATHTUB_FILLED_NOTMOVABLE) {
			std::shared_ptr<Item> tub = Item::CreateItem(ITEM_BATHTUB_FILLED);
			items.emplace_back(tub);
			++count;
			continue;
		} else if (!item->isSavedToHouses()) {
			continue;
		}

		items.emplace_back(item);
		++count;
	}

	if (!items.empty()) {
		std::reverse(items.begin(), items.end());

		const Position &tilePosition = tile->getPosition();
		stream.write<uint16_t>(tilePosition.x);
		stream.write<uint16_t>(tilePosition.y);
		stream.write<uint8_t>(tilePosition.z);

		stream.write<uint32_t>(count);
		for (const std::shared_ptr<Item> &item : items) {
			saveItem(stream, item);
		}
	}
}

Thx for your suggestion.