Merge lp:~s3734770/widelands/carli-ai into lp:widelands

[Carli]

Hi,

I improved the AI a bit and fixed some bugs.
The AI now takes in account that it needs material for building and for creating soldiers.
The barbarians axefactory upgrade works now.

What I further will do:
 - Treat upgradable buildings like normal buildings and create construction time plans
 - find an automatic balance for the consumer-producer problem

[SirVer (sirver)]

Cool that you work on that. However your changes also contain changes to compile.sh; and also I saw that you replaced bulldoze via dismantle. This let's the flag standing and let to problems in the past. Have you seen this as well?

review: Needs Fixing

[Carli (s3734770)]

Shouldnt the road optimizer detect the flag problems? I did not notice any problems with that.

About the compile.sh.... the make parameters should not be versioned.

So what should I fix? The make parameter....
But not the dismantle. It works for me. Which version of widelands do you have?

[SirVer (sirver)]

> Shouldnt the road optimizer detect the flag problems? I did not notice any
> problems with that.
It should, but notice the comment in 256. I guess it is there for a reason.

> About the compile.sh.... the make parameters should not be versioned.
Yep, it shouldn't. You have to be more careful what you commit. You can unmerge your changes or rely on me reverting it when this will be merged.

> But not the dismantle. It works for me. Which version of widelands do you
> have?
Current trunk obviously :). I am really reluctant to merge this for it is not clear what you did. Your branch contains a lot of lines where only whitespace changes happened (something which you should avoid. Use bzr diff or qdiff or similar to check that you do not do that). Some stuff is juggled around and in some places you changed one magic constant into another. What makes you believe that your AI is indeed an improvement?

[Carli (s3734770)]

I checked the source files with bzr diff, but i did not want to revert it because i _want_ to build with -j. (ok but i could have only committed the ai related files)

How should I comment the constant changes? In the commit message? I think a person who reads the source is not interested in the history of the constants.

[SirVer (sirver)]

At the very least you should comment in the source code. And you should fix up whitespace issues and repropose for merging.

I am sorry if I appear to be strict. I am sure your work makes widelands better. Nevertheless, we need to keep some standards int the quality of the code.

review: Needs Fixing

[Nicolai Hähnle (nha)]

Slightly off-topic for the discussion of this particular merge request, but this just highlights our need for some framework to properly judge the competitiveness of the AI, e.g. by letting different versions of it automatically play against each other.

[Carli (s3734770)]

This is also off-topic, but should i design the AI more learnable?

[SirVer (sirver)]

learnable is quite vague. Afaik most AI in games no longer use neural networks or similar because heuristics just perform so much better. Nevertheless, there is a bookshelve of literature on "good" game AI, some of them employ something like bayesian parameter tuning for "learning". That could be interesting of course.

[Carli (s3734770)]

With "learnable" i ment parameter tuning. My question was related to the fact that I changed some constants in the AI code. They shouldnt be constants. (Or be defined in a extra header)

[Nicolai Hähnle (nha)]

Yes, putting those parameters into a separate header or even a loadable file seems like a good idea. Even more so if it goes along with at least basic documentation of what the numbers actually mean.

[SirVer (sirver)]

Is this pull request still up for discussion? I.e. are you still planing to work on it carli?

[Carli (s3734770)]

> Is this pull request still up for discussion? I.e. are you still planing to
> work on it carli?

Currently not.
If someone would fix the C++ things with const vs & and such, it could be merged but otherwise I don't care.

[SirVer (sirver)]

Thanks for the feedback. Okay, I am setting this to rejected than to get it out of the review queue. It is unlikely that someone wants to clean up other peoples code :).

Unmerged revisions

6100. By Carli on 2011-11-13

Improved AI (is now able to defeat other AI enemies)

