Why is this function working haphazardly?

[Craig_Sutter]

I've been trying to get a building built via lua if an improved resource is in the city hexes. The function is working, but only sometimes. In testing, some civilizations are getting buildings as expected, however, my test civ only rarely gets a building when I fulfill the criteria (having a resource of the appropriate type improved in city owned hexes.

I think it is something I've done wrong with looping through the player or cities such that the function is stopping before going through all of the players. I could set up a separate function for each resource, but I feel that is not the most efficient way to write the code.

What am I missing?

Spoiler :

-- for player civ , loop through the players
-- for loop through cities to see eligible for Merchant House via improved resource and place if eligible

function MerchantHouseSpawn()

-- find eligible cities

	for PlayerID=0, GameDefines.MAX_CIV_PLAYERS - 1 do  

	local pPlayer = Players[PlayerID];

		Player = pPlayer

		if Player:IsEverAlive() then 

			-- Enumerate cities
			
			for pCity in Player:Cities() do
							
				-- City exists and and has improved resource... build Mechant House if eligible

				MerchantHouse = pCity:GetNumRealBuilding(GameInfoTypes["BUILDING_MERCHANT_HOUSE"])	

				if	pCity ~= nil and MerchantHouse == 0 then
					
					if	pCity:IsHasResourceLocal(GameInfoTypes["RESOURCE_COTTON"]) or 
						pCity:IsHasResourceLocal(GameInfoTypes["RESOURCE_IRON"]) or 
						pCity:IsHasResourceLocal(GameInfoTypes["RESOURCE_INCENSE"]) or 
						pCity:IsHasResourceLocal(GameInfoTypes["RESOURCE_COPPER"]) then
				
						pCity:SetNumRealBuilding(GameInfoTypes["BUILDING_MERCHANT_HOUSE"], 1);
						
						print(Player:GetName(), "...is spawning Merchant House in...", pCity:GetName())

					end
				end										
			end
		end
	end
end



--to set changes

local g_lastTurn = -1

local function OnPlayerDoTurn(iPlayer)

	local gameTurn = Game.GetGameTurn()

	if g_lastTurn < gameTurn then
		g_lastTurn = gameTurn

		--per game turn function here
		
		MerchantHouseSpawn()

	end
end
GameEvents.PlayerDoTurn.Add(OnPlayerDoTurn)

 

[Nutty]

It doesn't appear to be a problem as far as the logical stuff (looping through players and cities, and checking for all resources in one loop). If you really think that's the problem, you can always put in another debug print for the players and cities.

So that means it's a problem with the game functions you're using not giving you the result you expected. Are you sure the city has met the prerequisites for the building?

Do you mean to be looping through the city states as well as the major civs? [And why do you assign pPlayer only to turn around and immediately assign that result to Player? Why bother with a MerchantHouse variable?]

 

[Nutty]

Oh, hey... does

for pCity in Player:Cities() do

return the city ID to the pCity variable instead of the city object?

In other words, do you want it to be:

for CityID in Player:Cities() do
    pCity = Player:GetCityByID(CityID)

Note I'm not sure that's the case. [It is the case.] [Or not.] I think that might cause haphazard behavior, if the city in question is the player's capital.

 

[bc1]

Your code seems to include a lot of unnecessary logic

This might work:

GameEvents.PlayerDoTurn.Add(
function( playerID )
	local player = Players[ playerID ]
	if player and player:IsAlive() then -- this check is probably overkill
		for city in player:Cities() do
			if city:GetNumRealBuilding( GameInfoTypes.BUILDING_MERCHANT_HOUSE ) < 1 then
				if city:IsHasResourceLocal( GameInfoTypes.RESOURCE_COTTON )
				or city:IsHasResourceLocal( GameInfoTypes.RESOURCE_IRON )
				or city:IsHasResourceLocal( GameInfoTypes.RESOURCE_INCENSE )
				or city:IsHasResourceLocal( GameInfoTypes.RESOURCE_COPPER )
				then
					city:SetNumRealBuilding( GameInfoTypes.BUILDING_MERCHANT_HOUSE, 1 )
				end
			end
		end
	end
end )

Cheers

 

[Craig_Sutter]

I'll try that code.

I have to admit, I overdid the "logic". I started out a bit simpler, but the code gradually changed as I did different iterations.

I'll try the code bc1 set up... if that fails, I'll redo it with setting pCity based on city ID and see what happens.

Odd thing is, I've used this code before at various other times and have never had this problem before. However, I have never done so while looping through all the players... so this time it is giving somewhat different results than usual.

I'll repost after I've given it a run or two.

Thank-you.

 

[Nutty]

No, I think my city ID theory was wrong...

 

[Craig_Sutter]

Yes... I tried it and got nil value...

