Merge lp:~widelands-dev/widelands/bug-1829471-worker-preciousness into lp:widelands

[GunChleoc]

Some AI code cleanups

- Implement AI hints for workers
- Deduce whether a building is a barracks or recruits other workers from building outputs
- Move ware preciousness into a new AI hint object
- Fixed warning about signed/unsigned comparisons.

There is an assert that is failing now. Since I am not very familiar with the AI code, I don't know why it is there. @Tibor: Do you remember why you put it there?

[TiborB (tiborb95)]

See my comments below

BTW I am doing some training, I hope this will not interfere. Once it is merged, I will re-train too..

[TiborB (tiborb95)]

The logic is / was you cannot have max preciousness if you do not produce anything...

[GunChleoc (gunchleoc)]

Thanks, that fixed it!

There should be no real impact on training from this branch. This is a pure refactoring without any intended semantic changes except that the reindeer farm now has a minimum preciousness of 2.

[GunChleoc (gunchleoc)]

BTW that assert is already broken in trunk.

[TiborB (tiborb95)]

I will investigate that assert....

[TiborB (tiborb95)]

I am getting the failing assert for atlanteans horsefarm and barbarian cattlefarm - in trunk... They have max_needed_preciousness 1.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5015. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/534229981.
Appveyor build 4796. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1829471_worker_preciousness-4796.

[TiborB (tiborb95)]

Was you considering making barracks a recruitment? Because preciousness of barracks (output) is now hardcoded to 5, if I am right.

[GunChleoc (gunchleoc)]

I was only focusing on the code that hessenfarmer changed for the carrier2, but I think not having the barracks hard-coded would also be a good idea. I'll have a look.

[GunChleoc (gunchleoc)]

Done. No need to investigate the assert failure in trunk further, because it is fixed in this branch. It slipped past us when we reviewed hessenfarmer's branch.

[hessenfarmer (stephan-lutz)]

The failed assert came definitly from my changes. I did not compile and test them in debug, cause my machine is so slow. Sorry about that.
The problem is we were handling the recruitment as a producing building so it needed preciousness while in another part we handled it like a non producing building asserting that preciousness is 0.

The desired behaviour should be to handle the barracks and the recruiter like any other building with some output. This needs 2 things the definition of a preciousness for the soldier and the second carrier and some good decisiongates to be taken into account for determining neededness and priority for both of them.

[TiborB (tiborb95)]

AI code is simply overcomplicated...
Animals are also workers?
Do we expect that same workers will have no preciousness set?

The overall change looks good to me

review: Approve

[hessenfarmer (stephan-lutz)]

As soon as a worker is produced by a building he needs a preciousness (if we think of schools for example). If it is created in the warehouses he does not need a preciousness.

Maybe we have a problem now with the frisian reindeer farm, as it will now be identified as a recruitment site. this will limit the number of them to 2 which is far too less in terms of fur production. In my opinion we need to ensure, that the normal production part of a building takes precedence. But I need to make some tests to confirm this issue.

[GunChleoc (gunchleoc)]

Confirmed by looking at the code and fixed. It's fine that you're on release builds, I used to do that too before I bought my new machine.

And yes, animals are carriers, so they are workers.

Unlike for wares, preciousness is not mandatory for workers, since most of them aren't recruited by buildings.

[hessenfarmer (stephan-lutz)]

Hmm. I thought we are aiming at having more of the definition stuf in lua so we can change things without the need to recompile. I have the feeling that part of this branch is going in opposite direction. Fo the design of a new tribe I think I would like to have some flags in lua better than determine things from other values, cause this is giving us more flexibility.

[hessenfarmer (stephan-lutz)]

One thing I would like to have is a definition of normal AI limit to be able to have the limit values currently hardcoded in lua.

[GunChleoc (gunchleoc)]

There is a difference between hard-coding values like "preciousness = 5" and automatically deducing building types that get a completely different treatment and algorithm by the AI anyway.

[GunChleoc (gunchleoc)]

P.S. What we probably really should do here is to treat output workers in the exact same way as output wares and get rid of the special treatment code for recruiting sites, but that is out of scope for this branch.

[GunChleoc (gunchleoc)]

> One thing I would like to have is a definition of normal AI limit to be able to have the limit values currently hardcoded in lua.

Agreed, I have added it to the bug. I don't want to do it in this branch though, because then we will have to re-review the code in this branch for every new idea that we can come up. I prefer having separate branches for each issue.

[hessenfarmer (stephan-lutz)]

> There is a difference between hard-coding values like "preciousness = 5" and
> automatically deducing building types that get a completely different
> treatment and algorithm by the AI anyway.

Agreed on the difference. But not deducing the types and having flags instead gives us more flexibility. However for the moment being we will try to get this in until we need it the other way round again.

[hessenfarmer (stephan-lutz)]

I think the deduction of Recruitment of second carrier will not work this way as Outputs of recruitment sites contains the worker so it isn't empty. see inline comment as well

[hessenfarmer (stephan-lutz)]

Forget my last comment. I had the wrong conception of bo.outputs

So for me Code looks good will test this tonight.

review: Approve

[hessenfarmer (stephan-lutz)]

I have tested it now and found no anomalies (just tested with release build)

so from my side this could go in, although I would keep the first bug open.

[hessenfarmer (stephan-lutz)]

