Merge lp:~widelands-dev/widelands/bug-1489295-mine-is-exhausted into lp:widelands

[Toni Förster]

when main vein is exhausted, don't show tooltip for missing food

I tested this with barbarians and found no wrong tooltips. Needs extensive testing though.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4843. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/526562024.
Appveyor build 4624. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1489295_mine_is_exhausted-4624.

[Toni Förster (stonerl)]

It does currently not work for scouts because they do not set a proper out of resource header. So the "Did not start..." message is not shown.

Here is the log:

Production Result ()
Out of Resource ()
Production Result (Did not start scouting because Ration is missing)
Out of Resource ()
Production Result (Did not start scouting because Ration is missing)
Out of Resource ()

[Toni Förster (stonerl)]

the empty string check should do the trick.

[hessenfarmer (stephan-lutz)]

I don't get this.

According to your log the Production result was set properly so why do we need the empty string check?

and why doing it that complicated

why not

if ( ps.production_result() != ps.descr().out_of_resource_heading() || ps.descr().out_of_resource_heading().empty()) {
   ps.set_production_result(result_string);
  }

[Toni Förster (stonerl)]

The empty string check is necessary for buildings with an empty out_of_resource_heading. If we don't check for an empty string, the message "Did not start scouting because Ration is missing" for scouts will never show up, maybe other buildings will have missing tooltips as well (e.g. tavern/inn), if we don't check.

I did it the "complicated" way because I found it to be easier to read. And for log output (has to be removed later on though)

[Toni Förster (stonerl)]

I did only check for wrong tooltips. But forgot to check for missing tooltips. *stupidme

[hessenfarmer (stephan-lutz)]

Ok I just implem,ented my solution locally and it works as desired. Checked the scout with the help of the input slider. the did not scout message was turned in if no food and off if food supplied. rechecked lumberjach and arena as well. did see the correct did not start messages for brewery, bakery, tavern and inn. did even check the out of fields message for reed yard and farm. found no obvious errors anymore except one little thing.

while checking the coalmine I realized that if the mine finds some coal by the 5% chance it displays the produced coal message and afterwards the did not start message is shown again although the mine is depleted. So from my perspective we should supress the "produced..." mesage in the same way if the mine is depleted.

[Toni Förster (stonerl)]

Okay, then just push your implementation to this branch here. I don't mind at all.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4846. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/526796763.
Appveyor build 4627. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1489295_mine_is_exhausted-4627.

[hessenfarmer (stephan-lutz)]

do you agree we should suppress the rare success message if mine exhausted as well?

If yes I'll implement this to this branch.

Thanks for helping solving these very old and annoying things.

[Toni Förster (stonerl)]

Yes I agree :-)

[hessenfarmer (stephan-lutz)]

OK here are my fixes feel free to test and change again if not satisfied. I have not tested this final one though.

[Toni Förster (stonerl)]

Works as expected.

review: Approve (testing, playing, compiling)

[hessenfarmer (stephan-lutz)]

Thanks for the support about this topic.

let's have it then once our CI reports green

@bunnybot merge

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4848. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/526967000.
Appveyor build 4629. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1489295_mine_is_exhausted-4629.

[bunnybot (widelandsofficial)]

Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways.

Travis build 4848. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/526967000.

[Toni Förster (stonerl)]

Transient failure on Travis

@bunnybot merge force

=== modified file 'src/logic/map_objects/tribes/production_program.cc'
--- src/logic/map_objects/tribes/production_program.cc	2019-04-30 15:04:35 +0000
+++ src/logic/map_objects/tribes/production_program.cc	2019-05-01 17:12:13 +0000
@@ -565,8 +565,10 @@
 			                   .str();
 		}
 		}
-
-		ps.set_production_result(result_string);
+		if (ps.production_result() != ps.descr().out_of_resource_heading() || 
+			    ps.descr().out_of_resource_heading().empty()) {
+			ps.set_production_result(result_string);
+		}
 	}
 	return ps.program_end(game, result_);
 }
@@ -916,7 +918,10 @@
 		    is_missing_string)
 		      .str();
 
-		ps.set_production_result(result_string);
+		if (ps.production_result() != ps.descr().out_of_resource_heading() || 
+			    ps.descr().out_of_resource_heading().empty()) {
+			ps.set_production_result(result_string);
+		}
 		return ps.program_end(game, ProgramResult::kFailed);
 	} else {  //  we fulfilled all consumption requirements
 		for (size_t i = 0; i < inputqueues.size(); ++i) {
@@ -1006,7 +1011,10 @@
 	   /** TRANSLATORS: %s is a list of wares. String is fetched according to total amount of
 	      wares. */
 	   (boost::format(ngettext("Produced %s", "Produced %s", count)) % ware_list).str();
-	ps.set_production_result(result_string);
+	if (ps.production_result() != ps.descr().out_of_resource_heading() || 
+		    ps.descr().out_of_resource_heading().empty()) {
+		ps.set_production_result(result_string);
+	}
 }
 
 bool ProductionProgram::ActProduce::get_building_work(Game& game,