Still having trouble with this one... bc1's function is having the same difficulties as my first one... I think that the function is picking up the first city that fulfils the criteria and skipping the rest for each player... it is finding a merchant house andstopping... not going on to the next city.

Maybe.

I guess more print statements are in order.

Thanks.

 

[Craig_Sutter]

This is my latest try. Still getting haphazard results. Probably overdoing the logic as before. This seems to be working for only one city per player (maybe per resource as well) as far as I can tell, but in the last test, did not work for my player civ. There must be something about the logic I am using... it seems to run through the logic until the function completes one time for a civ (or resource), and then it stops. I don't think it treats players differently than non-player civs. I think it has to do with who improves a resource first, then the function runs and stops thereafter. I've yet to see any civ improve a resource in two cities in my tests. Sometimes, I succeed in getting the function to run in one of my cities, but never 2 as well, and not always consistently. Sometimes, the function does not run for any of my cities despite having fulfilled the criteria {i.e. having a resource in workable plot for city} and not having a Merchant House. Said cities show up no where on the print statements I have up... they should at least show on the ones I've outlined in red...

The print statements do work for those cities that somehow successfully get through whatever filter is separating success and failure.

-- for player civ , loop through the players
-- for loop through cities to see eligible for Merchant House via improved resource and place if eligible

function MerchantHouse()

local Player

-- Set up player

	for PlayerID=0, GameDefines.MAX_CIV_PLAYERS - 1 do  
	
		local player = Players[ PlayerID ]
	
		if player and player:IsAlive() then -- this check is probably overkill
		
		Player = player

		end
	end

-- Set up city		
		
	for city in Player:Cities() do
			
		if	city:IsHasResourceLocal( GameInfoTypes.RESOURCE_COTTON )
			or city:IsHasResourceLocal( GameInfoTypes.RESOURCE_IRON )
			or city:IsHasResourceLocal( GameInfoTypes.RESOURCE_INCENSE )
			or city:IsHasResourceLocal( GameInfoTypes.RESOURCE_COPPER )
			or city:IsHasResourceLocal( GameInfoTypes.RESOURCE_SALT )
			then

			[COLOR="Red"]print(Player:GetName(), "...has required resource in city area of...", city:GetName())[/COLOR]

			if city:GetNumRealBuilding( GameInfoTypes.BUILDING_MERCHANT_HOUSE ) >=1 then
						
			print(Player:GetName(), "has Merchant house already", city:GetName())

			elseif city:GetNumRealBuilding( GameInfoTypes.BUILDING_MERCHANT_HOUSE ) == 0 then
				
			city:SetNumRealBuilding( GameInfoTypes.BUILDING_MERCHANT_HOUSE, 1 )
			print(Player:GetName(), "...is spawning Merchant House due to resource in...", city:GetName())
				
			end
		end
	end
end 


--to set changes

local g_lastTurn = -1

local function OnPlayerDoTurn(iPlayer)

	local gameTurn = Game.GetGameTurn()

	if g_lastTurn < gameTurn then
		g_lastTurn = gameTurn

		--per game turn function here
		
		MerchantHouse()

	end
end

GameEvents.PlayerDoTurn.Add(OnPlayerDoTurn)

I'll keep trying with further variations on the code... but I seem to be running into a brick wall and I've no idea why.

 

[LastSword]

I would blindly guess that "IsHasResourceLocal" works only for strategic resources.

Btw, above code is wrong, because Player loop is ended before city iteration, which will result in checking only barbarian player cities. Though I would go with bc1 code (checking certain player on its turn instead checking all players at once).

 

[Craig_Sutter]

You might be right about the Strategic resources.

However, my print statements indicate that not just barb players are being called. The buildings are being added to player cities... and the print statements bear this out.

But your strat resource explanation makes a bit of sense and is something I can test for. My previous uses of a similar code worked, but those cases also involved sttategic resources...

Thanks.

 

[bc1]

Try this:

GameEvents.PlayerDoTurn.Add(
function( playerID )
	local player = Players[ playerID ]
	if player and player:IsAlive() then -- this check is probably overkill
		for city in player:Cities() do
			if city and city:GetNumRealBuilding( GameInfoTypes.BUILDING_MERCHANT_HOUSE ) < 1 then
				if city:IsHasResourceLocal( GameInfoTypes.RESOURCE_COTTON, false )
				or city:IsHasResourceLocal( GameInfoTypes.RESOURCE_IRON, false )
				or city:IsHasResourceLocal( GameInfoTypes.RESOURCE_INCENSE, false )
				or city:IsHasResourceLocal( GameInfoTypes.RESOURCE_COPPER, false )
				then
					city:SetNumRealBuilding( GameInfoTypes.BUILDING_MERCHANT_HOUSE, 1 )
				end
			end
		end
	end
end )

