Merge lp:~widelands-dev/widelands/cleanup_playercommand_enums into lp:widelands

[GunChleoc]

Get rid of duplicate listing of player commands. This breaks compatibility for replays only.

Keeping 2 lists of the same player commands is a bit nuts, so I got rid of one of them.

Since replays become incompatible pretty fast anyway, I decided not to add any compatibility code.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5244. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/549982685.
Appveyor build 5023. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_cleanup_playercommand_enums-5023.

[Benedikt Straub (nordfriese)]

Looks good to me. Some comments in the diff.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5260. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/556294766.
Appveyor build 5037. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_cleanup_playercommand_enums-5037.

[GunChleoc (gunchleoc)]

Addressed code review. Thanks for the review

[Toni Förster (stonerl)]

review: Approve

[GunChleoc (gunchleoc)]

@bunnybot merge

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5341. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/576769589.
Appveyor build 5111. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_cleanup_playercommand_enums-5111.

=== modified file 'src/logic/playercommand.cc'
--- src/logic/playercommand.cc	2019-06-23 14:28:50 +0000
+++ src/logic/playercommand.cc	2019-08-26 10:38:04 +0000
@@ -74,108 +74,83 @@
 
 }  // namespace
 
-// NOTE keep numbers of existing entries as they are to ensure backward compatible savegame loading
-enum {
-	PLCMD_UNUSED = 0,
-	PLCMD_BULLDOZE = 1,
-	PLCMD_BUILD = 2,
-	PLCMD_BUILDFLAG = 3,
-	PLCMD_BUILDROAD = 4,
-	PLCMD_FLAGACTION = 5,
-	PLCMD_STARTSTOPBUILDING = 6,
-	PLCMD_ENHANCEBUILDING = 7,
-	PLCMD_CHANGETRAININGOPTIONS = 8,
-	PLCMD_DROPSOLDIER = 9,
-	PLCMD_CHANGESOLDIERCAPACITY = 10,
-	PLCMD_ENEMYFLAGACTION = 11,
-	PLCMD_SETWAREPRIORITY = 12,
-	PLCMD_SETWARETARGETQUANTITY = 13,
-	PLCMD_RESETWARETARGETQUANTITY = 14,
-	PLCMD_SETWORKERTARGETQUANTITY = 15,
-	PLCMD_RESETWORKERTARGETQUANTITY = 16,
-	// Used to be PLCMD_CHANGEMILITARYCONFIG
-	PLCMD_MESSAGESETSTATUSREAD = 18,
-	PLCMD_MESSAGESETSTATUSARCHIVED = 19,
-	PLCMD_SETSTOCKPOLICY = 20,
-	PLCMD_SETINPUTMAXFILL = 21,
-	PLCMD_DISMANTLEBUILDING = 22,
-	PLCMD_EVICTWORKER = 23,
-	PLCMD_MILITARYSITESETSOLDIERPREFERENCE = 24,
-	PLCMD_SHIP_EXPEDITION = 25,
-	PLCMD_SHIP_SCOUT = 26,
-	PLCMD_SHIP_EXPLORE = 27,
-	PLCMD_SHIP_CONSTRUCT = 28,
-	PLCMD_SHIP_SINK = 29,
-	PLCMD_SHIP_CANCELEXPEDITION = 30,
-	PLCMD_PROPOSE_TRADE = 31,
-};
-
 /*** class PlayerCommand ***/
 
 PlayerCommand::PlayerCommand(const uint32_t time, const PlayerNumber s)
    : GameLogicCommand(time), sender_(s), cmdserial_(0) {
 }
 