=== modified file 'compile.sh'
--- compile.sh	2010-11-15 21:23:02 +0000
+++ compile.sh	2011-11-13 00:10:30 +0000
@@ -136,7 +136,7 @@
 
     echo " "
     cmake -DWL_PORTABLE=true .. -DCMAKE_EXE_CXX_FLAGS="${CFLAGS}" -DCMAKE_BUILD_TYPE="${var_build_type}"
-    make ${MAKEOPTS}
+    make ${MAKEOPTS} -j
     return 0
   }
 
@@ -182,7 +182,7 @@
             echo " "
             echo "bzr pull"
             echo "cd build"
-            echo "make"
+            echo "make -j"
             if [ $var_build_lang -eq 1 ] ; then
               echo "make lang"
             fi

=== modified file 'src/ai/ai_help_structs.h'
--- src/ai/ai_help_structs.h	2010-11-01 22:23:29 +0000
+++ src/ai/ai_help_structs.h	2011-11-13 00:10:30 +0000
@@ -268,6 +268,8 @@
 	uint8_t producers;
 	uint8_t consumers;
 	uint8_t preciousness;
+	uint32_t build_prio;
+	uint32_t fight_prio;
 };
 
 #endif

=== modified file 'src/ai/defaultai.cc'
--- src/ai/defaultai.cc	2011-10-12 21:05:21 +0000
+++ src/ai/defaultai.cc	2011-11-13 00:10:30 +0000
@@ -232,6 +232,30 @@
 		wares.at(i).producers    = 0;
 		wares.at(i).consumers    = 0;
 		wares.at(i).preciousness = tribe->get_ware_descr(i)->preciousness();
+		wares.at(i).build_prio   = 0;
+		wares.at(i).fight_prio   = 0;
+	}
+
+	Ware_Index const nr_workers = tribe->get_nrworkers();
+	for (Ware_Index i = Ware_Index::First(); i < nr_workers; ++i) {
+		// Find all build costs for soldiers
+		// is it a soldier?
+		if (tribe->get_worker_descr(i)->get_worker_type() == Worker_Descr::SOLDIER) {
+			Worker_Descr::Buildcost::const_iterator end = tribe->get_worker_descr(i)->buildcost().end();
+			for
+			  (Worker_Descr::Buildcost::const_iterator it = tribe->get_worker_descr(i)->buildcost().begin();
+			  it != end; ++it)
+			{
+				if (tribe->ware_index(it->first))
+					wares.at(tribe->ware_index(it->first)).fight_prio += it->second; // Add soldier-need priority
+			}
+		}
+	}
+
+	for (Ware_Index i = Ware_Index::First(); i < nr_wares; ++i) {
+		// We have three building sites
+		wares.at(i).consumers    += wares.at(i).build_prio > 0 ?
+			(wares.at(i).build_prio > 30 ? 2 : 1) : 0;
 	}
 
 	// collect information about the different buildings our tribe can construct
@@ -255,6 +279,12 @@
 		bo.current_stats          = 100;
 		bo.unoccupied             = false;
 
+		// Find all build costs
+		Buildcost::const_iterator end = bld.buildcost().end();
+		for (Buildcost::const_iterator it = bld.buildcost().begin(); it != end; ++it) {
+			wares.at(it->first).build_prio += it->second; // Add build-need priority
+		}
+
 		bo.is_basic               = false;
 
 		bo.is_buildable           = bld.is_buildable();
@@ -278,7 +308,8 @@
 				BuildingObserver::PRODUCTIONSITE;
 
 			container_iterate_const(Ware_Types, prod.inputs(), j)
-				bo.inputs.push_back(j.current->first.value());
+				for (uint32_t i = 0; i < j.current->second; i++) // consider input count
+					bo.inputs.push_back(j.current->first.value());
 
 			container_iterate_const
 				(ProductionSite_Descr::Output, prod.output_ware_types(), j)
@@ -318,6 +349,17 @@
 		}
 	}
 