@bunnybot merge

=== modified file 'data/tribes/atlanteans.lua'
--- data/tribes/atlanteans.lua	2018-07-29 13:49:09 +0000
+++ data/tribes/atlanteans.lua	2019-05-19 16:18:33 +0000
@@ -375,7 +375,6 @@
    soldier = "atlanteans_soldier",
    ship = "atlanteans_ship",
    port = "atlanteans_port",
-   barracks = "atlanteans_barracks",
    ironore = "iron_ore",
    rawlog = "log",
    refinedlog = "planks",

=== modified file 'data/tribes/barbarians.lua'
--- data/tribes/barbarians.lua	2018-07-29 13:49:09 +0000
+++ data/tribes/barbarians.lua	2019-05-19 16:18:33 +0000
@@ -308,7 +308,6 @@
    soldier = "barbarians_soldier",
    ship = "barbarians_ship",
    port = "barbarians_port",
-   barracks = "barbarians_barracks",
    ironore = "iron_ore",
    rawlog = "log",
    refinedlog = "blackwood",

=== modified file 'data/tribes/buildings/productionsites/atlanteans/horsefarm/init.lua'
--- data/tribes/buildings/productionsites/atlanteans/horsefarm/init.lua	2019-05-15 06:29:24 +0000
+++ data/tribes/buildings/productionsites/atlanteans/horsefarm/init.lua	2019-05-19 16:18:33 +0000
@@ -31,7 +31,6 @@
    },
 
    aihints = {
-      recruitment = true,
       prohibited_till = 610,
    },
 

=== modified file 'data/tribes/buildings/productionsites/barbarians/cattlefarm/init.lua'
--- data/tribes/buildings/productionsites/barbarians/cattlefarm/init.lua	2019-05-15 06:29:24 +0000
+++ data/tribes/buildings/productionsites/barbarians/cattlefarm/init.lua	2019-05-19 16:18:33 +0000
@@ -31,7 +31,6 @@
    },
 
    aihints = {
-      recruitment = true,
       prohibited_till = 610,
    },
 

=== modified file 'data/tribes/buildings/productionsites/empire/donkeyfarm/init.lua'
--- data/tribes/buildings/productionsites/empire/donkeyfarm/init.lua	2019-05-15 06:29:24 +0000
+++ data/tribes/buildings/productionsites/empire/donkeyfarm/init.lua	2019-05-19 16:18:33 +0000
@@ -31,7 +31,6 @@
    },
 
    aihints = {
-      recruitment = true,
       prohibited_till = 610,
    },
 

=== modified file 'data/tribes/empire.lua'
--- data/tribes/empire.lua	2018-07-29 13:49:09 +0000
+++ data/tribes/empire.lua	2019-05-19 16:18:33 +0000
@@ -350,7 +350,6 @@
    soldier = "empire_soldier",
    ship = "empire_ship",
    port = "empire_port",
-   barracks = "empire_barracks",
    ironore = "iron_ore",
    rawlog = "log",
    refinedlog = "planks",

=== modified file 'data/tribes/frisians.lua'
--- data/tribes/frisians.lua	2019-02-28 11:03:51 +0000
+++ data/tribes/frisians.lua	2019-05-19 16:18:33 +0000
@@ -349,7 +349,6 @@
    soldier = "frisians_soldier",
    ship = "frisians_ship",
    port = "frisians_port",
-   barracks = "frisians_barracks",
    ironore = "iron_ore",
    rawlog = "log",
    refinedlog = "brick",

=== modified file 'data/tribes/workers/atlanteans/horse/init.lua'
--- data/tribes/workers/atlanteans/horse/init.lua	2017-02-12 09:10:57 +0000
+++ data/tribes/workers/atlanteans/horse/init.lua	2019-05-19 16:18:33 +0000
@@ -24,4 +24,10 @@
    ware_hotspot = {-2, 12},
 
    animations = animations,
+
+   aihints = {
+      preciousness = {
+         atlanteans = 2
+      },
+   }
 }

=== modified file 'data/tribes/workers/atlanteans/soldier/init.lua'
--- data/tribes/workers/atlanteans/soldier/init.lua	2019-05-18 12:05:22 +0000
+++ data/tribes/workers/atlanteans/soldier/init.lua	2019-05-19 16:18:33 +0000
@@ -284,6 +284,12 @@
       pictures = path.list_files(dirname .. "evade_level?.png"),
    },
 
+   aihints = {
+      preciousness = {
+         atlanteans = 5
+      },
+   },
+
    -- Random animations for battle
    -- TODO(GunChleoc): Make more animations to use the random function
    attack_success_w = {

=== modified file 'data/tribes/workers/barbarians/ox/init.lua'
--- data/tribes/workers/barbarians/ox/init.lua	2017-02-12 09:10:57 +0000
+++ data/tribes/workers/barbarians/ox/init.lua	2019-05-19 16:18:33 +0000
@@ -25,4 +25,10 @@
    ware_hotspot = { -2, 13 },
 
    animations = animations,
+
+   aihints = {
+      preciousness = {
+         barbarians = 2
+      },
+   }
 }

=== modified file 'data/tribes/workers/barbarians/soldier/init.lua'
--- data/tribes/workers/barbarians/soldier/init.lua	2019-04-26 19:10:45 +0000
+++ data/tribes/workers/barbarians/soldier/init.lua	2019-05-19 16:18:33 +0000
@@ -110,6 +110,12 @@
       pictures = path.list_files(dirname .. "evade_level?.png"),
    },
 