+void PlayerCommand::write_id_and_sender(StreamWrite& ser) {
+	ser.unsigned_8(static_cast<uint8_t>(id()));
+	ser.unsigned_8(sender());
+}
+
 PlayerCommand* PlayerCommand::deserialize(StreamRead& des) {
-	switch (des.unsigned_8()) {
-	case PLCMD_BULLDOZE:
+	switch (static_cast<QueueCommandTypes>(des.unsigned_8())) {
+	case QueueCommandTypes::kBulldoze:
 		return new CmdBulldoze(des);
-	case PLCMD_BUILD:
+	case QueueCommandTypes::kBuild:
 		return new CmdBuild(des);
-	case PLCMD_BUILDFLAG:
+	case QueueCommandTypes::kBuildFlag:
 		return new CmdBuildFlag(des);
-	case PLCMD_BUILDROAD:
+	case QueueCommandTypes::kBuildRoad:
 		return new CmdBuildRoad(des);
-	case PLCMD_FLAGACTION:
+	case QueueCommandTypes::kFlagAction:
 		return new CmdFlagAction(des);
-	case PLCMD_STARTSTOPBUILDING:
+	case QueueCommandTypes::kStartStopBuilding:
 		return new CmdStartStopBuilding(des);
-	case PLCMD_SHIP_EXPEDITION:
-		return new CmdStartOrCancelExpedition(des);
-	case PLCMD_SHIP_SCOUT:
-		return new CmdShipScoutDirection(des);
-	case PLCMD_SHIP_EXPLORE:
-		return new CmdShipExploreIsland(des);
-	case PLCMD_SHIP_CONSTRUCT:
-		return new CmdShipConstructPort(des);
-	case PLCMD_SHIP_SINK:
-		return new CmdShipSink(des);
-	case PLCMD_SHIP_CANCELEXPEDITION:
-		return new CmdShipCancelExpedition(des);
-	case PLCMD_ENHANCEBUILDING:
+	case QueueCommandTypes::kEnhanceBuilding:
 		return new CmdEnhanceBuilding(des);
-	case PLCMD_CHANGETRAININGOPTIONS:
+
+	case QueueCommandTypes::kChangeTrainingOptions:
 		return new CmdChangeTrainingOptions(des);
-	case PLCMD_DROPSOLDIER:
+	case QueueCommandTypes::kDropSoldier:
 		return new CmdDropSoldier(des);
-	case PLCMD_CHANGESOLDIERCAPACITY:
+	case QueueCommandTypes::kChangeSoldierCapacity:
 		return new CmdChangeSoldierCapacity(des);
-	case PLCMD_ENEMYFLAGACTION:
+	case QueueCommandTypes::kEnemyFlagAction:
 		return new CmdEnemyFlagAction(des);
-	case PLCMD_SETWAREPRIORITY:
+
+	case QueueCommandTypes::kSetWarePriority:
 		return new CmdSetWarePriority(des);
-	case PLCMD_SETWARETARGETQUANTITY:
+	case QueueCommandTypes::kSetWareTargetQuantity:
 		return new CmdSetWareTargetQuantity(des);
-	case PLCMD_RESETWARETARGETQUANTITY:
+	case QueueCommandTypes::kResetWareTargetQuantity:
 		return new CmdResetWareTargetQuantity(des);
-	case PLCMD_SETWORKERTARGETQUANTITY:
+	case QueueCommandTypes::kSetWorkerTargetQuantity:
 		return new CmdSetWorkerTargetQuantity(des);
-	case PLCMD_RESETWORKERTARGETQUANTITY:
+	case QueueCommandTypes::kResetWorkerTargetQuantity:
 		return new CmdResetWorkerTargetQuantity(des);
-	case PLCMD_MESSAGESETSTATUSREAD:
+
+	case QueueCommandTypes::kMessageSetStatusRead:
 		return new CmdMessageSetStatusRead(des);
-	case PLCMD_MESSAGESETSTATUSARCHIVED:
+	case QueueCommandTypes::kMessageSetStatusArchived:
 		return new CmdMessageSetStatusArchived(des);
-	case PLCMD_SETSTOCKPOLICY:
+
+	case QueueCommandTypes::kSetStockPolicy:
 		return new CmdSetStockPolicy(des);
-	case PLCMD_SETINPUTMAXFILL:
+	case QueueCommandTypes::kSetInputMaxFill:
 		return new CmdSetInputMaxFill(des);
-	case PLCMD_DISMANTLEBUILDING:
+	case QueueCommandTypes::kDismantleBuilding:
 		return new CmdDismantleBuilding(des);
-	case PLCMD_EVICTWORKER:
+	case QueueCommandTypes::kEvictWorker:
 		return new CmdEvictWorker(des);
-	case PLCMD_MILITARYSITESETSOLDIERPREFERENCE:
+	case QueueCommandTypes::kMilitarysiteSetSoldierPreference:
 		return new CmdMilitarySiteSetSoldierPreference(des);
+
+	case QueueCommandTypes::kStartOrCancelExpedition:
+		return new CmdStartOrCancelExpedition(des);
+	case QueueCommandTypes::kShipScoutDirection:
+		return new CmdShipScoutDirection(des);
+	case QueueCommandTypes::kShipExploreIsland:
+		return new CmdShipExploreIsland(des);
+	case QueueCommandTypes::kShipConstructPort:
+		return new CmdShipConstructPort(des);
+	case QueueCommandTypes::kShipSink:
+		return new CmdShipSink(des);
+	case QueueCommandTypes::kShipCancelExpedition:
+		return new CmdShipCancelExpedition(des);
+
 	default:
 		throw wexception("PlayerCommand::deserialize(): Invalid command id encountered");
 	}
@@ -226,8 +201,7 @@
 }
 
 void CmdBulldoze::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_BULLDOZE);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 	ser.unsigned_8(recurse);
 }
