← Back to team overview

widelands-dev team mailing list archive

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

 

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 +0000
> +++ src/logic/game.cc	2017-09-23 15:17:19 +0000
> @@ -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<Market*>(objects().get_object(trade.initiator));
> +	auto* receiver = dynamic_cast<Market*>(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<Market*>(objects().get_object(trade.initiator));
> +	auto* receiver = dynamic_cast<Market*>(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<Market*>(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<Market*>(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<LuaGameInterface&>(EditorGameBase::lua());
>  }
> 
> === modified file 'src/logic/map_objects/tribes/market.cc'
> --- src/logic/map_objects/tribes/market.cc	2017-09-18 13:43:08 +0000
> +++ src/logic/map_objects/tribes/market.cc	2017-09-23 15:17:19 +0000
> @@ -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 to implement 'set_economy'. Maybe common code can be shared.
>  Market::Market(const MarketDescr& the_descr) : Building(the_descr) {
>  }
>  
>  Market::~Market() {
>  }
>  
> +void Market::new_trade(const int trade_id,
> +                       const BillOfMaterials& items,
> +                       const int num_batches,
> +                       const Serial other_side) {
> +	trade_orders_[trade_id] = TradeOrder{items, num_batches, 0, other_side, 0, nullptr, {}};
> +	auto& trade_order = trade_orders_[trade_id];
> +
> +	// Request one worker for each item in a batch.
> +	trade_order.worker_request.reset(
> +	   new Request(*this, descr().carrier(), Market::worker_arrived_callback, wwWORKER));
> +	trade_order.worker_request->set_count(trade_order.num_wares_per_batch());
> +
> +	// Make sure we have a wares queue for each item in this.
> +	for (const auto& entry : items) {
> +		ensure_wares_queue_exists(entry.first);
> +	}
> +}
> +
> +void Market::cancel_trade(const int trade_id) {
> +	// TODO(sirver,trading): Launch workers, release no longer required wares and delete now unneeded 'WaresQueue's
> +	trade_orders_.erase(trade_id);
> +}
> +
> +void Market::worker_arrived_callback(
> +   Game& game, Request& rq, DescriptionIndex /* widx */, Worker* const w, PlayerImmovable& target) {
> +	auto& market = dynamic_cast<Market&>(target);
> +
> +	assert(w);
> +	assert(w->get_location(game) == &market);
> +
> +	for (auto& trade_order_pair : market.trade_orders_) {
> +		auto& trade_order = trade_order_pair.second;
> +		if (trade_order.worker_request.get() != &rq) {
> +			continue;
> +		}
> +
> +		if (rq.get_count() == 0) {
> +			// Erase this request.
> +			trade_order.worker_request.reset();
> +		}
> +		w->start_task_idle(game, 0, -1);
> +		trade_order.workers.push_back(w);
> +		Notifications::publish(NoteBuilding(market.serial(), NoteBuilding::Action::kWorkersChanged));
> +		market.try_launching_batch(&game);
> +		return;
> +	}
> +	NEVER_HERE(); // We should have found and handled a match by now.
> +}
> +
> +void Market::ware_arrived_callback(Game& g, InputQueue*, DescriptionIndex, Worker*, void* data) {
> +	Market& market = *static_cast<Market*>(data);
> +	market.try_launching_batch(&g);
> +}
> +
> +void Market::try_launching_batch(Game* game) {
> +	for (auto& pair : trade_orders_) {
> +		if (!is_ready_to_launch_batch(pair.first)) {
> +			continue;
> +		}
> +
> +		auto* other_market =
> +		   dynamic_cast<Market*>(game->objects().get_object(pair.second.other_side));
> +		if (other_market == nullptr) {
> +			// TODO(sirver,trading): Can this even happen? Where is this function called from?
> +			// The other market seems to have vanished. The game tracks this and
> +			// should soon delete this trade request from us. We just ignore it.
> +			continue;
> +		}
> +		if (!other_market->is_ready_to_launch_batch(pair.first)) {
> +			continue;
> +		}
> +		launch_batch(pair.first, game);
> +		other_market->launch_batch(pair.first, game);
> +		break;
> +	}
> +}
> +
> +bool Market::is_ready_to_launch_batch(const int trade_id) {
> +	const auto it = trade_orders_.find(trade_id);
> +	if (it == trade_orders_.end()) {
> +		return false;
> +	}
> +	auto& trade_order = it->second;
> +	assert(!trade_order.fulfilled());
> +
> +	// Do we have all necessary wares for a batch?
> +	for (const auto& item_pair : trade_order.items) {
> +		const auto wares_it = wares_queue_.find(item_pair.first);
> +		if (wares_it == wares_queue_.end()) {
> +			return false;
> +		}
> +		if (wares_it->second->get_filled() < item_pair.second) {
> +			return false;
> +		}
> +	}
> +
> +	// Do we have enough people to carry wares?
> +	int num_available_carriers = 0;
> +	for (auto* worker : trade_order.workers) {
> +		num_available_carriers += worker->is_idle() ? 1 : 0;
> +	}
> +	return num_available_carriers == trade_order.num_wares_per_batch();
> +}
> +
> +void Market::launch_batch(const int trade_id, Game* game) {
> +	assert(is_ready_to_launch_batch(trade_id));
> +	auto& trade_order = trade_orders_.at(trade_id);
> +
> +	// Do we have all necessary wares for a batch?
> +	int worker_index = 0;
> +	for (const auto& item_pair : trade_order.items) {
> +		for (size_t i = 0; i < item_pair.second; ++i) {
> +			Worker* carrier = trade_order.workers.at(worker_index);
> +			++worker_index;
> +			assert(carrier->is_idle());
> +
> +			// Give the carrier a ware.
> +			WareInstance* ware =
> +			   new WareInstance(item_pair.first, game->tribes().get_ware_descr(item_pair.first));
> +			ware->init(*game);
> +			carrier->set_carried_ware(*game, ware);
> +
> +			// We have to remove this item from our economy. Otherwise it would be
> +			// considered idle (since it has no transport associated with it) and
> +			// the engine would want to transfer it to the next warehouse.
> +			ware->set_economy(nullptr);
> +			wares_queue_.at(item_pair.first)
> +			   ->set_filled(wares_queue_.at(item_pair.first)->get_filled() - 1);
> +
> +			// Send the carrier going.
> +			carrier->reset_tasks(*game);
> +			carrier->start_task_carry_trade_item(
> +			   *game, trade_id, ObjectPointer(game->objects().get_object(trade_order.other_side)));
> +		}
> +	}
> +}
> +
> +void Market::ensure_wares_queue_exists(int ware_index) {
> +	if (wares_queue_.count(ware_index) > 0) {
> +		return;
> +	}
> +	wares_queue_[ware_index] =
> +	   std::unique_ptr<WaresQueue>(new WaresQueue(*this, ware_index, kMaxPerItemTradeBatchSize));
> +	wares_queue_[ware_index]->set_callback(Market::ware_arrived_callback, this);
> +}
> +
> +InputQueue& Market::inputqueue(DescriptionIndex index, WareWorker ware_worker) {
> +	assert(ware_worker == wwWARE);
> +	auto it = wares_queue_.find(index);
> +	if (it != wares_queue_.end()) {
> +		return *it->second;
> +	}
> +	// The parent will throw an exception.
> +	return Building::inputqueue(index, ware_worker);
> +}
> +
> +void Market::cleanup(EditorGameBase& egbase) {
> +	for (auto& pair : wares_queue_) {
> +		pair.second->cleanup();
> +	}
> +	Building::cleanup(egbase);
> +}
> +
> +void Market::traded_ware_arrived(const int trade_id,  const DescriptionIndex ware_index, Game* game) {
> +	auto& trade_order = trade_orders_.at(trade_id);
> +
> +	WareInstance* ware = new WareInstance(ware_index, game->tribes().get_ware_descr(ware_index));
> +	ware->init(*game);
> +
> +	// TODO(sirver,trading): This is a hack. We should have a worker that
> +	// carriers stuff out. At the moment this assumes this market is barbarians
> +	// (which is always correct right now), creates a carrier for each received
> +	// ware to drop it off. The carrier then leaves the building and goes home.
> +	const WorkerDescr& w_desc =
> +	   *game->tribes().get_worker_descr(game->tribes().worker_index("barbarians_carrier"));

The problem is that this worker is not in the house. We have to define working_positions for the market so that we can actually have somebody on site that can carry out the wares. Right now we just create a carrier out of thin air - this has to go away eventually, so having the more explicit hack of creating a barbarian carrier will be more likely to get fixed.

> +	auto& worker = w_desc.create(*game, owner(), this, position_);
> +	worker.start_task_dropoff(*game, *ware);
> +	trade_order.received_traded_wares_in_this_batch += 1;
> +	owner().ware_produced(ware_index);
> +
> +	auto* other_market = dynamic_cast<Market*>(game->objects().get_object(trade_order.other_side));
> +	assert(other_market != nullptr);
> +	other_market->owner().ware_consumed(ware_index, 1);
> +	auto& other_trade_order = other_market->trade_orders_.at(trade_id);
> +	if (trade_order.received_traded_wares_in_this_batch == other_trade_order.num_wares_per_batch() &&
> +			other_trade_order.received_traded_wares_in_this_batch == trade_order.num_wares_per_batch()) {
> +		// This batch is completed.
> +		++trade_order.num_shipped_batches;
> +		trade_order.received_traded_wares_in_this_batch = 0;
> +		++other_trade_order.num_shipped_batches;
> +		other_trade_order.received_traded_wares_in_this_batch = 0;
> +		if (trade_order.fulfilled()) {
> +			assert(other_trade_order.fulfilled());
> +			// TODO(sirver,trading): This is not quite correct. This should for
> +			// example send a differnet message than actually canceling a trade.
> +			game->cancel_trade(trade_id);
> +		}
> +	}
> +}
> +
>  }  // namespace Widelands
> 
> === modified file 'src/logic/map_objects/tribes/market.h'
> --- src/logic/map_objects/tribes/market.h	2017-09-15 12:33:58 +0000
> +++ src/logic/map_objects/tribes/market.h	2017-09-23 15:17:19 +0000
> @@ -32,8 +36,10 @@
>  
>  	Building& create_object() const override;
>  
> +	DescriptionIndex carrier() const { return carrier_; }
> +
>  private:
> -	BillOfMaterials working_positions_;
> +	DescriptionIndex carrier_;

I do not follow what you mean. I removed working_positions_ here because it was not completely supported - it will be back in the next change. Instead I add a 'carrier_' which is a new property that only the market has, i.e. the worker that carriers the wares between the different markets.

>  };
>  
>  class Market : public Building {
> 
> === modified file 'src/logic/map_objects/tribes/worker.cc'
> --- src/logic/map_objects/tribes/worker.cc	2017-08-20 17:45:42 +0000
> +++ src/logic/map_objects/tribes/worker.cc	2017-09-23 15:17:19 +0000
> @@ -1594,6 +1595,88 @@
>  		send_signal(game, "update");
>  }
>  
> +// The task when a worker is part of the caravane that is trading items.
> +const Bob::Task Worker::taskCarryTradeItem = {
> +   "carry_trade_item", static_cast<Bob::Ptr>(&Worker::carry_trade_item_update), nullptr, nullptr, true};
> +
> +void Worker::start_task_carry_trade_item(Game& game,
> +                                         const int trade_id,
> +                                         ObjectPointer other_market) {
> +	push_task(game, taskCarryTradeItem);
> +	auto& state = top_state();
> +	state.ivar1 = 0;
> +	state.ivar2 = trade_id;
> +	state.objvar1 = other_market;
> +}
> +
> +// This is a state machine: leave building, go to the other market, drop off
> +// wares, and return.
> +void Worker::carry_trade_item_update(Game& game, State& state) {
> +	// Reset any signals that are not related to location
> +	std::string signal = get_signal();
> +	signal_handled();
> +	if (!signal.empty()) {
> +		// TODO(sirver,trading): Remove once signals are correctly handled.
> +		log("carry_trade_item_update: signal received: %s\n", signal.c_str());
> +	}
> +	if (signal == "evict") {
> +		return pop_task(game);
> +	}
> +
> +	// First of all, make sure we're outside
> +	if (state.ivar1 == 0) {
> +		start_task_leavebuilding(game, false);
> +		++state.ivar1;
> +		return;
> +	}
> +
> +	auto* other_market = dynamic_cast<Market*>(state.objvar1.get(game));
> +	if (state.ivar1 == 1) {
> +		// Arrived on site. Move to the building and advance our state.
> +		if (other_market->base_flag().get_position() == get_position()) {
> +			++state.ivar1;
> +			return start_task_move(
> +			   game, WALK_NW, descr().get_right_walk_anims(does_carry_ware()), true);
> +		}
> +
> +		// Otherwise continue making progress towards the other market.
> +		if (!start_task_movepath(game, other_market->base_flag().get_position(), 5,
> +		                         descr().get_right_walk_anims(does_carry_ware()))) {
> +			molog("carry_trade_item_update: Could not move to other flag.\n");
> +			// TODO(sirver,trading): something needs to happen here.
> +		}
> +		return;
> +	}
> +
> +	if (state.ivar1 == 2) {

I tried the switch but felt it reduced readability. else if is not necessary since all branches will early return.

> +		WareInstance* const ware = fetch_carried_ware(game);
> +		other_market->traded_ware_arrived(state.ivar2, ware->descr_index(), &game);
> +		ware->remove(game);
> +		++state.ivar1;
> +		start_task_move(game, WALK_SE, descr().get_right_walk_anims(does_carry_ware()), true);
> +		return;
> +	}
> +
> +	if (state.ivar1 == 3) {
> +		++state.ivar1;
> +		start_task_return(game, false);
> +		return;
> +	}
> +
> +	if (state.ivar1 == 4) {
> +		pop_task(game);
> +		start_task_idle(game, 0, -1);
> +		dynamic_cast<Market*>(get_location(game))->try_launching_batch(&game);
> +		return;
> +	}
> +	NEVER_HERE();
> +}
> +
> +void Worker::update_task_carry_trade_item(Game& game) {
> +	if (top_state().task == &taskCarryTradeItem)
> +		send_signal(game, "update");
> +}
> +
>  /**
>   * Evict the worker from its current building.
>   */
> 
> === modified file 'src/logic/playercommand.cc'
> --- src/logic/playercommand.cc	2017-06-25 21:56:53 +0000
> +++ src/logic/playercommand.cc	2017-09-23 15:17:19 +0000
> @@ -1784,4 +1805,75 @@
>  	fw.unsigned_8(ware_);
>  	fw.unsigned_8(static_cast<uint8_t>(policy_));
>  }
> +
> +CmdProposeTrade::CmdProposeTrade(uint32_t time, PlayerNumber pn, const Trade& trade)
> +   : PlayerCommand(time, pn), trade_(trade) {
> +}
> +
> +CmdProposeTrade::CmdProposeTrade() : PlayerCommand() {
> +}
> +
> +void CmdProposeTrade::execute(Game& game) {
> +	Player* plr = game.get_player(sender());
> +	if (plr == nullptr) {
> +		return;
> +	}
> +
> +	Market* initiator = dynamic_cast<Market*>(game.objects().get_object(trade_.initiator));
> +	if (initiator == nullptr) {
> +		log("CmdProposeTrade: initiator vanished or is not a market.\n");
> +		return;
> +	}
> +	if (&initiator->owner() != plr) {
> +		log("CmdProposeTrade: sender %u, but market owner %u\n", sender(),
> +		    initiator->owner().player_number());
> +		return;
> +	}
> +	Market* receiver = dynamic_cast<Market*>(game.objects().get_object(trade_.receiver));
> +	if (receiver == nullptr) {
> +		log("CmdProposeTrade: receiver vanished or is not a market.\n");
> +		return;
> +	}
> +	if (&initiator->owner() == &receiver->owner()) {
> +		log("CmdProposeTrade: Sending and receiving player are the same.\n");

That sounds like a great idea. Should be simple to implement as well, but I'd rather finish trade between different players.

> +		return;
> +	}
> +
> +	// TODO(sirver,trading): Maybe check connectivity between markets here and
> +	// report errors.
> +	game.propose_trade(trade_);
> +}
> +
> +CmdProposeTrade::CmdProposeTrade(StreamRead& des) : PlayerCommand(0, des.unsigned_8()) {
> +	trade_.initiator = des.unsigned_32();
> +	trade_.receiver = des.unsigned_32();
> +	trade_.send_items = deserialize_bill_of_materials(&des);

Changed this to items_to_send, similar for the received ones

> +	trade_.received_items = deserialize_bill_of_materials(&des);
> +	trade_.num_batches = des.signed_32();
> +}
> +
> +void CmdProposeTrade::serialize(StreamWrite& ser) {
> +	ser.unsigned_8(PLCMD_PROPOSE_TRADE);
> +	ser.unsigned_8(sender());
> +	ser.unsigned_32(trade_.initiator);
> +	ser.unsigned_32(trade_.receiver);
> +	serialize_bill_of_materials(trade_.send_items, &ser);
> +	serialize_bill_of_materials(trade_.received_items, &ser);
> +	ser.signed_32(trade_.num_batches);
> +}
> +
> +void CmdProposeTrade::read(FileRead& /* fr */,
> +                           EditorGameBase& /* egbase */,
> +                           MapObjectLoader& /* mol */) {
> +	// TODO(sirver,trading): Implement this.
> +	NEVER_HERE();
> +}
> +
> +void CmdProposeTrade::write(FileWrite& /* fw */,
> +                            EditorGameBase& /* egbase */,
> +                            MapObjectSaver& /* mos */) {
> +	// TODO(sirver,trading): Implement this.
> +	NEVER_HERE();
> +}
> +
>  }
> 
> === modified file 'src/scripting/lua_map.cc'
> --- src/scripting/lua_map.cc	2017-09-18 13:43:08 +0000
> +++ src/scripting/lua_map.cc	2017-09-23 15:17:19 +0000
> @@ -594,6 +595,52 @@
>  	}
>  	return 0;
>  }
> +
> +// Parses a table of name/count pairs as given from Lua.
> +void parse_wares_workers(lua_State* L,
> +                         int table_index,
> +                         const TribeDescr& tribe,
> +                         InputMap* ware_workers_list,
> +                         bool is_ware) {
> +	luaL_checktype(L, table_index, LUA_TTABLE);
> +	lua_pushnil(L);
> +	while (lua_next(L, table_index) != 0) {
> +		if (is_ware) {
> +			if (tribe.ware_index(luaL_checkstring(L, -2)) == INVALID_INDEX) {
> +				report_error(L, "Illegal ware %s", luaL_checkstring(L, -2));
> +			}
> +		} else {
> +			if (tribe.worker_index(luaL_checkstring(L, -2)) == INVALID_INDEX) {
> +				report_error(L, "Illegal worker %s", luaL_checkstring(L, -2));
> +			}
> +		}
> +
> +		if (is_ware) {

done.

> +			ware_workers_list->insert(
> +			   std::make_pair(std::make_pair(tribe.ware_index(luaL_checkstring(L, -2)),
> +			                                 Widelands::WareWorker::wwWARE),
> +			                  luaL_checkuint32(L, -1)));
> +		} else {
> +			ware_workers_list->insert(
> +			   std::make_pair(std::make_pair(tribe.worker_index(luaL_checkstring(L, -2)),
> +			                                 Widelands::WareWorker::wwWORKER),
> +			                  luaL_checkuint32(L, -1)));
> +		}
> +		lua_pop(L, 1);
> +	}
> +}
> +
> +BillOfMaterials parse_wares_as_bill_of_material(lua_State* L, int table_index,
> +		const TribeDescr& tribe) {
> +	InputMap input_map;
> +	parse_wares_workers(L, table_index, tribe, &input_map, true /* is_ware */);
> +	BillOfMaterials result;
> +	for (const auto& pair : input_map) {
> +		result.push_back(std::make_pair(pair.first.first, pair.second));
> +	}
> +	return result;
> +}
> +
>  }  // namespace
>  
>  /*
> @@ -5106,6 +5130,35 @@
>   ==========================================================
>   */
>  
> +/* RST
> +   .. method:: propose_trade(other_market, num_batches, send_items, received_items)
> +
> +      TODO(sirver,trading): document
> +
> +      :returns: :const:`nil`
> +*/
> +int LuaMarket::propose_trade(lua_State* L) {
> +	if (lua_gettop(L) != 5) {
> +		report_error(L, "Takes 4 arguments.");
> +	}
> +	Game& game = get_game(L);
> +	Market* self = get(L, game);
> +	Market* other_market = (*get_user_class<LuaMarket>(L, 2))->get(L, game);
> +	const int num_batches = luaL_checkinteger(L, 3);
> +
> +	const BillOfMaterials send_items = parse_wares_as_bill_of_material(L, 4, self->owner().tribe());
> +	// TODO(sirver,trading): unsure if correct. Test inter-tribe trading, i.e.
> +	// barbarians trading with empire, but shipping atlantean only wares.

I'd like to support sending any ware, not only the ones the tribe knows how to process. The idea is that an empire could trade with two different atlanteans and relay wares between them.

Not sure if that has any useful application, so maybe I'll go with your suggestion and only "useful" wares can be traded.

> +	const BillOfMaterials received_items = parse_wares_as_bill_of_material(L, 5, self->owner().tribe());
> +	const int trade_id = game.propose_trade(
> +	   Trade{send_items, received_items, num_batches, self->serial(), other_market->serial()});
> +
> +	// TODO(sirver,trading): Wrap 'Trade' into its own Lua class?
> +	lua_pushint32(L, trade_id);
> +	return 1;
> +}
> +
> +
>  /*
>   ==========================================================
>   C METHODS


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


References