+   aihints = {
+      preciousness = {
+         barbarians = 5
+      },
+   },
+
    -- Random animations for battle
    attack_success_w = {
       atk_ok_w = all_levels_bar,

=== modified file 'data/tribes/workers/empire/donkey/init.lua'
--- data/tribes/workers/empire/donkey/init.lua	2017-02-12 09:10:57 +0000
+++ data/tribes/workers/empire/donkey/init.lua	2019-05-19 16:18:33 +0000
@@ -24,4 +24,10 @@
    ware_hotspot =  { -2, 8 },
 
    animations = animations,
+
+   aihints = {
+      preciousness = {
+         empire = 2
+      },
+   }
 }

=== modified file 'data/tribes/workers/empire/soldier/init.lua'
--- data/tribes/workers/empire/soldier/init.lua	2019-04-26 19:10:45 +0000
+++ data/tribes/workers/empire/soldier/init.lua	2019-05-19 16:18:33 +0000
@@ -110,6 +110,12 @@
       pictures = path.list_files(dirname .. "evade_level?.png"),
    },
 
+   aihints = {
+      preciousness = {
+         empire = 5
+      },
+   },
+
    -- Random animations for battle
    attack_success_w = {
       atk_ok_w = all_levels_emp,

=== modified file 'data/tribes/workers/frisians/reindeer/init.lua'
--- data/tribes/workers/frisians/reindeer/init.lua	2018-02-06 11:17:48 +0000
+++ data/tribes/workers/frisians/reindeer/init.lua	2019-05-19 16:18:33 +0000
@@ -24,4 +24,10 @@
    ware_hotspot = { 0, 18 },
 
    animations = animations,
+
+   aihints = {
+      preciousness = {
+         frisians = 2
+      },
+   }
 }

=== modified file 'data/tribes/workers/frisians/soldier/init.lua'
--- data/tribes/workers/frisians/soldier/init.lua	2019-05-18 20:43:25 +0000
+++ data/tribes/workers/frisians/soldier/init.lua	2019-05-19 16:18:33 +0000
@@ -434,6 +434,12 @@
       pictures = path.list_files (dirname .. "evade_level?.png"),
    },
 
