Well, props to you for not giving up at least. You've given this more attempts than some others.
Responding to your code snippets in order...
On the surface, your code technically should, and probably does, run fine. It's simply not doing what you are expecting, because your logic is incorrect.
You have to understand what exactly is happening when you call
iterators such as
pPlayer:Cities() and
pPlayer:Units().
Here is a hypothetical scenario:
Let us assume you have a fairly small Empire right now - You only have
3 Cities.
They are creatively named
CityA,
CityB, and
CityC.
Let us also assume that you have managed to construct your wonder in
CityB.
CityB is already connected to your Capital, which is
CityA.
CityC will be connected in a few more turns, but for now is unconnected.
Setting aside all the other logic for now, let us look specifically at the loop iterator:
Code:
for [B]pCity[/B] in [B]pPlayer:Cities()[/B] do
As I've mentioned before,
pPlayer:Cities() is an
iterator -- this means it
iterates, or goes through a list of items (in this case,
Cities) one by one.
The game knows that you only have 3 Cities, so this means that this loop will run
three times.
The first part of the loop declaration defines
the name of a variable that you want to
temporarily hold the data for each iteration. In this case, every City the game finds from
pPlayer:Cities() is assigned
temporarily to the variable
pCity. This is why you do not need to "define" or "declare"
pCity beforehand.
Now, let's look at this in action:
What do you think
pCity holds during each iteration?
On loop #1,
pCity holds...
On loop #2,
pCity holds...
On loop #3,
pCity holds...
On loop #4,
pCity holds...
With that out of the way, let us inject the next part of your logic:
Code:
for pCity in pPlayer:Cities() do
if pCity:[B]IsHasBuilding[/B](HooverWonder) then
Notice anything?
You're checking for the presence of your Wonder
in that City before proceeding with any of your functions!
Let's apply this logic to the above information:
On loop #1,
pCity holds the data for
CityA...It doesn't have the Wonder, and there's no commands for what to do in this case, so it skips it and moves on.
On loop #2,
pCity holds the data for
CityB...It
does have the Wonder, so it proceeds to fire off the bit of code attached to this condition -- namely, adding the dummy building
to CityB.
On loop #3,
pCity holds the data for
CityC...It doesn't have the Wonder again, so it skips this City as well and moves on. Except there are no more Cities, so the whole function ends, because no other commands come after this.
Unrelated, but I also personally disagree with the whole "then return end" line, because if your conditions are set properly, it will end anyway without this explicit declaration. It just makes code more annoying to read for me. It also means that your first function will fail spectacularly because you've now got an extra "
end" in there. I know you tried to match up the indents to properly "close" each if/for block, except that one line
already has an end! That means you have an extra dangling
end in that function, causing Lua to go on a fritz and probably give you an
eof error. (Hard to tell? Delete
everything inside that indented pair of if/end, and you'll see that you have:
if ... then return end end)
Your second function is very bad as well. You do
not want to be nesting loops over the same list like that, as all that means is that you are exponentially increasing the resource load of your function for no benefit at all.
Normally calling the Cities iterator will net you 3 loops: once per City you own (from the above example.)
Nesting a Cities iterator inside a Cities iterator means that you loop over your Cities at every individual City loop, thus making your code run 12 loops for those same three Cities:
Loop #1 -
CityA -> Loop through
CityA,
CityB, and
CityC
Loop #2 -
CityB -> Loop through
CityA,
CityB, and
CityC
Loop #3 -
CityC -> Loop through
CityA,
CityB, and
CityC
However, since you stuck a condition there, you will actually only run 6 loops, since the secondary loop would only fire for
CityB. Still bad, though, as those extra loops are unnecessary.
It would be better if you simply queried the game to see if the player had the Wonder or not directly, since that is essentially the only reason for your checking of the Cities to find it. I believe there's a Player method that will return the number of buildings in a specified building class, though I'm having trouble remembering what it is. Maybe
pPlayer:GetNumBuildingClass() or something. LeeS will probably know this one.
Having said that, it
should theoretically be able to add the dummies to connected Cities, but as that's not shown to be hooked up to anything, I don't know. If it
was hooked up, it would properly provide the dummy to
CityA, but not
CityC until it completed its connection to
CityA.
Now, your third code snippet is probably close, but it's fairly inefficient.
First, separating functions into separate smaller functions is not necessarily a bad idea, and technically could even be a good idea, since it allows more modularity for re-using code between different functions.
The big problem is that the word
function is a word reserved for
creating functions in Lua. This is important, because this is the error you have.
This is fine:
Code:
function UpdateOrRemoveChecker(playerId)
As is this:
Code:
function AddHooverDummies(playerId, cityId, buildingType)
However! This is bad:
Code:
[COLOR="Blue"][B]if[/B][/COLOR] pCity:IsHasBuilding(HooverWonder) then
[B][COLOR="Red"]function[/COLOR][/B] AddHooverDummies(playerId, cityId, buildingType)
[COLOR="Blue"][B]end[/B][/COLOR]
There are two problems:
- When this if block runs, instead of calling your function, you end up trying to define a new one.
- This new function declaration does not have an associated end paired with it.
These two issues would cause that piece of the code to fail.
Removing the word
function would likely have that pair of functions start working, but in the end, you've still got the problem of snippet #2 -- double loops over the same list going on. Your functions also won't handle the scenario where a City's connection to
CityA becomes broken.
Again, the word
function is reserved for
creating functions. If you only want to
call the function, you must omit the word
function.
You really need to enable logging and check your Lua logs, because most of these issues would have produced an error in the logs which would allow you to pinpoint the issues, instead of forcing you to guess randomly.
Production in cities with a connection to the Capital.
), is there a better way I could write it to not take up much resources every turn when the wonder doesn't exist? Would it be better if broken up into 2-3 functions or is just 1 big one fine?