@@ -273,8 +247,7 @@
 }
 
 void CmdBuild::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_BUILD);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.signed_16(bi);
 	write_coords_32(&ser, coords);
 }
@@ -317,8 +290,7 @@
 }
 
 void CmdBuildFlag::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_BUILDFLAG);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	write_coords_32(&ser, coords);
 }
 
@@ -387,8 +359,7 @@
 }
 
 void CmdBuildRoad::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_BUILDROAD);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	write_coords_32(&ser, start);
 	ser.unsigned_16(nsteps);
 
@@ -444,8 +415,7 @@
 }
 
 void CmdFlagAction::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_FLAGACTION);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_8(0);
 	ser.unsigned_32(serial);
 }
@@ -497,8 +467,7 @@
 }
 
 void CmdStartStopBuilding::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_STARTSTOPBUILDING);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 }
 
@@ -535,8 +504,7 @@
 }
 
 void CmdMilitarySiteSetSoldierPreference::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_MILITARYSITESETSOLDIERPREFERENCE);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 	ser.unsigned_8(static_cast<uint8_t>(preference));
 }
@@ -605,8 +573,7 @@
 }
 
 void CmdStartOrCancelExpedition::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_SHIP_EXPEDITION);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 }
 
@@ -653,8 +620,7 @@
 }
 
 void CmdEnhanceBuilding::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_ENHANCEBUILDING);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 	ser.unsigned_16(bi);
 }
@@ -700,8 +666,7 @@
 }
 
 void CmdDismantleBuilding::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_DISMANTLEBUILDING);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 }
 
@@ -744,8 +709,7 @@
 }
 
 void CmdEvictWorker::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_EVICTWORKER);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 }
 
@@ -799,8 +763,7 @@
 }
 
 void CmdShipScoutDirection::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_SHIP_SCOUT);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 	ser.unsigned_8(static_cast<uint8_t>(dir));
 }
@@ -857,8 +820,7 @@
 }
 
 void CmdShipConstructPort::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_SHIP_CONSTRUCT);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 	write_coords_32(&ser, coords);
 }
@@ -918,8 +880,7 @@
 }
 
 void CmdShipExploreIsland::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_SHIP_EXPLORE);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 	ser.unsigned_8(static_cast<uint8_t>(island_explore_direction));
 }
@@ -967,8 +928,7 @@
 }
 
 void CmdShipSink::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_SHIP_SINK);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 }
 
@@ -1012,8 +972,7 @@
 }
 
 void CmdShipCancelExpedition::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_SHIP_CANCELEXPEDITION);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 }
 