+	for (Ware_Index i = Ware_Index::First(); i < nr_wares; ++i) {
+		if (wares.at(i).build_prio)
+			printf
+			  ("Ware %s ist construction material with priority %d (ToDo: use this information)\n",
+			  tribe->get_ware_descr(i)->name().data(), wares.at(i).build_prio);
+		if (wares.at(i).fight_prio)
+			printf
+			  ("Ware %s ist fight material with priority %d (ToDo: use this information)\n",
+			  tribe->get_ware_descr(i)->name().data(), wares.at(i).fight_prio);
+	}
+
 	total_constructionsites       =    0;
 	next_construction_due         =    0;
 	next_road_due                 = 1000;
@@ -767,7 +809,7 @@
 		spots += spots_avail.at(BUILDCAPS_MEDIUM);
 		spots += spots_avail.at(BUILDCAPS_BIG);
 		if (type == AGGRESSIVE)
-			spots -= militarysites.size() / 20;
+			spots -= militarysites.size() / 30;
 		if (spots < 16)
 			expand_factor *= 2;
 		if ((type == AGGRESSIVE) && spots < 32)
@@ -791,7 +833,7 @@
 	//if (TODO) expand_factor = 0;
 
 	// Defensive AIs also attack sometimes (when they want to expand)
-	if (type == DEFENSIVE && expand_factor > 1)
+	if (expand_factor > 1)
 		if (next_attack_consideration_due <= game().get_gametime())
 			consider_attack(game().get_gametime());
 
