petra AI queueplanBuilding error when starting with entities of a different civ

[elexis]

In this fortress replay (but not every fortress replay) with r20511 I get the following error spammed:

ERROR: JavaScript error: simulation/ai/petra/queueplanBuilding.js line 91
TypeError: builder is undefined
  m.ConstructionPlan.prototype.start@simulation/ai/petra/queueplanBuilding.js:91:3
  m.Queue.prototype.startNext@simulation/ai/petra/queue.js:60:3
  m.QueueManager.prototype.startNextItems@simulation/ai/petra/queueManager.js:374:6
  m.QueueManager.prototype.update@simulation/ai/petra/queueManager.js:407:2
  m.PetraBot.prototype.OnUpdate@simulation/ai/petra/_petrabot.js:121:3
  m.BaseAI.prototype.HandleMessage@simulation/ai/common-api/baseAI.js:64:2

[elexis]

Hold on, actually not triggered with clean svn.

[elexis]

It was an off by 1 in my upcoming rmgen diff. It placed starting units and the starting walls for all civs on that Fortress map for a wrong owner.

The error can be reproduced on any map by spawning a unit of a different civ than the player civ. For instance using this:

Index: binaries/data/mods/public/maps/random/rmgen/player.js
===================================================================
--- binaries/data/mods/public/maps/random/rmgen/player.js	(revision 20512)
+++ binaries/data/mods/public/maps/random/rmgen/player.js	(working copy)
@@ -5,11 +5,11 @@
 /**
  * Gets the default starting entities for the civ of the given player, as defined by the civ file.
  */
 function getStartingEntities(playerID)
 {
-	return g_CivData[getCivCode(playerID)].StartEntities;
+	return g_CivData.brit.StartEntities;
 }
 
 /**
  * Places the given entities at the given location (typically a civic center and starting units).
  * @param civEntities - An array of objects with the Template property and optionally a Count property.

So it's not urgent at all but only a nice-to-have for weird maps or mods.

Since petra can already deal with owning things of other civs, it could be able to handle starting units of different civs too (unless there is some catch).

[mimo]

Rather a simulation problem than an ai one: we should decide what is allowed to be built in that case (specially if we ever implement some unit capture). As i don't think that when capturing a unit we should have access to all structures of the captured unit but rather conversion of the captured unit to its new culture, we must change the replacement in Builder.js from the Identity civ to the player civ (as is done in ProductionQueue). If going that way, here is a patch with the needed changes in the ai (i've put the changes in rmgen from elexis to allow easy test of the patch). I'll add it to phabricator when the Builder.js change is agreed.

[elexis]

I'd love to hear sanderd17 and fatherbushidos opinion on the diff, but I guess that won't happen.

Other than that the diff should be in accordance with our current design idea that units can only build buildings of the civ they own, not buildings of the civs of the units the player owns.

Building buildings of other civs was only useful in cheat games for testing purposes with the "salad bowl" cheat. Perhaps there could be some mod that wants to allow this however (Delenda Est?).

We could make that design decision a template boolean in Builder of the three template_unit_* files: OnlyPlayerCiv or AllowUnitCivBuildings.

(Linting: Only one comment line in queueplanBuilding.js needs change, doesn't really matter though)

[mimo]

OK see ​phab:D1065

Replying to elexis:

Perhaps there could be some mod that wants to allow this however (Delenda Est?).

We could make that design decision a template boolean in Builder of the three template_unit_* files: OnlyPlayerCiv or AllowUnitCivBuildings.

AI is not yet able to cope with such changes: nothing difficult to do a priori, but it would need somebody to do it

Edit: And such a behaviour in mods can be obtained by replacing the {civ} directly in the unit template.

[mimo]

Fixed by r20521

[elexis]

In 20527:

Fix Builder test following rP20521, refs #4870.
Also fix confusion of playerID and playerEntityID in that test following rP19488.

Differential Revision: ​https://code.wildfiregames.com/D1067
Reviewed By: bb, Itms
Comments By: Stan