@@ -1124,8 +1083,7 @@
 }
 
 void CmdSetWarePriority::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_SETWAREPRIORITY);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial_);
 	ser.unsigned_8(type_);
 	ser.signed_32(index_);
@@ -1238,8 +1196,7 @@
 }
 
 void CmdSetInputMaxFill::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_SETINPUTMAXFILL);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial_);
 	ser.signed_32(index_);
 	if (type_ == wwWARE) {
@@ -1279,7 +1236,7 @@
 }
 
 void CmdChangeTargetQuantity::serialize(StreamWrite& ser) {
-	ser.unsigned_8(sender());
+	// Subclasses take care of writing their id() and sender()
 	ser.unsigned_32(economy());
 	ser.unsigned_8(ware_type());
 }
@@ -1330,7 +1287,7 @@
 }
 
 void CmdSetWareTargetQuantity::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_SETWARETARGETQUANTITY);
+	write_id_and_sender(ser);
 	CmdChangeTargetQuantity::serialize(ser);
 	ser.unsigned_32(permanent_);
 }
@@ -1377,7 +1334,7 @@
 }
 
 void CmdResetWareTargetQuantity::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_RESETWARETARGETQUANTITY);
+	write_id_and_sender(ser);
 	CmdChangeTargetQuantity::serialize(ser);
 }
 
@@ -1428,7 +1385,7 @@
 }
 
 void CmdSetWorkerTargetQuantity::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_SETWORKERTARGETQUANTITY);
+	write_id_and_sender(ser);
 	CmdChangeTargetQuantity::serialize(ser);
 	ser.unsigned_32(permanent_);
 }
@@ -1479,7 +1436,7 @@
 }
 
 void CmdResetWorkerTargetQuantity::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_RESETWORKERTARGETQUANTITY);
+	write_id_and_sender(ser);
 	CmdChangeTargetQuantity::serialize(ser);
 }
 
@@ -1497,8 +1454,7 @@
 }
 
 void CmdChangeTrainingOptions::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_CHANGETRAININGOPTIONS);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 	ser.unsigned_8(static_cast<uint8_t>(attribute));
 	ser.unsigned_16(value);
@@ -1550,8 +1506,7 @@
 }
 
 void CmdDropSoldier::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_DROPSOLDIER);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 	ser.unsigned_32(soldier);
 }
@@ -1625,8 +1580,7 @@
 }
 
 void CmdChangeSoldierCapacity::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_CHANGESOLDIERCAPACITY);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(serial);
 	ser.signed_16(val);
 }
@@ -1702,8 +1656,7 @@
 }
 
 void CmdEnemyFlagAction::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_ENEMYFLAGACTION);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_8(1);
 	ser.unsigned_32(serial);
 	ser.unsigned_8(sender());
@@ -1808,8 +1761,7 @@
 }
 
 void CmdMessageSetStatusRead::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_MESSAGESETSTATUSREAD);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(message_id().value());
 }
 
@@ -1821,8 +1773,7 @@
 }
 
 void CmdMessageSetStatusArchived::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_MESSAGESETSTATUSARCHIVED);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(message_id().value());
 }
 
@@ -1888,8 +1839,7 @@
 }
 
 void CmdSetStockPolicy::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_SETSTOCKPOLICY);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(warehouse_);
 	ser.unsigned_8(isworker_);
 	ser.unsigned_8(ware_);
@@ -1972,8 +1922,7 @@
 }
 
 void CmdProposeTrade::serialize(StreamWrite& ser) {
-	ser.unsigned_8(PLCMD_PROPOSE_TRADE);
-	ser.unsigned_8(sender());
+	write_id_and_sender(ser);
 	ser.unsigned_32(trade_.initiator);
 	ser.unsigned_32(trade_.receiver);
 	serialize_bill_of_materials(trade_.items_to_send, &ser);
 
=== modified file 'src/logic/playercommand.h'
--- src/logic/playercommand.h	2019-06-23 14:28:50 +0000
+++ src/logic/playercommand.h	2019-08-26 10:38:04 +0000
@@ -52,6 +52,8 @@
 	PlayerCommand() : GameLogicCommand(0), sender_(0), cmdserial_(0) {
 	}
 
+	void write_id_and_sender(StreamWrite& ser);
+
 	PlayerNumber sender() const {
 		return sender_;
 	}
@@ -62,6 +64,7 @@
 		cmdserial_ = s;
 	}
 