@@ -863,8 +905,8 @@
 					continue;
 				if (bo.need_trees) {
 					// Priority of woodcutters depend on the number of near trees
-					prio += bf->trees_nearby * 3;
-					prio /= 3 * (1 + bf->producers_nearby.at(bo.outputs.at(0)));
+					prio += bf->trees_nearby * 4;
+					prio /= 2 * (1 + bf->producers_nearby.at(bo.outputs.at(0)));
 
 					// TODO improve this - it's still useless to place lumberjack huts randomly
 					/*if (prio <= 0) // no, sometimes we need wood without having a forest
@@ -898,6 +940,7 @@
 					// production hint (f.e. associate forester with trunks)
 
 					// Calculate the need for this building
+
 					int16_t inout = wares.at(bo.production_hint).consumers;
 					if
 						(tribe->safe_ware_index("trunk").value()
@@ -907,6 +950,8 @@
 					inout -= wares.at(bo.production_hint).producers;
 					if (inout < 1)
 						inout = 1;
+					if (inout > 3)
+						inout = 2;
 					// the ware they're refreshing
 					Ware_Index wt(static_cast<size_t>(bo.production_hint));
 					container_iterate(std::list<EconomyObserver *>, economies, l) {
@@ -948,12 +993,23 @@
 					prio = recalc_with_border_range(*bf, prio);
 				} else { // "normal" productionsites
 
-					// ToDo: prefer soldier producing things
 					// Ware_Index const soldier_index = tribe().worker_index("soldier");
 
 					if (bo.is_basic && (bo.total_count() == 0))
 						prio += 100; // for very important buildings
 
+
+
+					// we can enhance this building. build more
+					// maybe the enhancement can produce needed ware
+					if (bo.desc->enhancements().size() > 0) {
+						// this code builds more metalworks
+						if (bo.total_count() == 0)
+							prio += 10;
+						if (bo.total_count() == 1)
+							prio += 2;
+					}
+
 					// Check if the produced wares are needed
 					container_iterate(std::list<EconomyObserver *>, economies, l) {
 						// Don't check if the economy has no warehouse.
@@ -968,6 +1024,11 @@
 								 (*l.current)->economy.ware_target_quantity(wt).permanent)
 								prio -= 20;
 
+							// if we have none of them, but they are construction material, build!
+							if (bo.total_count() == 0 && wares.at(bo.outputs.at(m)).build_prio) {
+								prio += 100;
+							}
+
 							// if the economy needs this ware
 							if ((*l.current)->economy.needs_ware(wt)) {
 								prio += 1 + wares.at(bo.outputs.at(m)).preciousness;
@@ -975,16 +1036,6 @@
 									// big bonus, this site might be elemental
 									prio += 3 * wares.at(bo.outputs.at(m)).preciousness;
 							}
-
-							// we can enhance this building. build more
-							// maybe the enhancement can produce needed ware
-							if (bo.desc->enhancements().size() > 0) {
-								// this code builds more metalworks
-								if (bo.total_count() == 0)
-									prio += 2;
-								if (bo.total_count() == 1)
-									prio += 8;
-							}
 						}
 						for (uint32_t m = 0; m < bo.inputs.size(); ++m) {
 							Ware_Index wt(static_cast<size_t>(bo.inputs.at(m)));
@@ -1018,14 +1069,14 @@
 
 					// do not construct more than one building,
 					// if supply line is already broken.
-					if (!check_supply(bo) && bo.total_count() > 0)
+					if (bo.total_count() > 0 && !check_supply(bo))
 						prio -= 12;
 
 				}
 			} else if (bo.type == BuildingObserver::MILITARYSITE) {
 				if (!bf->unowned_land_nearby)
 					continue;
-				prio  = bf->unowned_land_nearby * (1 + type);
+				prio  = bf->unowned_land_nearby * (2 + type);
 				prio -= bf->military_influence * (5 - type);
 				// set to at least 1
 				prio  = prio > 0 ? prio : 1;
@@ -1033,14 +1084,14 @@
 				prio /= 2;
 
 				if (bf->enemy_nearby)
-					prio *= 2;
+					prio = (prio * 5) / 4;
 				else
 					prio -= bf->military_influence * 2;
 
 				if (bf->avoid_military)
 					prio /= 5;
 
-				prio -= militarysites.size() - productionsites.size() / (3 - type);
+				prio -= militarysites.size() - productionsites.size() / (4 - type);
 
 			} else if (bo.type == BuildingObserver::WAREHOUSE) {
 				//  Build one warehouse for ~every 35 productionsites and mines.
@@ -1209,7 +1260,7 @@
 			// multiply with current statistics of all other buildings of this
 			// type to avoid constructing buildings where already some are running
 			// on low resources.
-			prio *= 5 + bo.current_stats;
+			prio *= 40 + bo.current_stats;
 			prio /= 100;
 
 			if (onlymissing) // mines aren't *that* important
@@ -1613,7 +1664,7 @@
 			// destruct the building and it's flag (via flag destruction)
 			// the destruction of the flag avoids that defaultAI will have too many
 			// unused roads - if needed the road will be rebuild directly.
-			game().send_player_bulldoze(site.site->base_flag());
+			game().send_player_dismantle(*(site.site));
 			return true;
 		}
 	}
@@ -1632,7 +1683,7 @@
 		// destruct the building and it's flag (via flag destruction)
 		// the destruction of the flag avoids that defaultAI will have too many
 		// unused roads - if needed the road will be rebuild directly.
-		game().send_player_bulldoze(site.site->base_flag());
+		game().send_player_dismantle(*(site.site));
 		return true;
 	}
 
@@ -1662,7 +1713,7 @@
 				//
 				// Add a bonus if one building of this type is still unoccupied
 				if (((game().get_gametime() % 4) + site.bo->unoccupied) > 2) {
-					game().send_player_bulldoze(site.site->base_flag());
+					game().send_player_dismantle(*(site.site));
 					return true;
 				}
 				else
@@ -1675,7 +1726,7 @@
 
 	// Do not have too many constructionsites
 	uint32_t producers = mines.size() + productionsites.size();
-	if (total_constructionsites >= (5 + (producers / 10)))
+	if (total_constructionsites >= (5 + (producers / 5)))
 		return false;
 
 	// Check whether building is enhanceable and if wares of the enhanced
@@ -1709,7 +1760,7 @@
 						Ware_Index wt(static_cast<size_t>(current_outputs.at(j)));
 						if (site.site->economy().needs_ware(wt))
 							prio -=
-								(2 + wares.at(current_outputs.at(j)).preciousness) / 2;
+								(2 + wares.at(current_outputs.at(j)).preciousness) / 4;
 						continue;
 					}
 				new_outputs.push_back(static_cast<int16_t>(i));
@@ -1718,28 +1769,30 @@
 			// Check if the new wares are needed in economy of the building
 			for (uint32_t i = 0; i < new_outputs.size(); ++i) {
 				Ware_Index wt(static_cast<size_t>(new_outputs.at(i)));
-				if (site.site->economy().needs_ware(wt))
+				if (site.site->economy().needs_ware(wt) || wares.at(wt).fight_prio)
 					prio += 2 + wares.at(new_outputs.at(i)).preciousness;
 			}
 
 			// Compare the number of buildings of current type with the number
 			// of buildings of enhanced type
-			prio += (site.bo->total_count() - en_bo.total_count()) * 2;
+			prio += (2 * site.bo->total_count() - en_bo.total_count());
 
 			// If the new wares are needed
 			if (prio > 0) {
 				prio = calculate_need_for_ps(en_bo, prio);
 				if (prio > maxprio) {
+					// we always enhance if it is the first building of this type
+					if (en_bo.total_count() == 0) prio += 500;
 					maxprio = prio;
 					enbld = (*x.current);
 				}
-			}
+			};
 		}
 	}
 
 	// Enhance if enhanced building is useful
 	// additional: we dont want to lose the old building
-	if (maxprio > 0 && site.bo->total_count() > 1) {
+	if (maxprio > 0 && (maxprio > 500 || site.bo->total_count() > 1)) {
 		game().send_player_enhance_building(*site.site, enbld);
 		changed = true;
 	}
@@ -1778,7 +1831,7 @@
 		// destruct the building and it's flag (via flag destruction)
 		// the destruction of the flag avoids that defaultAI will have too many
 		// unused roads - if needed the road will be rebuild directly.
-		game().send_player_bulldoze(site.site->base_flag());
+		game().send_player_dismantle(*(site.site));
 		return true;
 	}
 
@@ -1880,7 +1933,7 @@
 							(static_cast<int32_t>(ms->maxSoldierCapacity() * 4)
 							 <
 							 bf.military_influence)
-							game().send_player_bulldoze(ms->base_flag());
+							game().send_player_dismantle(*ms);
 
 						// Else consider enhancing the building (if possible)
 						else {
@@ -1995,7 +2048,7 @@
 		WareObserver & wo = wares.at(bo.outputs.at(k));
 		if (wo.consumers > 0) {
 			output_prio += wo.preciousness;
-			output_prio += wo.consumers * 2;
+			output_prio += wo.consumers * 3;
 			output_prio -= wo.producers * 2;
 			if (bo.total_count() == 0)
 				output_prio += 10; // add a big bonus
@@ -2006,12 +2059,18 @@
 			(ceil(output_prio / sqrt(static_cast<double>(bo.outputs.size()))));
 	prio += 2 * output_prio;
 
+	if (!bo.inputs.empty() && !bo.outputs.empty() && bo.current_stats >= 80) {
+		// extra bonus for high productivity
+		// this should compensate various output count of a production step
+		prio += 10;
+	}
+
 	// If building consumes some wares, multiply with current statistics of all
 	// other buildings of this type to avoid constructing buildings where already
 	// some are running on low resources.
 	// Else at least add a part of the stats t the calculation.
 	if (!bo.inputs.empty()) {
-		prio *= bo.current_stats;
+		prio = (prio + 50) * bo.current_stats;
 		prio /= 100;
 	} else
 		prio = ((prio * bo.current_stats) / 100) + (prio / 2);
@@ -2142,8 +2201,14 @@
 			militarysites.back().site = &ref_cast<MilitarySite, Building>(b);
 			militarysites.back().bo = &bo;
 			militarysites.back().checks = bo.desc->get_size();
-		} else if (bo.type == BuildingObserver::WAREHOUSE)
+		} else if (bo.type == BuildingObserver::WAREHOUSE) {
 			++numof_warehouses;
+			Ware_Index const nr_wares = tribe->get_nrwares();
+			for (Ware_Index i = Ware_Index::First(); i < nr_wares; ++i) {
+				// Warehouses are consumers of weapons
+				wares.at(i).consumers    += 2 * wares.at(i).fight_prio;
+			}
+		}
 	}
 }
 
@@ -2207,6 +2272,11 @@
 		} else if (bo.type == BuildingObserver::WAREHOUSE) {
 			assert(numof_warehouses > 0);
 			--numof_warehouses;
+			Ware_Index const nr_wares = tribe->get_nrwares();
+			for (Ware_Index i = Ware_Index::First(); i < nr_wares; ++i) {
+				// Warehouses are consumers of weapons
+				wares.at(i).consumers    -= 2 * wares.at(i).fight_prio;
+			}
 		}
 	}
 	m_buildable_changed = true;
@@ -2220,23 +2290,23 @@
 // TODO: It needs profiling and optimization.
 bool DefaultAI::check_supply(BuildingObserver const & bo)
 {
-	size_t supplied = 0;
 	container_iterate_const(std::vector<int16_t>, bo.inputs, i)
 		container_iterate_const(std::vector<BuildingObserver>, buildings, j)
 			if
 				(j.current->cnt_built &&
-				 std::find
-				 	(j.current->outputs.begin(), j.current->outputs.end(),
-				 	 *i.current)
-				 !=
-				 j.current->outputs.end()
-				 &&
-				 check_supply(*j.current))
+				std::find
+					(j.current->outputs.begin(), j.current->outputs.end(),
+					 *i.current)
+				!=
+				j.current->outputs.end()
+				&&
+				check_supply(*j.current))
 			{
-				++supplied;
 				break;
+			} else {
+				return false;
 			}
-	return supplied == bo.inputs.size();
+	return true;
 }
 
 
@@ -2277,12 +2347,12 @@
 				continue;
 			if (bld->canAttack()) {
 				int32_t ta = player->findAttackSoldiers(bld->base_flag());
-				if (type == NORMAL)
-					ta = ta * 2 / 3;
+				if (type != AGGRESSIVE)
+					ta = (ta * 2) / 3;
 				if (ta < 1)
 					continue;
 
-				int32_t const tc = ta - bld->presentSoldiers().size();
+				int32_t const tc = ta * 2 - bld->presentSoldiers().size() * 3;
 				if (tc > chance) {
 					target = bld;
 					chance = tc;
@@ -2313,7 +2383,7 @@
 
 	// Return if chance to win is too low
 	if (chance < 3) {
-		next_attack_consideration_due = gametime % 7 * 1000 + gametime;
+		next_attack_consideration_due = (2 + gametime % 7) * 1000 + gametime;
 		return false;
 	}
 
@@ -2326,6 +2396,6 @@
 		(target->base_flag(), pn, attackers, retreat);
 
 	//  Do not attack again too soon - returning soldiers must get healed first.
-	next_attack_consideration_due = (gametime % 51 + 10) * 1000 + gametime;
+	next_attack_consideration_due = (gametime % 51 + 20) * 1000 + gametime;
 	return true;
 }

=== modified file 'src/ui_basic/table.cc'
--- src/ui_basic/table.cc	2011-11-06 14:23:36 +0000
+++ src/ui_basic/table.cc	2011-11-13 00:10:30 +0000
@@ -429,6 +429,7 @@
 	return result;
 }
 
+
 /**
  * Scroll to the given position, in pixels.
 */