Need help correcting a Lua code. (Also found a neet tool to correct syntax).

[Hardstyle]

Welp, so I made a code that took me the whole day (even if it is simple as heck), only to end up with it not working at all even after correcting all syntax errors i could have made.
Oh, by the way here is the online tool I used to find my syntax errors: https://www.tutorialspoint.com/execute_lua_online.php

So what I'm trying to achieve is to get + 2 additional faith per turn every time I unlock a policy. So here is my code:

function OnPolicyAdopted(playerID)
local player = Players[playerID]
        if player:IsAlive() and player:GetCivilizationType() == testleaderID then
        for city in player:Cities() do
       if city:IsHasBuilding(palace) then
       local numPolicies = Player:GetNumPolicies()
       city:SetNumRealBuilding(GameInfoTypes.BUILDING_SMH_DUMMY_FAITHPOLICIES, numPolicies)
       end
end
end
end
if isTestLeaderActive then
   GameEvents.PlayerAdoptPolicy.Add(OnPolicyAdopted)
end

I Have made "locals" for:
-palace
-testleaderID
and they work fine on other codes i used and i have no errors in the lua.log.

Edit: nevermind for the "neat tool" just realized modbuddy does the same apparently.

 

[Troller0001]

if player:IsAlive() and player:GetCivilizationType() == testleaderID then

This line is your issue. Your civilization will not necessarily equal a civ's leader. E.g. CIVILIZATION_MONGOL with an ID of 20 vs LEADER_GENGHIS_KHAN with an ID of 19!

Spoiler :

It can be fixed by doing one of the following things:

if player:IsAlive() and player:GetLeaderType() == testleaderID then

local testCivilizationID = GameInfoTypes.CIVILIZATION_HARDSTYLE_FAITH_FOR_POLICIES_OR_STH;
--snip
if player:IsAlive() and player:GetCivilizationType() == testCivilizationID then

The latter case is more commonly used in my experience.

---
There's a typo here too:

local numPolicies = Player:GetNumPolicies()

should be (note the capital letter!)

local numPolicies = player:GetNumPolicies()

----
Also, while the following part isn't necessarily wrong, it is rather inefficient and/or redundant

for city in player:Cities() do
      if city:IsHasBuilding(palace) then
          local numPolicies = Player:GetNumPolicies()
          city:SetNumRealBuilding(GameInfoTypes.BUILDING_SMH_DUMMY_FAITHPOLICIES, numPolicies)
      end
end

It loops over all cities checking if they have a palace (which in all cases denotes that city being the capital). If that city has it, then it receives the dummies.

A more efficient way of doing it would be to replace that entire fragment with the following, since that only involves checking one city:

local pCapital = player:GetCapitalCity();
--do nothing if the player doesn't have a capital
if pCapital then
   local numPolicies = player:GetNumPolicies();
   pCapital:SetNumRealBuilding(GameInfoTypes.BUILDING_SMH_DUMMY_FAITHPOLICIES, numPolicies);
end

---
Note that in the case of your capital being captured you lose all faith output until you adopt another policy! Add a GameEvents.CityCaptureComplete(...)-check to see when that happens so you can move the dummies to the new capital!

---
Also, I'm assuming you have your file set up correctly: https://forums.civfanatics.com/thre...t-vfs-ingameuiaddin-or-updatedatabase.487846/

 

[Hardstyle]

I didn't think about efficiency ... your code indeed looks more straigt forward.
Thanks for the correction!

 

[Hardstyle]

Here I'v got another problem understanding a bit of code. I'm trying to understand a code made by JFD to reverse engineer it, but there is one thing I'm not sure to fully understand with the syntax.

local BuildingOneCSID = GameInfoTypes["BUILDING_SMH_LIBYA_ONE_CITYSTATE"]
function SMH_GetNumCityStateFollower(playerID, religionID)
local numCityStateFollowers = 0
for otherPlayerID = 0, GameDefines.MAX_MINOR_CIVS - 1 do
   local minor = Players[otherPlayerID]
   local minorCapital = minor:GetCapitalCity()
   if (minor:IsAlive() and minor:IsMinorCiv() and minorCapital) then
       if (minorCapital:GetReligiousMajority() == religionID and religionID > 0) then
           if (not minorCapital:IsHasBuilding(BuildingOneCSID)) then
               minorCapital:SetNumRealBuilding(BuildingOneCSID, 1)
           end
               numCityStateFollowers = numCityStateFollowers + 1
           else
               if minorCapital:IsHasBuilding(BuildingOneCSID) then
                   minorCapital:SetNumRealBuilding(BuildingOneCSID, 0)
               end
           end
       end
   end
   return numCityStateFollowers
end

