Why would this code not work properly with vanilla?

[General Tso]

This code is supposed to provide bonuses to a player one time when they acquire their first city. It works fine for me when I test it using my Gods & Kings version of the game. I have even tried disabling the Gods & Kings part of the code and it still works for me. A user that is using the vanilla version of the game says that it gives then the bonuses every turn. I can't see how that is happening. Does anybody have any idea what could be causing the problem for the vanilla user?

I assume the important parts are the GetHasCityItems and SetHasCityItems functions since they deal with determining if the bonuses have already been given.

-- ModData access ----------------------------------------------------------------------------------------------------- 
SaveData = Modding.OpenSaveData();
HasCityItems = {};


-- Place city items if necessary (for Gods & Kings users) ---------------------------------------------------------------------------------------
function OnPlayerCityFounded(playerID, cityX, cityY)
	PlaceCityItems(playerID, cityX, cityY);
end
GameEvents.PlayerCityFounded.Add(OnPlayerCityFounded);

-- Place city items if necessary (for all users) ------------------------------------------------------------------------------------------------
function OnCityCaptured(playerID, bCapital, cityX, cityY, newPlayerID)
	PlaceCityItems(newPlayerID, cityX, cityY);
end
GameEvents.CityCaptureComplete.Add(OnCityCaptured);

-- Place city items if necessary (for users that do not have Gods & Kings) ----------------------------------------------------------------------
function OnPlayerDoCityTurn(playerID)
	player = Players[playerID];
	city = player:GetCapitalCity();
	
	if city ~= nil then
		PlaceCityItems(playerID, city:GetX(), city:GetY());
	end
end
GameEvents.PlayerDoTurn.Add(OnPlayerDoCityTurn);

-- Internal Functions ----------------------------------------------------------------------------------------------------------------

function GetHasCityItems(playerID)
	local name = "GTAS_HasCityItems:" .. tostring(playerID);
	
	if HasCityItems[name] == nil then
		HasCityItems[name] = SaveData.GetValue(name);
	end

	return HasCityItems[name];
end

function SetHasCityItems(playerID, hasItems)
	local name = "GTAS_HasCityItems:" .. tostring(playerID);
	
	if HasCityItems[name] ~= hasItems then
		HasCityItems[name] = hasItems;
		SaveData.SetValue(name, hasItems);
	end
end

-- If player hasn't been given city items yet in this game then give them and update status.
function PlaceCityItems(playerID, cityX, cityY)
	local player = Players[playerID];
	
	if not player:IsMinorCiv() then		
		if GetHasCityItems(playerID) ~= 1 then
			local slot = SlotData:GetSlot(playerID);
			
			if slot ~= nil then
				player:SetGold(player:GetGold() + slot.startGold);
				player:SetJONSCulture(player:GetJONSCulture() + slot.startCulture);
				player:SetNumFreeTechs(player:GetNumFreeTechs() + slot.freeTechs);
				player:SetFaith(player:GetFaith() + slot.startFaith);
			end
	
			SetHasCityItems(playerID, 1);
		end		
	end
end

I've also attached the original file in case you would rather look at that instead of the scrunched up on screen code box.

 

[whoward69]

In Vanilla, player:SetFaith() will fail, which means that SetHasCityItems(playerID, 1); will never execute which means the player will get the Gold, Culture, FreeTechs and (failing) Faith every turn

 

[General Tso]

Thank you! I never thought about that.

If that code would only have generated Lua errors I would have found it long ago. I should be used to that situation by now.

 

[General Tso]

Just to make sure:

Would this code cause any problems in vanilla?

if player.SetFaith ~= nil then
    player:SetFaith(player:GetFaith() + slot.startFaith);
end

It works fine with G&K.

I realize this is a very obvious question but I can't test it my self. I just wanted to make sure I'm not missing something else.

 

[whoward69]

Why not just use one of the techniques in this thread http://forums.civfanatics.com/showthread.php?p=12201777

 

[General Tso]

Ha Ha there always one in every crowd.

I don't actual need to know if the expansion exists. I need to know if SetFaith exists. In this case I thought this way would be OK.

 

[whoward69]

Problem is, if some other mod in the future comes along and (tries to) add a SetFaith() method, your code will be unpredictable.

From a logic point of view you want

if (thisIsGodsAndKings) then
doSomething()
end

and not

if (somethingHappensToExist) then
doSomething()
end

Defining a (global) flag to indicate if G&K is present is therefore (IMHO) cleaner, clearer and easier to understand/maintain in the future.

 

[General Tso]

OK you talked me into it. I'll use the idea I suggested in the other thread (and you tried to talk me out of).

 

[whoward69]

You want the code in post #4 or #2 (not the loop in #1 - which is what I was trying to talk you out of!)

 

[General Tso]

I just used the loop to show every DLC that was present it wasn't meant to be used for checking if only the expansion was present. I only need one check for that. Thanks for the help either way.