widelands-dev team mailing list archive

[Benedikt Straub <benedikt-straub@xxxxxx>]

Benedikt Straub has proposed merging lp:~widelands-dev/widelands/productionsite-bugs into lp:widelands.

Commit message:
– Switch building to unoccupied animation when worker is removed
– Allow basic tasks like carrying out superfluous wares even if not all workers are present yet

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1643170 in widelands: "Decreasing number of wares, when working slot is vacant"
  https://bugs.launchpad.net/widelands/+bug/1643170
  Bug #1699776 in widelands: "Switch buildings to idle/unoccupied animation when a worker is kicked out"
  https://bugs.launchpad.net/widelands/+bug/1699776

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/productionsite-bugs/+merge/367306
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/productionsite-bugs into lp:widelands.

=== modified file 'src/logic/map_objects/tribes/building.h'
--- src/logic/map_objects/tribes/building.h	2019-04-26 05:52:49 +0000
+++ src/logic/map_objects/tribes/building.h	2019-05-11 12:50:41 +0000
@@ -326,14 +326,14 @@
 	                  uint32_t throttle_time = 0,
 	                  uint32_t throttle_radius = 0);
 
+	void start_animation(EditorGameBase&, uint32_t anim);
+
 protected:
 	// Updates 'statistics_string' with the string that should be displayed for
 	// this building right now. Overwritten by child classes.
 	virtual void update_statistics_string(std::string*) {
 	}
 
-	void start_animation(EditorGameBase&, uint32_t anim);
-
 	bool init(EditorGameBase&) override;
 	void cleanup(EditorGameBase&) override;
 	void act(Game&, uint32_t data) override;

=== modified file 'src/logic/map_objects/tribes/production_program.cc'
--- src/logic/map_objects/tribes/production_program.cc	2019-05-05 14:05:07 +0000
+++ src/logic/map_objects/tribes/production_program.cc	2019-05-11 12:50:41 +0000
@@ -699,7 +699,7 @@
 
 void ProductionProgram::ActCallWorker::execute(Game& game, ProductionSite& ps) const {
 	// Always main worker is doing stuff
-	ps.working_positions_[0].worker->update_task_buildingwork(game);
+	ps.working_positions_[ps.main_worker_].worker->update_task_buildingwork(game);
 }
 
 bool ProductionProgram::ActCallWorker::get_building_work(Game& game,
@@ -985,7 +985,7 @@
 void ProductionProgram::ActProduce::execute(Game& game, ProductionSite& ps) const {
 	assert(ps.produced_wares_.empty());
 	ps.produced_wares_ = produced_wares_;
-	ps.working_positions_[0].worker->update_task_buildingwork(game);
+	ps.working_positions_[ps.main_worker_].worker->update_task_buildingwork(game);
 
 	const TribeDescr& tribe = ps.owner().tribe();
 	assert(produced_wares_.size());
@@ -1071,7 +1071,7 @@
 void ProductionProgram::ActRecruit::execute(Game& game, ProductionSite& ps) const {
 	assert(ps.recruited_workers_.empty());
 	ps.recruited_workers_ = recruited_workers_;
-	ps.working_positions_[0].worker->update_task_buildingwork(game);
+	ps.working_positions_[ps.main_worker_].worker->update_task_buildingwork(game);
 
 	const TribeDescr& tribe = ps.owner().tribe();
 	assert(recruited_workers_.size());
@@ -1523,7 +1523,7 @@
 	if (map.find_reachable_immovables(area, &immovables, cstep, FindImmovableByDescr(descr))) {
 		state.objvar = immovables[0].object;
 
-		psite.working_positions_[0].worker->update_task_buildingwork(game);
+		psite.working_positions_[psite.main_worker_].worker->update_task_buildingwork(game);
 		return;
 	}
 
@@ -1559,7 +1559,7 @@
 
 		state.coord = best_coords;
 
-		psite.working_positions_[0].worker->update_task_buildingwork(game);
+		psite.working_positions_[psite.main_worker_].worker->update_task_buildingwork(game);
 		return;
 	}
 

=== modified file 'src/logic/map_objects/tribes/productionsite.cc'
--- src/logic/map_objects/tribes/productionsite.cc	2019-05-05 14:05:07 +0000
+++ src/logic/map_objects/tribes/productionsite.cc	2019-05-11 12:50:41 +0000
@@ -270,7 +270,8 @@
      last_stat_percent_(0),
      crude_percent_(0),
      is_stopped_(false),
-     default_anim_("idle") {
+     default_anim_("idle"),
+     main_worker_(-1) {
 	calc_statistics();
 }
 
@@ -575,6 +576,9 @@
 				// was
 				// evicted to make place for a level 0 worker.
 				// Therefore we again request the worker from the WorkingPosition of descr()
+				if (main_worker_ == worker_index) {
+					main_worker_ = -1;
+				}
 				*wp = WorkingPosition(&request_worker(worker_index), nullptr);
 				Building::remove_worker(w);
 				return;
@@ -685,11 +689,14 @@
 		program_timer_ = false;
 
 		if (!can_start_working()) {
-			while (!stack_.empty())
+			start_animation(game, descr().get_unoccupied_animation());
+			while (!stack_.empty()) {
 				program_end(game, ProgramResult::kFailed);
+			}
 		} else {
+			assert(main_worker_ >= 0);
 			if (stack_.empty()) {
-				working_positions_[0].worker->update_task_buildingwork(game);
+				working_positions_[main_worker_].worker->update_task_buildingwork(game);
 				return;
 			}
 
@@ -737,8 +744,10 @@
 bool ProductionSite::fetch_from_flag(Game& game) {
 	++fetchfromflag_;
 
-	if (can_start_working())
-		working_positions_[0].worker->update_task_buildingwork(game);
+	if (main_worker_ >= 0) {
+		assert(working_positions_[main_worker_].worker);
+		working_positions_[main_worker_].worker->update_task_buildingwork(game);
+	}
 
 	return true;
 }
@@ -767,10 +776,19 @@
 }
 
 void ProductionSite::try_start_working(Game& game) {
-	if (can_start_working() && descr().working_positions().size()) {
-		Worker& main_worker = *working_positions_[0].worker;
-		main_worker.reset_tasks(game);
-		main_worker.start_task_buildingwork(game);
+	if (main_worker_ >= 0) {
+		return;
+	}
+	const size_t nr_workers = descr().working_positions().size();
+	for (uint32_t i = 0; i < nr_workers; ++i) {
+		if (Worker* worker = working_positions_[i].worker) {
+			// We may start even if can_start_working() returns false, because basic actions
+			// like unloading extra wares should take place anyway
+			main_worker_ = i;
+			worker->reset_tasks(game);
+			worker->start_task_buildingwork(game);
+			return;
+		}
 	}
 }
 
@@ -781,7 +799,8 @@
  */
 bool ProductionSite::get_building_work(Game& game, Worker& worker, bool const success) {
 	assert(descr().working_positions().size());
-	assert(&worker == working_positions_[0].worker);
+	assert(main_worker_ >= 0);
+	assert(&worker == working_positions_[main_worker_].worker);
 
 	// If unsuccessful: Check if we need to abort current program
 	if (!success) {
@@ -860,8 +879,11 @@
 	}
 
 	// Check if all workers are there
-	if (!can_start_working())
-		return false;
+	if (!can_start_working()) {
+		// Try again a bit later
+		worker.start_task_idle(game, 0, 3000);
+		return true;
+	}
 
 	// Start program if we haven't already done so
 	State* state = get_state();

=== modified file 'src/logic/map_objects/tribes/productionsite.h'
--- src/logic/map_objects/tribes/productionsite.h	2019-05-05 14:05:07 +0000
+++ src/logic/map_objects/tribes/productionsite.h	2019-05-11 12:50:41 +0000
@@ -333,6 +333,8 @@
 	std::string statistics_string_on_changed_statistics_;
 	std::string production_result_;  // hover tooltip text
 
+	int32_t main_worker_;
+
 	DISALLOW_COPY_AND_ASSIGN(ProductionSite);
 };
 

=== modified file 'src/logic/map_objects/tribes/worker.cc'
--- src/logic/map_objects/tribes/worker.cc	2019-05-04 10:47:44 +0000
+++ src/logic/map_objects/tribes/worker.cc	2019-05-11 12:50:41 +0000
@@ -1639,7 +1639,15 @@
 	// Reset any signals that are not related to location
 	std::string signal = get_signal();
 	signal_handled();
+
+	upcast(Building, building, get_location(game));
+
 	if (signal == "evict") {
+		if (building) {
+			// If the building was working, we do not tell it to cancel – it'll notice by itself soon –
+			// but we already change the animation so it won't look strange
+			building->start_animation(game, building->descr().get_unoccupied_animation());
+		}
 		return pop_task(game);
 	}
 
@@ -1647,7 +1655,6 @@
 		state.ivar1 = (signal == "fail") * 2;
 
 	// Return to building, if necessary
-	upcast(Building, building, get_location(game));
 	if (!building)
 		return pop_task(game);
 

=== modified file 'src/map_io/map_buildingdata_packet.cc'
--- src/map_io/map_buildingdata_packet.cc	2019-04-09 17:16:11 +0000
+++ src/map_io/map_buildingdata_packet.cc	2019-05-11 12:50:41 +0000
@@ -65,7 +65,7 @@
 // Responsible for warehouses and expedition bootstraps
 constexpr uint16_t kCurrentPacketVersionWarehouse = 7;
 constexpr uint16_t kCurrentPacketVersionMilitarysite = 6;
-constexpr uint16_t kCurrentPacketVersionProductionsite = 6;
+constexpr uint16_t kCurrentPacketVersionProductionsite = 7;
 constexpr uint16_t kCurrentPacketVersionTrainingsite = 5;
 
 void MapBuildingdataPacket::read(FileSystem& fs,
@@ -710,6 +710,12 @@
 				productionsite.statistics_[i] = fr.unsigned_8();
 			productionsite.statistics_string_on_changed_statistics_ = fr.c_string();
 			productionsite.production_result_ = fr.c_string();
+
+			if (kCurrentPacketVersionProductionsite >= 7) {
+				productionsite.main_worker_ = fr.signed_32();
+			} else {
+				productionsite.main_worker_ = productionsite.working_positions_[0].worker ? 0 : -1;
+			}
 		} else {
 			throw UnhandledVersionError("MapBuildingdataPacket - Productionsite", packet_version,
 			                            kCurrentPacketVersionProductionsite);
@@ -1173,6 +1179,8 @@
 		fw.unsigned_8(productionsite.statistics_[i]);
 	fw.string(productionsite.statistics_string_on_changed_statistics_);
 	fw.string(productionsite.production_result());
+
+	fw.signed_32(productionsite.main_worker_);
 }
 
 /*