How does "local numCityStateFollowers = 0" is beeing affected by this code? It is never used in the part adding the building "BuildingOneCSID" and then the code looks like it change the value of this variable to "numCityStateFollowers = numCityStateFollowers + 1" without any reason. What I don't get is how this variable is connected to the rest of the code since it looks to me that this value is not affected by a line like

if minorCapital:IsHasBuilding(BuildingOneCSID) then
numCityStateFollowers = numCityStateFollowers + 1

and just sit there without any apparent link.

Could someone enlighten me?

 

[Troller0001]

How does "local numCityStateFollowers = 0" is beeing affected by this code? It is never used in the part adding the building "BuildingOneCSID" and then the code looks like it change the value of this variable to "numCityStateFollowers = numCityStateFollowers + 1" without any reason. What I don't get is how this variable is connected to the rest of the code [snip]

What the code also does is add BuildingOneCSID to a city if that city is a City State's capital with religionID as its majority religion (It excludes religious pantheons (which have an ID of 0)).
BuildingOneCSID is removed if the city is not having religionID as its majority religion (or religionID = 0 in which case it's a pantheon).

numCityStateFollowers keeps track of how many City states pass the 'non-pantheon religious majority'-check. I.e. how many CSs received BuildignOneCSID. This value is also returned.

---

What is interesting to me though is this line:

for otherPlayerID = 0, GameDefines.MAX_MINOR_CIVS - 1 do

since that loops over the first 41 players (player 0 up-to and including player 40). The first 22 players, however, will never be city states, as these slots are reserved for major civs.
Furthermore, players 43-62 will not be considered either, while these may be city states!
It seems to me that it should have been the following (considering the function name and what it checks for)

for otherPlayerID = GameDefines.MAX_MAJOR_CIVS, GameDefines.MAX_CIV_PLAYERS-1 do

since this loops only over the City States (players 22 up-to and including 62)
NOTE: It is still a good idea to check for player:IsMinorCiv() though as certain mods may allow CS slots to be taken by major civ slots (I.e. mods that allow you to play with more than 22 major civs!)

 

[Hardstyle]

Well thanks for the explaination . After what you said I'm thinking maybe this mod wasnt designed to work with 43 civ optional from vox populi. because I tested the civ from JFD and the UA defined by this code doesn't work at all with 43 civ.

 

[Hardstyle]

Well, now I have two codes I need help with.
The first one is supposed to give invisibility to my unit when it enters a forest or jungle tile, but when my unit gets out of these terrains type the promotion isn't removed. The base code is from LastSword.

here is the first code:

GameEvents.UnitSetXY.Add(function(player, unit, x, y)
local cUnit = Players[player]:GetUnitByID(unit);
   if cUnit:IsHasPromotion( GameInfoTypes.PROMOTION_SMH_VDV ) then
       if Map.GetPlot(x,y) ~= nil then
           local cPlot = Map.GetPlot(x,y);
           if cPlot:GetFeatureType() == GameInfoTypes.FEATURE_FOREST or GameInfoTypes.FEATURE_JUNGLE  then
               if not cUnit:IsHasPromotion( GameInfoTypes.PROMOTION_INVISIBLE_SUBMARINE ) then
                   cUnit:SetHasPromotion( GameInfoTypes.PROMOTION_INVISIBLE_SUBMARINE ,true);
               end
           else
               if cUnit:IsHasPromotion( GameInfoTypes.PROMOTION_INVISIBLE_SUBMARINE ) then
                   cUnit:SetHasPromotion( GameInfoTypes.PROMOTION_INVISIBLE_SUBMARINE ,false);
               end
           end
       end
   end
end)

For the second one I wanted to add a dummy building to my capital that give +1 delegate for each global monopoly (Vox populi). Jarcast2 gave me a code that works perfectly for another dummy building I add to certain cities. So I wanted to use the same code to add another dummy building, but only in the capital. and of course it doesn't works.

the second code:

function Oblast_MonopolyBonus(iPlayer)
    local pPlayer = Players[iPlayer]
    if (pPlayer:GetCivilizationType() == GameInfoTypes.CIVILIZATION_SMH_RUSSIA and pPlayer:IsAlive()) then
        local monopolycount = 0
        for pResource in GameInfo.Resources() do
            if(pResource.IsMonopoly == 1) and pPlayer:GetMonopolyPercent(pResource.ID)>= 51 then
            monopolycount = monopolycount + 1
            end
        end
        for pCity in pPlayer:Cities() do
            if pCity:IsHasBuilding(GameInfoTypes["BUILDING_SMH_OBLAST"]) then
                pCity:SetNumRealBuilding(GameInfoTypes["BUILDING_SMH_OBLAST_YIELD_MODIFIER"], monopolycount)
            end
        end
    end
end
    
GameEvents.PlayerDoTurn.Add(Oblast_MonopolyBonus)

and the code I added to get the other dummy building only in my capital

function SMH_Delegate_PlayerDoTurn(playerID)
   local pPlayer = Players[playerID]
   if (pPlayer:GetCivilizationType() == GameInfoTypes.CIVILIZATION_SMH_RUSSIA and pPlayer:IsAlive()) then
local pCapital = player:GetCapitalCity();
if pCapital then
   pCapital:SetNumRealBuilding(GameInfoTypes["BUILDING_SMH_RUSSIAN_DELEGATE"], monopolycount);
end
end

 

[LeeS]

 if cPlot:GetFeatureType() == GameInfoTypes.FEATURE_FOREST or GameInfoTypes.FEATURE_JUNGLE  then

needs to be

 if (cPlot:GetFeatureType() == GameInfoTypes.FEATURE_FOREST) or (cPlot:GetFeatureType() == GameInfoTypes.FEATURE_JUNGLE)  then

--------------------------------------------

(pResource.IsMonopoly == 1)

If column "IsMonopoly" is defined as a boolean column, then lua will expect comparison to (and return) true/false rather than 0/1

--------------------------------------------

local pCapital = player:GetCapitalCity();

player is an undefined object variable and therefore gives a nil value error when attempting methods on it such as :GetCapitalCity();

You defined the variable within function SMH_Delegate_PlayerDoTurn as

local pPlayer = Players[playerID]

pPlayer ~= player.

 

[Hardstyle]

Well, now thanks to Lee everything works perfectly except for the line that is supposed to add a dummy building to the capital. In fact I'm not sure if the problem comes from my code itself or the fact I'm using a variable from another code. (but if I'm not mistaken it should work since "monopolycount" is a global value?)

function SMH_Delegate_PlayerDoTurn(playerID)
   local pPlayer = Players[playerID]
   if (pPlayer:GetCivilizationType() == GameInfoTypes.CIVILIZATION_SMH_RUSSIA and pPlayer:IsAlive()) then
       local pCapital = pPlayer:GetCapitalCity()
       if pCapital then
           pCapital:SetNumRealBuilding(GameInfoTypes["BUILDING_SMH_RUSSIAN_DELEGATE"], monopolycount)
        end
    end
end

GameEvents.PlayerDoTurn.Add(SMH_Delegate_PlayerDoTurn)

Or maybe there is a way to incorporate this chunk to the other working code to add both "BUILDING_SMH_OBLAST_YIELD_MODIFIER" and "BUILDING_SMH_RUSSIAN_DELEGATE" but with different conditions?

Reminder: what i want to achieve is get the "BUILDING_SMH_OBLAST_YIELD_MODIFIER" in cities that have the "Oblast" building for each global monopoly( this parts works flawlessly) and add the "BUILDING_SMH_RUSSIAN_DELEGATE" only to the capital but again for each global monopoly.

 

[LeeS]

monopolycount is not a global, it is localized within the Oblast_MonopolyBonus function and therefore does not exist (ie, is a nil value) anywhere else. Even if you have a variable called monopolycount at the "root" level of the file, this variable will not be updated by anything that happens within function Oblast_MonopolyBonus. Since both Oblast_MonopolyBonus and SMH_Delegate_PlayerDoTurn appear to be hooked to GameEvents.PlayerDoTurn you can roll the code into one master function.

It would be simplest to add the capital city effects directly to function Oblast_MonopolyBonus in place of your current "for" loop through the player's cities as

for pCity in pPlayer:Cities() do
	if pCity:IsHasBuilding(GameInfoTypes["BUILDING_SMH_OBLAST"]) then
		pCity:SetNumRealBuilding(GameInfoTypes["BUILDING_SMH_OBLAST_YIELD_MODIFIER"], monopolycount)
	end
	if pCity:IsCapital() then
		pCity:SetNumRealBuilding(GameInfoTypes["BUILDING_SMH_RUSSIAN_DELEGATE"], monopolycount)
	else
		pCity:SetNumRealBuilding(GameInfoTypes["BUILDING_SMH_RUSSIAN_DELEGATE"], 0)
	end
end

And then you can dispense entirely with SMH_Delegate_PlayerDoTurn.

Just be warned though that you have no code to remove either of these two buildings from cities that have been captured by another player, unless you've added NeverCapture=true in the XML or SQL that defines the two buildings.

 

[Hardstyle]

Hmm, I see. I read somewhere that variables that are not preceded with "local" are considered globals. I probably got that wrong...
Anyway thank you very much for your time Lee!

 

[LeeS]

you had

local monopolycount = 0

inside your function, which localized the usage of that variable name to that function. Even if you had the same variable-name established outside of that function, the function would use and only update the localized version of the variable, and would not update the "global" version of the variable.