+   aihints = {
+      preciousness = {
+         frisians = 5
+      },
+   },
+
    -- Random animations for battle
    attack_success_e = {
       atk_ok_1_w = fri_rookie,

=== modified file 'doc/sphinx/source/lua_tribes_workers.rst.org'
--- doc/sphinx/source/lua_tribes_workers.rst.org	2018-11-30 10:36:42 +0000
+++ doc/sphinx/source/lua_tribes_workers.rst.org	2019-05-19 16:18:33 +0000
@@ -17,60 +17,68 @@
 
 Workers are defined with Lua functions called ``new_<worker_type>_type{table}``. The contents of ``table`` depend on the type of worker that you are defining. The common properties shared by all workers are:
 
-  **msgctxt**: The context that Gettext will use to disambiguate the
-  translations for strings in this table.
-
-  **name**: A string containing the internal name of this worker.
-
-  **descname**: The translatable display name. Use ``pgettext`` with the
-  ``msgctxt`` above to fetch the string.
-
-  **helptext_script**: The full path to the ``helptexts.lua`` script for this worker.
-
-  **icon**: The full path to the menu icon for this worker.
-
-  **vision_range**
-      The size of the radius that the worker sees.
-
-  **buildcost**
-      *Optional*. A table with the wares and workers used by warehouses to
-      create this worker, containing warename - amount pairs, e.g.::
-
-          buildcost = { atlanteans_carrier = 1, hammer = 1 }
-
-  **default_target_quantity**:
-      *Optional*. An int defining the default target quantity for the worker's
-      tribe's economy. Use this if the worker is produced by a production site
-      rather than the warehouses. For example, ``default_target_quantity = 10``
-
-  **experience**
-      *Optional*. The amount of experience that the worker needs to gather
-      in order to be transformend into a higher worker type. If `becomes`
-      is defined, this needs to be set as well.
-
-  **becomes**
-      *Optional*. The name of the higher worker type that this worker will
-      transform to after gaining enough experience. If `experience`
-      is defined, this needs to be set as well.
-
-  **animations**:
-       A table containing all animations for this worker.
-       Workers have an "idle" animation. They also have directional animations
-       called "walk" and "walkload" which are defined with the help of
-       :func:`add_walking_animations`, plus additional :ref:`animations` used in their
-       worker programs. The "idle" and "walk" animations are mandatory.
-
-  **programs**:
-      *Optional*. If the worker leaves the building to do his work, the :ref:`tribes_worker_programs` that define which type of space or resource the worker has to find
-      on the map in order to do his work, and what that work is, including any
-      animations and sounds played.
-
-  **ware_hotspot**
-      *Optional*. The x, y coordinates for adjusting the placement of the
-      ware being carried. The default value is ``{0, 15}``. Increase ``x``
-      to shift the ware to the left and ``y`` to shift it upwards. For example::
-
-          ware_hotspot = { -2, 13 },
+**msgctxt**: The context that Gettext will use to disambiguate the
+translations for strings in this table.
+
+**name**: A string containing the internal name of this worker.
+
+**descname**: The translatable display name. Use ``pgettext`` with the
+``msgctxt`` above to fetch the string.
+
+**helptext_script**: The full path to the ``helptexts.lua`` script for this worker.
+
+**icon**: The full path to the menu icon for this worker.
+
+**vision_range**
+    The size of the radius that the worker sees.
+
+**buildcost**
+    *Optional*. A table with the wares and workers used by warehouses to
+    create this worker, containing warename - amount pairs, e.g.::
+
+        buildcost = { atlanteans_carrier = 1, hammer = 1 }
+
+**default_target_quantity**:
+    *Optional*. An int defining the default target quantity for the worker's
+    tribe's economy. Use this if the worker is produced by a production site
+    rather than the warehouses. For example, ``default_target_quantity = 10``
+
+**experience**
+    *Optional*. The amount of experience that the worker needs to gather
+    in order to be transformend into a higher worker type. If `becomes`
+    is defined, this needs to be set as well.
+
+**becomes**
+    *Optional*. The name of the higher worker type that this worker will
+    transform to after gaining enough experience. If `experience`
+    is defined, this needs to be set as well.
+
+**animations**:
+     A table containing all animations for this worker.
+     Workers have an "idle" animation. They also have directional animations
+     called "walk" and "walkload" which are defined with the help of
+     :func:`add_walking_animations`, plus additional :ref:`animations` used in their
+     worker programs. The "idle" and "walk" animations are mandatory.
+
+**programs**:
+    *Optional*. If the worker leaves the building to do his work, the
+    :ref:`tribes_worker_programs` that define which type of space or resource
+    the worker has to find on the map in order to do his work, and what that
+    work is, including any animations and sounds played.
+
+**ware_hotspot**
+    *Optional*. The x, y coordinates for adjusting the placement of the
+    ware being carried. The default value is ``{0, 15}``. Increase ``x``
+    to shift the ware to the left and ``y`` to shift it upwards. For example::
+
+        ware_hotspot = { -2, 13 },
+
+**aihints**
+    *Optional*. A list of hints for the AI. Can contain the following optional entries:
+
+        **preciousness**: How precious this worker is to each tribe. For example,
+        ``{ atlanteans = 0, empire = 1 }``. You can use this for workers that are recruited.
+
 
 .. _lua_tribes_workers_helptexts:
 

=== modified file 'src/ai/ai_help_structs.h'
--- src/ai/ai_help_structs.h	2019-05-05 14:05:07 +0000
+++ src/ai/ai_help_structs.h	2019-05-19 16:18:33 +0000
@@ -477,6 +477,7 @@
 	bool requires_supporters;
 
 	// information needed for decision on new building construction
+	int16_t initial_preciousness;
 	int16_t max_preciousness;
 	int16_t max_needed_preciousness;
 

=== modified file 'src/ai/ai_hints.cc'
--- src/ai/ai_hints.cc	2019-02-23 11:00:49 +0000
+++ src/ai/ai_hints.cc	2019-05-19 16:18:33 +0000
@@ -223,7 +223,6 @@
 BuildingHints::BuildingHints(std::unique_ptr<LuaTable> table)
    : mines_(table->has_key("mines") ? table->get_string("mines") : ""),
      needs_water_(table->has_key("needs_water") ? table->get_bool("needs_water") : false),
-     recruitment_(table->has_key("recruitment") ? table->get_bool("recruitment") : false),
      space_consumer_(table->has_key("space_consumer") ? table->get_bool("space_consumer") : false),
      expansion_(table->has_key("expansion") ? table->get_bool("expansion") : false),
      fighting_(table->has_key("fighting") ? table->get_bool("fighting") : false),
@@ -274,3 +273,28 @@
 	}
 	NEVER_HERE();
 }
+
+
+// TODO(GunChleoc): WareDescr has a bare "preciousness" table that should be moved below a new "aihints" table.
+void WareWorkerHints::read_preciousness(const LuaTable& table) {
+	for (const std::string& key : table.keys<std::string>()) {
+		preciousnesses_.insert(std::make_pair(key, table.get_int(key)));
+	}
+}
+
+/// Returns the preciousness of the ware, or kInvalidWare if the tribe doesn't use the ware.
+int WareWorkerHints::preciousness(const std::string& tribename) const {
+	if (preciousnesses_.count(tribename) > 0) {
+		return preciousnesses_.at(tribename);
+	}
+	return Widelands::kInvalidWare;
+}
+
+WareHints::WareHints(const LuaTable& table) : WareWorkerHints() {
+	read_preciousness(table);
+}
+
+
+WorkerHints::WorkerHints(const LuaTable& table) : WareWorkerHints() {
+	read_preciousness(*table.get_table("preciousness"));
+}

=== modified file 'src/ai/ai_hints.h'
--- src/ai/ai_hints.h	2019-02-23 11:00:49 +0000
+++ src/ai/ai_hints.h	2019-05-19 16:18:33 +0000
@@ -23,9 +23,11 @@
 #include <memory>
 #include <stdint.h>
 #include <string>
+#include <unordered_map>
 
 #include "base/log.h"
 #include "base/macros.h"
+#include "logic/widelands.h"
 #include "scripting/lua_table.h"
 
 namespace Widelands {
@@ -56,10 +58,6 @@
 		return needs_water_;
 	}
 