Also note that GameEvents.PlayerDoTurn does not fire reliably on 1st turn.

 

[Craig_Sutter]

I think LastSword nailed it.

I removed iron from the resource list, and got no results in my print statements. The functions used, all of ours, are reliable... city:IsHasResourceLocal appears to fire only for Strategic Resources.

I'm going to have to figure out another way to track things... likely, I'll create a number of (cheap) warehouse buildings that will require certain resources to be built, and when they are built, install the Merchant House via lua that searches cities for those warehouse buildings. Perhaps "BUILDING_SMELTER" for metals, "BUILDING_LUMBERYARD" for wood, "BUILDING_RESIN_WAREHOUSE", etc. would be built if certain resources around (similar to stable, for example), and then lua to search through all cities for said buildings and install MERCHANT_HOUSE is any are in a city and there is no MERCHANT_HOUSE yet spawned.

Unless anyone has a way to track nonstrategic resources and the city that is working them.

Thank-you.

I'll check your lua, bc1... I think it may be the same in effect, though.

 

[whoward69]

The second parameter to city:IsHasResourceLocal() is not optional, so if omitted will be a nil value, ie false, so city:IsHasResourceLocal( GameInfoTypes.RESOURCE_COTTON ) and city:IsHasResourceLocal( GameInfoTypes.RESOURCE_COTTON, false ) are the same

city:IsHasResourceLocal( GameInfoTypes.RESOURCE_COTTON, false ) checks if the city is currently working that resource, ie the city has a tile with that resource within its limits, is currently working that tile AND the tile is improved.

city:IsHasResourceLocal( GameInfoTypes.RESOURCE_COTTON, true ) checks if the city could work the resource, ie the city has a tile with that resource within its limits and the resource is currently revealed to the team.

Without knowing what the second parameter is called (bTestVisible) its usage is counter-intuative

The method works for ALL resources, not just strategics

> pCity = Players[0]:GetCapitalCity()
> pCity:IsHasResourceLocal(GameInfoTypes.RESOURCE_IRON)
false
> pCity:IsHasResourceLocal(GameInfoTypes.RESOURCE_IRON, false)
false
> pCity:IsHasResourceLocal(GameInfoTypes.RESOURCE_IRON, true)
true
> pCity:IsHasResourceLocal(GameInfoTypes.RESOURCE_WHEAT, false)
false
> pCity:IsHasResourceLocal(GameInfoTypes.RESOURCE_WHEAT, true)
true
> pCity:IsHasResourceLocal(GameInfoTypes.RESOURCE_INCENSE, false)
false
> pCity:IsHasResourceLocal(GameInfoTypes.RESOURCE_INCENSE, true)
true

> -- Build improvements

> pCity:IsHasResourceLocal(GameInfoTypes.RESOURCE_IRON, false)
true
> pCity:IsHasResourceLocal(GameInfoTypes.RESOURCE_IRON, true)
true
> pCity:IsHasResourceLocal(GameInfoTypes.RESOURCE_WHEAT, false)
true
> pCity:IsHasResourceLocal(GameInfoTypes.RESOURCE_WHEAT, true)
true
> pCity:IsHasResourceLocal(GameInfoTypes.RESOURCE_INCENSE, false)
true
> pCity:IsHasResourceLocal(GameInfoTypes.RESOURCE_INCENSE, true)
true

 

[Craig_Sutter]

Thank-you... that's definitive.

Removing iron did have an effect on my print statements, but perhaps that's because the cities I generally place on my scenario maps are prone to having that resource nearby. It may by just RNG in action.

I guess I'm going to have to revisit my function to figure out what's going wrong then. I'll look closely at the examples provided and see what else I can figure out.

 

[Craig_Sutter]

It occurs to me that my cities, being low population, may not have been working the improved resources. I'll try "true" and see what occurs. That may be the ticket.

I'll inform of results.

 

[Craig_Sutter]

Just an update...

I have figured out the problem. It has to do with Worldbuilder.

In world builder, if you pre-place cities and/or paint culture, the resources (which I generate randomly with the worldbuilder resource lua) are not considered part of any city workable plots. This shows up in two ways... the lua function above only works for resources in newly acquired plots... when I buy plots during the game, the resources activate the lua file as expected. However, if they are within owned plots at the game start, they do not register.

This shows up in another way as well. Buildings that require a improved workable resource, Stable for example, will not be buildable if the resource required is inside cultural borders at the start of the game. If the resource is in a newly purchased or acquired plot, the building is buildable.

I do not know the genesis of this problem... it could be the worldbuilder resource lua or something in how cultural plots are assigned to cities by the worldbuilder. It does cause a problem, though.

My solution, and I'll post further if it works, is to add cities via lua rather than preplace them on the map. This will be in the initiation sequence, which I believe happens after resource placement.

I'll post further as things develop.