widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

I added some more comments for a follow-up branch.

Diff comments:

> 
> === 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"));

DescriptionIndex carrier() const; is equivalent to worker_index("barbarians_carrier"), the only difference is that you access it via the building's owner's tribe, thus giving you the correct carrier rather than a Barbarian carrier for all tribes.

we could also consider having a working position for an inkeeper or some such, to sort out the accomodation for tired traders.

> +	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_;

Forget my comment, now that I think about it, this is actually better - working position is for workers who stay at the building or just roam a bit.

>  };
>  
>  class Market : public 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");

Of course, 1 thing at a time :)

> +		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);

OK :)

> +	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
> @@ -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.

that's certainly a usecase that I didn't think of. Let's keep this more open then, unless it's creating difficulties with the implementation or the UI.

> +	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.