-	bool for_recruitment() const {
-		return recruitment_;
-	}
-
 	bool is_space_consumer() const {
 		return space_consumer_;
 	}
@@ -110,7 +108,6 @@
 private:
 	const std::string mines_;
 	const bool needs_water_;
-	const bool recruitment_;
 	const bool space_consumer_;
 	const bool expansion_;
 	const bool fighting_;
@@ -131,4 +128,29 @@
 	DISALLOW_COPY_AND_ASSIGN(BuildingHints);
 };
 
+/// Hints common to wares and workers
+struct WareWorkerHints {
+	WareWorkerHints() = default;
+
+	/// Returns the preciousness of the ware/worker, or kInvalidWare if the tribe doesn't use the ware/worker or the worker has no preciousness defined for the tribe.
+	int preciousness(const std::string& tribename) const;
+
+protected:
+	void read_preciousness(const LuaTable& table);
+
+private:
+	// tribename, preciousness. No default.
+	std::unordered_map<std::string, int> preciousnesses_;
+};
+
+/// Hints for wares
+struct WareHints : WareWorkerHints {
+	explicit WareHints(const LuaTable& table);
+};
+
+/// Hints for workers
+struct WorkerHints : WareWorkerHints {
+	explicit WorkerHints(const LuaTable& table);
+};
+
 #endif  // end of include guard: WL_AI_AI_HINTS_H

=== modified file 'src/ai/defaultai.cc'
--- src/ai/defaultai.cc	2019-05-17 11:45:39 +0000
+++ src/ai/defaultai.cc	2019-05-19 16:18:33 +0000
@@ -52,7 +52,7 @@
 #include "logic/player.h"
 #include "logic/playercommand.h"
 
-// following is in miliseconds (widelands counts time in ms)
+// following is in milliseconds (widelands counts time in ms)
 constexpr int kFieldInfoExpiration = 12 * 1000;
 constexpr int kMineFieldInfoExpiration = 20 * 1000;
 constexpr int kNewMineConstInterval = 19000;
@@ -517,7 +517,7 @@
 	for (DescriptionIndex i = 0; i < static_cast<DescriptionIndex>(game().tribes().nrwares()); ++i) {
 		wares.at(i).producers = 0;
 		wares.at(i).consumers = 0;
-		wares.at(i).preciousness = game().tribes().get_ware_descr(i)->preciousness(tribe_->name());
+		wares.at(i).preciousness = game().tribes().get_ware_descr(i)->ai_hints().preciousness(tribe_->name());
 	}
 
 	const DescriptionIndex& nr_buildings = game().tribes().nrbuildings();
@@ -637,9 +637,6 @@
 		if (bh.is_space_consumer()) {
 			bo.set_is(BuildingAttribute::kSpaceConsumer);
 		}
-		if (bh.for_recruitment()) {
-			bo.set_is(BuildingAttribute::kRecruitment);
-		}
 		bo.expansion_type = bh.is_expansion_type();
 		bo.fighting_type = bh.is_fighting_type();
 		bo.mountain_conqueror = bh.is_mountain_conqueror();
@@ -658,6 +655,7 @@
 			bo.set_is(BuildingAttribute::kPort);
 		}
 		bo.max_trainingsites_proportion = 100;
+		bo.initial_preciousness = 0;
 		bo.max_preciousness = 0;
 		bo.max_needed_preciousness = 0;
 
@@ -684,6 +682,26 @@
 			for (const DescriptionIndex& temp_output : prod.output_ware_types()) {
 				bo.outputs.push_back(temp_output);
 			}
+
+			// Read information about worker outputs
+			if (prod.output_worker_types().size() > 0) {
+				for (const DescriptionIndex& temp_output : prod.output_worker_types()) {
+					if (temp_output == tribe_->soldier()) {
+						bo.set_is(BuildingAttribute::kBarracks);
+					}
+					const WorkerHints* worker_hints = tribe_->get_worker_descr(temp_output)->ai_hints();
+					if (worker_hints != nullptr) {
+						const int worker_preciousness = worker_hints->preciousness(tribe_->name());
+						if (worker_preciousness != Widelands::kInvalidWare) {
+							bo.initial_preciousness += worker_preciousness;
+						}
+					}
+				}
+				if (!bo.is(BuildingAttribute::kBarracks) && bo.outputs.empty()) {
+					bo.set_is(BuildingAttribute::kRecruitment);
+				}
+			}
+
 			for (const auto& temp_position : prod.working_positions()) {
 				bo.positions.push_back(temp_position.first);
 			}
@@ -719,9 +737,6 @@
 			if (bh.is_shipyard()) {
 				bo.set_is(BuildingAttribute::kShipyard);
 			}
-			if (building_index == tribe_->barracks()) {
-				bo.set_is(BuildingAttribute::kBarracks);
-			}
 			// Identify refined log producer
 			if (bo.outputs.size() == 1 && bo.outputs[0] == tribe_->refinedlog()) {
 				bo.set_is(BuildingAttribute::kLogRefiner);
@@ -2489,20 +2504,23 @@
 
 			// Now verifying that all 'buildable' buildings has positive max_needed_preciousness
 			// if they have outputs, all other must have zero max_needed_preciousness
-			if ((bo.new_building == BuildingNecessity::kNeeded ||
-			     bo.new_building == BuildingNecessity::kForced ||
-			     bo.new_building == BuildingNecessity::kAllowed ||
-			     bo.new_building == BuildingNecessity::kNeededPending) &&
-			    (!bo.outputs.empty() || bo.is(BuildingAttribute::kBarracks))) {
+
+			if (bo.new_building == BuildingNecessity::kForbidden) {
+				bo.max_needed_preciousness = 0;
+			} else if ((bo.new_building == BuildingNecessity::kNeeded ||
+					 bo.new_building == BuildingNecessity::kForced ||
+					 bo.new_building == BuildingNecessity::kAllowed ||
+					 bo.new_building == BuildingNecessity::kNeededPending) &&
+					(!bo.outputs.empty() || bo.initial_preciousness > 0)) { // bo.initial_preciousness signals that we have a worker output
+				bo.max_needed_preciousness = std::max(bo.max_needed_preciousness, bo.initial_preciousness);
+				bo.max_preciousness = std::max(bo.max_preciousness, bo.initial_preciousness);
+
 				if (bo.max_needed_preciousness <= 0) {
-					throw wexception("AI: Max presciousness must not be <= 0 for building: %s",
-					                 bo.desc->name().c_str());
+					throw wexception("AI: Max preciousness must not be <= 0 for building: %s",
+									 bo.desc->name().c_str());
 				}
-			} else if (bo.new_building == BuildingNecessity::kForbidden) {
-				bo.max_needed_preciousness = 0;
 			} else {
 				// For other situations we make sure max_needed_preciousness is zero
-
 				assert(bo.max_needed_preciousness == 0);
 			}
 
@@ -2965,7 +2983,6 @@
 					}
 
 				} else if (bo.is(BuildingAttribute::kRecruitment)) {
-					bo.max_needed_preciousness = 2;
 					prio += bo.primary_priority;
 					prio -= bf->unowned_land_nearby * 2;
 					prio -= (bf->enemy_nearby) * 100;
@@ -4487,7 +4504,7 @@
 		       gametime &&
 		    site.site->can_start_working() &&
 		    get_stocklevel(*site.bo, gametime) >
-		       (std::abs(management_data.get_military_number_at(168)) / 5)) {
+		      static_cast<unsigned int>((std::abs(management_data.get_military_number_at(168)) / 5))) {
 
 			if (connected_to_wh) {
 				game().send_player_dismantle(*site.site);
@@ -4980,42 +4997,41 @@
 	// Let deal with productionsites now
 	// First we iterate over outputs of building, count warehoused stock
 	// and deciding if we have enough on stock (in warehouses)
-	bo.max_preciousness = 0;
-	bo.max_needed_preciousness = 0;
-
-	if (!bo.is(BuildingAttribute::kBarracks)) {  // barracks are now excluded from calculation
-		// preciousness is assigned below in this fuction
-		for (uint32_t m = 0; m < bo.outputs.size(); ++m) {
-			DescriptionIndex wt(static_cast<size_t>(bo.outputs.at(m)));
-
-			uint16_t target = tribe_->get_ware_descr(wt)->default_target_quantity(tribe_->name());
-			if (target == Widelands::kInvalidWare) {
-				target = kTargetQuantCap;
-			}
-			target /= 3;
-
-			// at least  1
-			target = std::max<uint16_t>(target, 1);
-
-			// it seems there are wares with 0 preciousness (no entry in init files?), but we need
-			// positive value here.
-			const uint16_t preciousness =
-			   std::max<uint16_t>(wares.at(bo.outputs.at(m)).preciousness, 1);
-
-			if (calculate_stocklevel(wt) < target ||
-			    site_needed_for_economy == BasicEconomyBuildingStatus::kEncouraged) {
-				if (bo.max_needed_preciousness < preciousness) {
-					bo.max_needed_preciousness = preciousness;
-				}
-				if (site_needed_for_economy == BasicEconomyBuildingStatus::kEncouraged) {
-					bo.max_needed_preciousness +=
-					   std::abs(management_data.get_military_number_at(144)) / 10;
-				}
-			}
-
-			if (bo.max_preciousness < preciousness) {
-				bo.max_preciousness = preciousness;
-			}
+
+	// Calulate preciousness
+	bo.max_preciousness = bo.initial_preciousness;
+	bo.max_needed_preciousness = bo.initial_preciousness;
+	for (uint32_t m = 0; m < bo.outputs.size(); ++m) {
+		DescriptionIndex wt(static_cast<size_t>(bo.outputs.at(m)));
+
+		uint16_t target = tribe_->get_ware_descr(wt)->default_target_quantity(tribe_->name());
+		if (target == Widelands::kInvalidWare) {
+			target = kTargetQuantCap;
+		}
+		target /= 3;
+
+		// at least  1
+		target = std::max<uint16_t>(target, 1);
+
+		// it seems there are wares with 0 preciousness (no entry in init files?), but we need
+		// positive value here.
+		// TODO(GunChleoc): Since we require in Tribes::postload() that this is set for all wares used by a tribe, something seems to be wrong here. It should always be > 0.
+		const uint16_t preciousness =
+		   std::max<uint16_t>(wares.at(bo.outputs.at(m)).preciousness, 1);
+
+		if (calculate_stocklevel(wt) < target ||
+			site_needed_for_economy == BasicEconomyBuildingStatus::kEncouraged) {
+			if (bo.max_needed_preciousness < preciousness) {
+				bo.max_needed_preciousness = preciousness;
+			}
+			if (site_needed_for_economy == BasicEconomyBuildingStatus::kEncouraged) {
+				bo.max_needed_preciousness +=
+				   std::abs(management_data.get_military_number_at(144)) / 10;
+			}
+		}
+
+		if (bo.max_preciousness < preciousness) {
+			bo.max_preciousness = preciousness;
 		}
 	}
 
@@ -5102,12 +5118,7 @@
 		}
 
 		if (bo.forced_after < gametime && bo.total_count() == 0 && !has_substitution_building) {
-			if (!bo.is(BuildingAttribute::kBarracks)) {
-				bo.max_needed_preciousness = bo.max_preciousness;
-			} else {
-				// barracks has no genuine preciousness as of now
-				bo.max_needed_preciousness = 5;
-			}
+			bo.max_needed_preciousness = bo.max_preciousness;
 			return BuildingNecessity::kForced;
 		} else if (bo.prohibited_till > gametime) {
 			return BuildingNecessity::kForbidden;
@@ -5417,7 +5428,8 @@
 				bo.max_preciousness = bo.max_needed_preciousness;
 				return BuildingNecessity::kNeeded;
 			} else {
-				assert(bo.max_needed_preciousness == 0);
+				bo.max_needed_preciousness = 0;
+				bo.max_preciousness = 0;
 				return BuildingNecessity::kForbidden;
 			}
 		} else if (bo.type == BuildingObserver::Type::kMine) {
@@ -5829,7 +5841,7 @@
 			return BuildingNecessity::kNeeded;
 		} else if (bo.inputs.size() == 1 &&
 		           calculate_stocklevel(static_cast<size_t>(bo.inputs.at(0))) >
-		              std::abs(management_data.get_military_number_at(171))) {
+		              static_cast<unsigned int>(std::abs(management_data.get_military_number_at(171)))) {
 			return BuildingNecessity::kNeeded;
 		} else {
 			return BuildingNecessity::kNotNeeded;

=== modified file 'src/logic/map_objects/tribes/tribe_descr.cc'
--- src/logic/map_objects/tribes/tribe_descr.cc	2019-05-04 10:47:44 +0000
+++ src/logic/map_objects/tribes/tribe_descr.cc	2019-05-19 16:18:33 +0000
@@ -199,7 +199,6 @@
 		}
 
 		port_ = add_special_building(table.get_string("port"));
-		barracks_ = add_special_building(table.get_string("barracks"));
 
 		ironore_ = add_special_ware(table.get_string("ironore"));
 		rawlog_ = add_special_ware(table.get_string("rawlog"));
@@ -328,10 +327,6 @@
 	assert(tribes_.building_exists(port_));
 	return port_;
 }
-DescriptionIndex TribeDescr::barracks() const {
-	assert(tribes_.building_exists(barracks_));
-	return barracks_;
-}
 DescriptionIndex TribeDescr::ironore() const {
 	assert(tribes_.ware_exists(ironore_));
 	return ironore_;

=== modified file 'src/logic/map_objects/tribes/tribe_descr.h'
--- src/logic/map_objects/tribes/tribe_descr.h	2019-03-01 04:19:53 +0000
+++ src/logic/map_objects/tribes/tribe_descr.h	2019-05-19 16:18:33 +0000
@@ -116,7 +116,6 @@
 	DescriptionIndex soldier() const;
 	DescriptionIndex ship() const;
 	DescriptionIndex port() const;
-	DescriptionIndex barracks() const;
 	DescriptionIndex ironore() const;
 	DescriptionIndex rawlog() const;
 	DescriptionIndex refinedlog() const;
@@ -206,7 +205,6 @@
 	DescriptionIndex soldier_;     // The soldier that this tribe uses
 	DescriptionIndex ship_;        // The ship that this tribe uses
 	DescriptionIndex port_;        // The port that this tribe uses
-	DescriptionIndex barracks_;    // The barracks to create soldiers
 	DescriptionIndex ironore_;     // Iron ore
 	DescriptionIndex rawlog_;      // Simple log
 	DescriptionIndex refinedlog_;  // Refined log, e.g. wood or blackwood

=== modified file 'src/logic/map_objects/tribes/tribes.cc'
--- src/logic/map_objects/tribes/tribes.cc	2019-05-16 09:15:03 +0000
+++ src/logic/map_objects/tribes/tribes.cc	2019-05-19 16:18:33 +0000
@@ -357,7 +357,7 @@
 
 		// Verify that the preciousness has been set for all of the tribe's wares
 		for (const DescriptionIndex wi : tribe_descr->wares()) {
-			if (tribe_descr->get_ware_descr(wi)->preciousness(tribe_descr->name()) == kInvalidWare) {
+			if (tribe_descr->get_ware_descr(wi)->ai_hints().preciousness(tribe_descr->name()) == kInvalidWare) {
 				throw GameDataError("The ware '%s' needs to define a preciousness for tribe '%s'",
 				                    tribe_descr->get_ware_descr(wi)->name().c_str(),
 				                    tribe_descr->name().c_str());

=== modified file 'src/logic/map_objects/tribes/ware_descr.cc'
--- src/logic/map_objects/tribes/ware_descr.cc	2019-02-23 11:00:49 +0000
+++ src/logic/map_objects/tribes/ware_descr.cc	2019-05-19 16:18:33 +0000
@@ -35,7 +35,8 @@
  * /data/tribes/wares/armor/init.lua
  */
 WareDescr::WareDescr(const std::string& init_descname, const LuaTable& table)
-   : MapObjectDescr(MapObjectType::WARE, table.get_string("name"), init_descname, table) {
+   : MapObjectDescr(MapObjectType::WARE, table.get_string("name"), init_descname, table),
+	 ai_hints_(new WareHints(*table.get_table("preciousness"))) {
 	if (helptext_script().empty()) {
 		throw GameDataError("Ware %s has no helptext script", name().c_str());
 	}
@@ -51,19 +52,9 @@
 	for (const std::string& key : items_table->keys<std::string>()) {
 		default_target_quantities_.emplace(key, items_table->get_int(key));
 	}
-
-	items_table = table.get_table("preciousness");
-	for (const std::string& key : items_table->keys<std::string>()) {
-		preciousnesses_.emplace(key, items_table->get_int(key));
-	}
-}
-
-int WareDescr::preciousness(const std::string& tribename) const {
-	if (preciousnesses_.count(tribename) > 0) {
-		return preciousnesses_.at(tribename);
-	}
-	return kInvalidWare;
-}
+}
+
+
 
 DescriptionIndex WareDescr::default_target_quantity(const std::string& tribename) const {
 	if (default_target_quantities_.count(tribename) > 0) {

=== modified file 'src/logic/map_objects/tribes/ware_descr.h'
--- src/logic/map_objects/tribes/ware_descr.h	2019-02-27 19:00:36 +0000
+++ src/logic/map_objects/tribes/ware_descr.h	2019-05-19 16:18:33 +0000
@@ -21,12 +21,14 @@
 #define WL_LOGIC_MAP_OBJECTS_TRIBES_WARE_DESCR_H
 
 #include <cstring>
+#include <memory>
 #include <string>
 #include <unordered_map>
 
 #include <stdint.h>
 
 #include "base/macros.h"
+#include "ai/ai_hints.h"
 #include "logic/map_objects/map_object.h"
 #include "scripting/lua_table.h"
 
@@ -52,9 +54,10 @@
 	~WareDescr() override {
 	}
 
-	/// Returns the preciousness of the ware, or kInvalidWare if the tribe doesn't use the ware.
-	/// It is used by the computer player.
-	int preciousness(const std::string& tribename) const;
+	/// AI hints for this ware type
+	const WareHints& ai_hints() const {
+		return *ai_hints_;
+	}
 
 	/// How much of the ware type an economy should store in warehouses.
 	/// The special value kInvalidWare means that the target quantity of this ware type will never be
@@ -82,8 +85,9 @@
 private:
 	// tribename, quantity. No default.
 	std::unordered_map<std::string, int> default_target_quantities_;
-	// tribename, preciousness. No default.
-	std::unordered_map<std::string, int> preciousnesses_;
+
+	// Hints for the AI
+	std::unique_ptr<WareHints> ai_hints_;
 
 	std::set<DescriptionIndex> consumers_;  // Buildings that consume this ware
 	std::set<DescriptionIndex> producers_;  // Buildings that produce this ware

=== modified file 'src/logic/map_objects/tribes/worker_descr.cc'
--- src/logic/map_objects/tribes/worker_descr.cc	2019-05-05 14:05:07 +0000
+++ src/logic/map_objects/tribes/worker_descr.cc	2019-05-19 16:18:33 +0000
@@ -52,6 +52,7 @@
      becomes_(table.has_key("experience") ? tribes.safe_worker_index(table.get_string("becomes")) :
                                             INVALID_INDEX),
      needed_experience_(table.has_key("becomes") ? table.get_int("experience") : INVALID_INDEX),
+	 ai_hints_(table.has_key("aihints") ? new WorkerHints(*table.get_table("aihints")) : nullptr),
      tribes_(tribes) {
 	if (helptext_script().empty()) {
 		throw GameDataError("Worker %s has no helptext script", name().c_str());

=== modified file 'src/logic/map_objects/tribes/worker_descr.h'
--- src/logic/map_objects/tribes/worker_descr.h	2019-05-11 13:48:12 +0000
+++ src/logic/map_objects/tribes/worker_descr.h	2019-05-19 16:18:33 +0000
@@ -22,6 +22,7 @@
 
 #include <memory>
 
+#include "ai/ai_hints.h"
 #include "base/macros.h"
 #include "graphic/diranimations.h"
 #include "logic/map_objects/bob.h"
@@ -116,6 +117,11 @@
 		return programs_;
 	}
 
+	/// AI hints for this worker type. Can be nullptr.
+	const WorkerHints* ai_hints() const {
+		return ai_hints_.get();
+	}
+
 protected:
 	Programs programs_;
 
@@ -145,6 +151,9 @@
 	std::set<DescriptionIndex> employers_;
 
 private:
+	// Hints for the AI
+	std::unique_ptr<WorkerHints> ai_hints_;
+
 	const Tribes& tribes_;
 	DISALLOW_COPY_AND_ASSIGN(WorkerDescr);
 };