+	// For networking and replays
 	virtual void serialize(StreamWrite&) = 0;
 	static Widelands::PlayerCommand* deserialize(StreamRead&);
 
@@ -138,7 +141,7 @@
 	void read(FileRead&, EditorGameBase&, MapObjectLoader&) override;
 
 	QueueCommandTypes id() const override {
-		return QueueCommandTypes::kFlag;
+		return QueueCommandTypes::kBuildFlag;
 	}
 
 	void execute(Game&) override;
@@ -207,7 +210,7 @@
 	void read(FileRead&, EditorGameBase&, MapObjectLoader&) override;
 
 	QueueCommandTypes id() const override {
-		return QueueCommandTypes::kStopBuilding;
+		return QueueCommandTypes::kStartStopBuilding;
 	}
 
 	explicit CmdStartStopBuilding(StreamRead&);
@@ -257,7 +260,7 @@
 	void read(FileRead&, EditorGameBase&, MapObjectLoader&) override;
 
 	QueueCommandTypes id() const override {
-		return QueueCommandTypes::kPortStartExpedition;
+		return QueueCommandTypes::kStartOrCancelExpedition;
 	}
 
 	explicit CmdStartOrCancelExpedition(StreamRead&);
@@ -356,7 +359,7 @@
 	void read(FileRead&, EditorGameBase&, MapObjectLoader&) override;
 
 	QueueCommandTypes id() const override {
-		return QueueCommandTypes::kShipScout;
+		return QueueCommandTypes::kShipScoutDirection;
 	}
 
 	explicit CmdShipScoutDirection(StreamRead&);
@@ -408,7 +411,7 @@
 	void read(FileRead&, EditorGameBase&, MapObjectLoader&) override;
 
 	QueueCommandTypes id() const override {
-		return QueueCommandTypes::kShipExplore;
+		return QueueCommandTypes::kShipExploreIsland;
 	}
 
 	explicit CmdShipExploreIsland(StreamRead&);
@@ -431,7 +434,7 @@
 	void read(FileRead&, EditorGameBase&, MapObjectLoader&) override;
 
 	QueueCommandTypes id() const override {
-		return QueueCommandTypes::kSinkShip;
+		return QueueCommandTypes::kShipSink;
 	}
 
 	explicit CmdShipSink(StreamRead&);
 
