Skip to content

Commit d7204ff

Browse files
committed
Code review: Add named constants for deserialization code for better readibility
1 parent 32217de commit d7204ff

File tree

3 files changed

+32
-9
lines changed

3 files changed

+32
-9
lines changed

libs/s25main/AddonHelperFunctions.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,18 @@ class GlobalGameSettings;
1414
std::function<bool(const BuildingType type)> makeIsUnusedBuilding(const GlobalGameSettings& ggs);
1515
std::function<bool(const GoodType type)> makeIsUnusedWare(const GlobalGameSettings& ggs);
1616
std::function<bool(const Job job)> makeIsUnusedJob(const GlobalGameSettings& ggs);
17+
18+
// Only used for deserialization, can be removed when breaking compatibility (GetGameDataVersion)
19+
constexpr int leatherAddonBuildings = 3;
20+
constexpr int wineAddonBuildings = 3;
21+
constexpr int wineAndLeatherAddonBuildings = wineAddonBuildings + leatherAddonBuildings;
22+
23+
constexpr int leatherAddonGoods = 3;
24+
constexpr int wineAddonGoods = 2;
25+
constexpr int wineAndLeatherAddonGoods = wineAddonGoods + leatherAddonGoods;
26+
27+
constexpr int leatherAddonJobs = 3;
28+
constexpr int wineAddonJobs = 3;
29+
constexpr int wineAndLeatherAddonJobs = wineAddonJobs + leatherAddonJobs;
30+
31+
constexpr uint8_t transportPrioOfLeatherworks = 7;

libs/s25main/GameCommands.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// SPDX-License-Identifier: GPL-2.0-or-later
44

55
#include "GameCommands.h"
6+
#include "AddonHelperFunctions.h"
67
#include "GamePlayer.h"
78
#include "LeatherLoader.h"
89
#include "WineLoader.h"
@@ -148,7 +149,8 @@ ChangeBuildOrder::ChangeBuildOrder(Deserializer& ser)
148149
i = helpers::popEnum<BuildingType>(ser);
149150
} else
150151
{
151-
auto countOfNotAvailableBuildingsInSaveGame = ser.getDataVersion() < 1 ? 6 : 3;
152+
auto countOfNotAvailableBuildingsInSaveGame =
153+
ser.getDataVersion() < 1 ? wineAndLeatherAddonBuildings : leatherAddonBuildings;
152154
std::vector<BuildingType> buildOrder(data.size() - countOfNotAvailableBuildingsInSaveGame);
153155

154156
if(ser.getDataVersion() < 1)
@@ -192,10 +194,10 @@ ChangeTransport::ChangeTransport(Deserializer& ser) : GameCommand(GCType::Change
192194

193195
helpers::popContainer(ser, tmpData, true);
194196
std::copy(tmpData.begin(), tmpData.end(), data.begin());
195-
// all transport prios greater equal 7 are increased by one because the new leatherwork
196-
// uses prio 7
197+
// all transport prios greater equal transportPrioOfLeatherworks are increased by one because the new
198+
// leatherwork uses prio transportPrioOfLeatherworks
197199
std::transform(data.begin(), data.end() - leatherAddonAdditionalTransportOrders, data.begin(),
198-
[](uint8_t& prio) { return prio < 7 ? prio : prio + 1; });
200+
[](uint8_t& prio) { return prio < transportPrioOfLeatherworks ? prio : prio + 1; });
199201
data[std::tuple_size<TransportOrders>::value - leatherAddonAdditionalTransportOrders] =
200202
STD_TRANSPORT_PRIO[GoodType::Leather];
201203
}

libs/s25main/GamePlayer.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// SPDX-License-Identifier: GPL-2.0-or-later
44

55
#include "GamePlayer.h"
6+
#include "AddonHelperFunctions.h"
67
#include "Cheats.h"
78
#include "EventManager.h"
89
#include "FindWhConditions.h"
@@ -299,7 +300,8 @@ void GamePlayer::Deserialize(SerializedGameData& sgd)
299300

300301
if(sgd.GetGameDataVersion() < 12)
301302
{
302-
auto countOfNotAvailableBuildingsInSaveGame = sgd.GetGameDataVersion() < 11 ? 6 : 3;
303+
auto countOfNotAvailableBuildingsInSaveGame =
304+
sgd.GetGameDataVersion() < 11 ? wineAndLeatherAddonBuildings : leatherAddonBuildings;
303305
std::vector<BuildingType> build_order_raw(build_order.size() - countOfNotAvailableBuildingsInSaveGame);
304306
helpers::popContainer(sgd, build_order_raw, true);
305307

@@ -313,12 +315,14 @@ void GamePlayer::Deserialize(SerializedGameData& sgd)
313315

314316
std::copy(build_order_raw.begin(), build_order_raw.end(), build_order.begin());
315317

316-
auto countOfNotAvailableGoodsInSaveGame = sgd.GetGameDataVersion() < 11 ? 5 : 3;
318+
auto countOfNotAvailableGoodsInSaveGame =
319+
sgd.GetGameDataVersion() < 11 ? wineAndLeatherAddonGoods : leatherAddonGoods;
317320
std::vector<uint8_t> transportPrio_raw(transportPrio.size() - countOfNotAvailableGoodsInSaveGame);
318321
helpers::popContainer(sgd, transportPrio_raw, true);
319322
std::copy(transportPrio_raw.begin(), transportPrio_raw.end(), transportPrio.begin());
320323
std::transform(transportPrio.begin(), transportPrio.end() - countOfNotAvailableGoodsInSaveGame,
321-
transportPrio.begin(), [](uint8_t& prio) { return prio < 7 ? prio : prio + 1; });
324+
transportPrio.begin(),
325+
[](uint8_t& prio) { return prio < transportPrioOfLeatherworks ? prio : prio + 1; });
322326
} else
323327
{
324328
helpers::popContainer(sgd, build_order);
@@ -334,13 +338,15 @@ void GamePlayer::Deserialize(SerializedGameData& sgd)
334338

335339
if(sgd.GetGameDataVersion() < 12)
336340
{
337-
auto countOfNotAvailableGoodsInSaveGame = sgd.GetGameDataVersion() < 11 ? 5 : 3;
341+
auto countOfNotAvailableGoodsInSaveGame =
342+
sgd.GetGameDataVersion() < 11 ? wineAndLeatherAddonGoods : leatherAddonGoods;
338343
std::vector<unsigned int> global_inventory_good_raw(global_inventory.goods.size()
339344
- countOfNotAvailableGoodsInSaveGame);
340345
helpers::popContainer(sgd, global_inventory_good_raw, true);
341346
std::copy(global_inventory_good_raw.begin(), global_inventory_good_raw.end(), global_inventory.goods.begin());
342347

343-
auto countOfNotAvailableJobsInSaveGame = sgd.GetGameDataVersion() < 11 ? 6 : 3;
348+
auto countOfNotAvailableJobsInSaveGame =
349+
sgd.GetGameDataVersion() < 11 ? wineAndLeatherAddonJobs : leatherAddonJobs;
344350
std::vector<unsigned int> global_inventory_people_raw(global_inventory.people.size()
345351
- countOfNotAvailableJobsInSaveGame);
346352
helpers::popContainer(sgd, global_inventory_people_raw, true);

0 commit comments

Comments
 (0)