Looks good to me. Some comments in the diff. Diff comments:
> === 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 > @@ -226,7 +200,7 @@ > } > > void CmdBulldoze::serialize(StreamWrite& ser) { > - ser.unsigned_8(PLCMD_BULLDOZE); > + write_id(ser); > ser.unsigned_8(sender()); These two lines are used pretty much everywhere. Pull out into a parent class method? > ser.unsigned_32(serial); > ser.unsigned_8(recurse); > @@ -1263,6 +1237,7 @@ > } > > void CmdChangeTargetQuantity::serialize(StreamWrite& ser) { > + // Does not implement id(), so we don't have any id header to serialize Why doesn´t it implement it? Looks like a bug to me… > ser.unsigned_8(sender()); > ser.unsigned_32(economy()); > ser.unsigned_8(ware_type()); > > === 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 { Since you´re already changing it, perhaps we should make it uint16_t so we won´t run out of IDs some day > > /* ID zero is reserved and must never be used */ > kNone = 0, > > /* PLAYER COMMANDS BELOW */ > kBuild, > - kFlag, > + kBuildFlag, > kBuildRoad, > kFlagAction, > - kStopBuilding, > + kStartStopBuilding, > kEnhanceBuilding, > kBulldoze, > -- https://code.launchpad.net/~widelands-dev/widelands/cleanup_playercommand_enums/+merge/369267 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/cleanup_playercommand_enums into lp:widelands. _______________________________________________ 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