=== modified file 'src/logic/queue_cmd_factory.cc'
--- src/logic/queue_cmd_factory.cc	2019-06-09 10:33:51 +0000
+++ src/logic/queue_cmd_factory.cc	2019-08-26 10:38:04 +0000
@@ -34,13 +34,13 @@
 	switch (id) {
 	case QueueCommandTypes::kBuild:
 		return *new CmdBuild();
-	case QueueCommandTypes::kFlag:
+	case QueueCommandTypes::kBuildFlag:
 		return *new CmdBuildFlag();
 	case QueueCommandTypes::kBuildRoad:
 		return *new CmdBuildRoad();
 	case QueueCommandTypes::kFlagAction:
 		return *new CmdFlagAction();
-	case QueueCommandTypes::kStopBuilding:
+	case QueueCommandTypes::kStartStopBuilding:
 		return *new CmdStartStopBuilding();
 	case QueueCommandTypes::kEnhanceBuilding:
 		return *new CmdEnhanceBuilding();
@@ -80,17 +80,17 @@
 		return *new CmdMilitarySiteSetSoldierPreference();
 	case QueueCommandTypes::kProposeTrade:
 		return *new CmdProposeTrade();
-	case QueueCommandTypes::kSinkShip:
+	case QueueCommandTypes::kShipSink:
 		return *new CmdShipSink();
 	case QueueCommandTypes::kShipCancelExpedition:
 		return *new CmdShipCancelExpedition();
-	case QueueCommandTypes::kPortStartExpedition:
+	case QueueCommandTypes::kStartOrCancelExpedition:
 		return *new CmdStartOrCancelExpedition();
 	case QueueCommandTypes::kShipConstructPort:
 		return *new CmdShipConstructPort();
-	case QueueCommandTypes::kShipScout:
+	case QueueCommandTypes::kShipScoutDirection:
 		return *new CmdShipScoutDirection();
-	case QueueCommandTypes::kShipExplore:
+	case QueueCommandTypes::kShipExploreIsland:
 		return *new CmdShipExploreIsland();
 	case QueueCommandTypes::kDestroyMapObject:
 		return *new CmdDestroyMapObject();
 
=== modified file 'src/logic/queue_cmd_ids.h'
--- src/logic/queue_cmd_ids.h	2019-06-09 10:33:51 +0000
+++ src/logic/queue_cmd_ids.h	2019-08-26 10:38:04 +0000
@@ -33,17 +33,21 @@
 
 namespace Widelands {
 
-enum class QueueCommandTypes {
+// The command types are used by the QueueCmdFactory, for network serialization
+// and for savegame compatibility.
+// DO NOT change the order
+// TODO(GunChleoc): Whenever we break savegame compatibility, clean this up and change data type to uint16_t.
+enum class QueueCommandTypes : uint8_t {
 
 	/* ID zero is reserved and must never be used */
 	kNone = 0,
 
 	/* PLAYER COMMANDS BELOW */
 	kBuild,
-	kFlag,
+	kBuildFlag,
 	kBuildRoad,
 	kFlagAction,
-	kStopBuilding,
+	kStartStopBuilding,
 	kEnhanceBuilding,
 	kBulldoze,
 
@@ -73,12 +77,14 @@
 	kMilitarysiteSetSoldierPreference,
 	kProposeTrade,  // 27
 
-	kSinkShip = 121,
+	kShipSink = 121,
 	kShipCancelExpedition,
-	kPortStartExpedition,
+	kStartOrCancelExpedition,
 	kShipConstructPort,
-	kShipScout,
-	kShipExplore,
+	kShipScoutDirection,
+	kShipExploreIsland,
+
+	// The commands below are never serialized, but we still keep the IDs stable for savegame compatibility.
 
 	kDestroyMapObject,
 	kAct,  // 128
 
=== modified file 'src/logic/replay.cc'
--- src/logic/replay.cc	2019-02-24 22:50:04 +0000
+++ src/logic/replay.cc	2019-08-26 10:38:04 +0000
@@ -38,8 +38,9 @@
 namespace Widelands {
 
 // File format definitions
+constexpr uint32_t kReplayKnownToDesync = 0x2E21A100;
 constexpr uint32_t kReplayMagic = 0x2E21A101;
-constexpr uint8_t kCurrentPacketVersion = 2;
+constexpr uint8_t kCurrentPacketVersion = 3;
 constexpr uint32_t kSyncInterval = 200;
 
 enum { pkt_end = 2, pkt_playercommand = 3, pkt_syncreport = 4 };
@@ -93,13 +94,15 @@
 
 	try {
 		const uint32_t magic = cmdlog_->unsigned_32();
-		if (magic == 0x2E21A100)
+		if (magic == kReplayKnownToDesync) {
 			// Note: This was never released as part of a build
 			throw wexception("%s is a replay from a version that is known to have desync "
 			                 "problems",
 			                 filename.c_str());
-		if (magic != kReplayMagic)
+		}
+		if (magic != kReplayMagic) {
 			throw wexception("%s apparently not a valid replay file", filename.c_str());
+		}
 
 		const uint8_t packet_version = cmdlog_->unsigned_8();
 		if (packet_version != kCurrentPacketVersion) {