GunChleoc has proposed merging lp:~widelands-dev/widelands/cleanup_playercommand_enums into lp:widelands.
Commit message: Get rid of duplicate listing of player commands. This breaks compatibility for replays only. Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net/~widelands-dev/widelands/cleanup_playercommand_enums/+merge/369267 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. -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/cleanup_playercommand_enums into lp:widelands.
=== modified file 'src/logic/playercommand.cc' --- src/logic/playercommand.cc 2019-06-23 11:41:17 +0000 +++ src/logic/playercommand.cc 2019-06-24 22:00:34 +0000 @@ -74,108 +74,82 @@ } // 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(StreamWrite& ser) { + ser.unsigned_8(static_cast<uint8_t>(id())); +} + 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,7 +200,7 @@ } void CmdBulldoze::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_BULLDOZE); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); ser.unsigned_8(recurse); @@ -273,7 +247,7 @@ } void CmdBuild::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_BUILD); + write_id(ser); ser.unsigned_8(sender()); ser.signed_16(bi); write_coords_32(&ser, coords); @@ -317,7 +291,7 @@ } void CmdBuildFlag::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_BUILDFLAG); + write_id(ser); ser.unsigned_8(sender()); write_coords_32(&ser, coords); } @@ -387,7 +361,7 @@ } void CmdBuildRoad::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_BUILDROAD); + write_id(ser); ser.unsigned_8(sender()); write_coords_32(&ser, start); ser.unsigned_16(nsteps); @@ -444,7 +418,7 @@ } void CmdFlagAction::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_FLAGACTION); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_8(0); ser.unsigned_32(serial); @@ -497,7 +471,7 @@ } void CmdStartStopBuilding::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_STARTSTOPBUILDING); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); } @@ -535,7 +509,7 @@ } void CmdMilitarySiteSetSoldierPreference::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_MILITARYSITESETSOLDIERPREFERENCE); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); ser.unsigned_8(static_cast<uint8_t>(preference)); @@ -605,7 +579,7 @@ } void CmdStartOrCancelExpedition::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_SHIP_EXPEDITION); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); } @@ -653,7 +627,7 @@ } void CmdEnhanceBuilding::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_ENHANCEBUILDING); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); ser.unsigned_16(bi); @@ -700,7 +674,7 @@ } void CmdDismantleBuilding::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_DISMANTLEBUILDING); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); } @@ -744,7 +718,7 @@ } void CmdEvictWorker::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_EVICTWORKER); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); } @@ -799,7 +773,7 @@ } void CmdShipScoutDirection::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_SHIP_SCOUT); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); ser.unsigned_8(static_cast<uint8_t>(dir)); @@ -857,7 +831,7 @@ } void CmdShipConstructPort::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_SHIP_CONSTRUCT); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); write_coords_32(&ser, coords); @@ -918,7 +892,7 @@ } void CmdShipExploreIsland::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_SHIP_EXPLORE); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); ser.unsigned_8(static_cast<uint8_t>(island_explore_direction)); @@ -967,7 +941,7 @@ } void CmdShipSink::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_SHIP_SINK); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); } @@ -1012,7 +986,7 @@ } void CmdShipCancelExpedition::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_SHIP_CANCELEXPEDITION); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); } @@ -1117,7 +1091,7 @@ } void CmdSetWarePriority::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_SETWAREPRIORITY); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial_); ser.unsigned_8(type_); @@ -1223,7 +1197,7 @@ } void CmdSetInputMaxFill::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_SETINPUTMAXFILL); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial_); ser.signed_32(index_); @@ -1263,6 +1237,7 @@ } void CmdChangeTargetQuantity::serialize(StreamWrite& ser) { + // Does not implement id(), so we don't have any id header to serialize ser.unsigned_8(sender()); ser.unsigned_32(economy()); ser.unsigned_8(ware_type()); @@ -1314,7 +1289,7 @@ } void CmdSetWareTargetQuantity::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_SETWARETARGETQUANTITY); + write_id(ser); CmdChangeTargetQuantity::serialize(ser); ser.unsigned_32(permanent_); } @@ -1361,7 +1336,7 @@ } void CmdResetWareTargetQuantity::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_RESETWARETARGETQUANTITY); + write_id(ser); CmdChangeTargetQuantity::serialize(ser); } @@ -1412,7 +1387,7 @@ } void CmdSetWorkerTargetQuantity::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_SETWORKERTARGETQUANTITY); + write_id(ser); CmdChangeTargetQuantity::serialize(ser); ser.unsigned_32(permanent_); } @@ -1463,7 +1438,7 @@ } void CmdResetWorkerTargetQuantity::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_RESETWORKERTARGETQUANTITY); + write_id(ser); CmdChangeTargetQuantity::serialize(ser); } @@ -1481,7 +1456,7 @@ } void CmdChangeTrainingOptions::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_CHANGETRAININGOPTIONS); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); ser.unsigned_8(static_cast<uint8_t>(attribute)); @@ -1534,7 +1509,7 @@ } void CmdDropSoldier::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_DROPSOLDIER); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); ser.unsigned_32(soldier); @@ -1609,7 +1584,7 @@ } void CmdChangeSoldierCapacity::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_CHANGESOLDIERCAPACITY); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(serial); ser.signed_16(val); @@ -1686,7 +1661,7 @@ } void CmdEnemyFlagAction::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_ENEMYFLAGACTION); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_8(1); ser.unsigned_32(serial); @@ -1792,7 +1767,7 @@ } void CmdMessageSetStatusRead::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_MESSAGESETSTATUSREAD); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(message_id().value()); } @@ -1805,7 +1780,7 @@ } void CmdMessageSetStatusArchived::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_MESSAGESETSTATUSARCHIVED); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(message_id().value()); } @@ -1872,7 +1847,7 @@ } void CmdSetStockPolicy::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_SETSTOCKPOLICY); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(warehouse_); ser.unsigned_8(isworker_); @@ -1956,7 +1931,7 @@ } void CmdProposeTrade::serialize(StreamWrite& ser) { - ser.unsigned_8(PLCMD_PROPOSE_TRADE); + write_id(ser); ser.unsigned_8(sender()); ser.unsigned_32(trade_.initiator); ser.unsigned_32(trade_.receiver); === modified file 'src/logic/playercommand.h' --- src/logic/playercommand.h 2019-06-19 07:34:19 +0000 +++ src/logic/playercommand.h 2019-06-24 22:00:34 +0000 @@ -52,6 +52,8 @@ PlayerCommand() : GameLogicCommand(0), sender_(0), cmdserial_(0) { } + void write_id(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-06-24 22:00:34 +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-06-24 22:00:34 +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. +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-06-24 22:00:34 +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) {
_______________________________________________ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp