[Widelands-dev] [Merge] lp:~widelands-dev/widelands/market1 into lp:widelands

2017-10-03 Thread noreply
The proposal to merge lp:~widelands-dev/widelands/market1 into lp:widelands has 
been updated.

Status: Needs review => Merged

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/market1/+merge/331233
-- 
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/market1.

___
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


Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/market1 into lp:widelands

2017-10-03 Thread SirVer
Review: Resubmit

Thank you for the code review! All addressed.

@bunnybot merge

Diff comments:

> 
> === modified file 'src/logic/game.cc'
> --- src/logic/game.cc 2017-08-30 12:01:47 +
> +++ src/logic/game.cc 2017-09-23 15:17:19 +
> @@ -753,6 +754,87 @@
>  get_gametime(), ship.get_owner()->player_number(), ship.serial()));
>  }
>  
> +void Game::send_player_propose_trade(const Trade& trade) {
> + auto* object = objects().get_object(trade.initiator);
> + assert(object != nullptr);
> + send_player_command(
> +*new CmdProposeTrade(get_gametime(), 
> object->get_owner()->player_number(), trade));
> +}
> +
> +int Game::propose_trade(const Trade& trade) {
> + // TODO(sirver,trading): Check if a trade is possible (i.e. if there is 
> a
> + // path between the two markets);
> + const int id = next_trade_agreement_id_;
> + ++next_trade_agreement_id_;
> +
> + auto* initiator = 
> dynamic_cast(objects().get_object(trade.initiator));
> + auto* receiver = 
> dynamic_cast(objects().get_object(trade.receiver));
> + // This is only ever called through a PlayerCommand and that already 
> made
> + // sure that the objects still exist. Since no time has passed, they 
> should
> + // not have vanished under us.
> + assert(initiator != nullptr);
> + assert(receiver != nullptr);
> +
> + receiver->removed.connect(
> +[this, id](const uint32_t /* serial */) { cancel_trade(id); });

unfortunately not, since the MapObject::remove slot defines the interface, I 
can't change it easily.

> + initiator->removed.connect(
> +[this, id](const uint32_t /* serial */) { cancel_trade(id); });
> +
> + receiver->send_message(*this, Message::Type::kTradeOfferReceived, 
> receiver->descr().descname(),
> +receiver->descr().icon_filename(), 
> receiver->descr().descname(),
> +_("This Market received a new trade offer."), 
> true);

done

> + trade_agreements_[id] = 
> TradeAgreement{TradeAgreement::State::kProposed, trade};
> +
> + // TODO(sirver,trading): this should be done through another 
> player_command, but I
> + // want to get to the trade logic implementation now.
> + accept_trade(id);
> + return id;
> +}
> +
> +void Game::accept_trade(const int trade_id) {
> + auto it = trade_agreements_.find(trade_id);
> + if (it == trade_agreements_.end()) {
> + log("Game::accept_trade: Trade %d has vanished. Ignoring.\n", 
> trade_id);
> + return;
> + }
> + const Trade& trade = it->second.trade;
> + auto* initiator = 
> dynamic_cast(objects().get_object(trade.initiator));
> + auto* receiver = 
> dynamic_cast(objects().get_object(trade.receiver));
> + if (initiator == nullptr || receiver == nullptr) {
> + cancel_trade(trade_id);
> + return;
> + }
> +
> + initiator->new_trade(trade_id, trade.send_items, trade.num_batches, 
> trade.receiver);
> + receiver->new_trade(trade_id, trade.received_items, trade.num_batches, 
> trade.initiator);
> +
> + // TODO(sirver,trading): Message the users that the trade has been 
> accepted.
> +}
> +
> +void Game::cancel_trade(int trade_id) {
> + // The trade id might be long gone - since we never disconnect from the
> + // 'removed' signal of the two buildings, we might be invoked long 
> after the
> + // trade was deleted for other reasons.
> + const auto it = trade_agreements_.find(trade_id);
> + if (it == trade_agreements_.end()) {
> + return;
> + }
> + const auto& trade = it->second.trade;
> +
> + auto* initiator = 
> dynamic_cast(objects().get_object(trade.initiator));
> + if (initiator != nullptr) {
> + initiator->cancel_trade(trade_id);
> + // TODO(sirver,trading): Send message to owner that the trade 
> has been canceled.
> + }
> +
> + auto* receiver = 
> dynamic_cast(objects().get_object(trade.receiver));
> + if (receiver != nullptr) {
> + receiver->cancel_trade(trade_id);
> + // TODO(sirver,trading): Send message to owner that the trade 
> has been canceled.
> + }
> + trade_agreements_.erase(trade_id);
> +}
> +
>  LuaGameInterface& Game::lua() {
>   return static_cast(EditorGameBase::lua());
>  }
> 
> === modified file 'src/logic/map_objects/tribes/market.cc'
> --- src/logic/map_objects/tribes/market.cc2017-09-18 13:43:08 +
> +++ src/logic/map_objects/tribes/market.cc2017-09-23 15:17:19 +
> @@ -38,10 +46,223 @@
>   return *new Market(*this);
>  }
>  
> +int Market::TradeOrder::num_wares_per_batch() const {
> + int sum = 0;
> + for (const auto& item_pair : items) {
> + sum += item_pair.second;
> + }
> + return sum;
> +}
> +
> +bool Market::TradeOrder::fulfilled() const {
> + return num_shipped_batches == initial_num_batches;
> +}
> +
> +// TODO(sirver,trading): This needs