3UC/4UC for VP: Project Coordination Thread

[Infixo]

No, India was untouched. I ran lua and IsMounted was not reacting. It is also not visible in wikia. I used db.

This function is added in Community Patch.
I don't know what this means "not reacting".
If you call a function on an object and that function is not defined -> you get Lua error. No error means that the functions was called.
If you call a field name from a table and that field is undefined -> you get nil
.IsMounted is field in table Units, but :IsMounted() is function for Unit object.

 

[adan_eslavo]

I noticed it now:

local iGameSpeedModifier2 = GameInfo.GameSpeeds[ Game.GetGameSpeedType() ].GoldPercent / 100

for culture. Should be .CulturePercent ==> Dhanurgraha

Not reacting = I used print statements and with IGE I put 3 Horseman units. Lua read that they where Horseman (name) but did not enter next stage after "if pUnit:IsMounted() then". Print statement inside that if was not printed in lua.log. I changed it into db statement "GameInfo.Units[pUnit:GetUnitType()].IsMounted" and it worked. I cannot explain that.

 

[adan_eslavo]

Uff, busy bugfixing day. But good we managed to make it work.
No bugs:

 

[Infixo]

Not reacting = I used print statements and with IGE I put 3 Horseman units. Lua read that they where Horseman (name) but did not enter next stage after "if pUnit:IsMounted() then". Print statement inside that if was not printed in lua.log. I changed it into db statement "GameInfo.Units[pUnit:GetUnitType()].IsMounted" and it worked. I cannot explain that.

Because Horseman is NOT IsMounted... this flag is only used together with Archery units to distinguish between normal archers and mounted archers.
So, if you want to check for any mounted unit you need to check for combat class Mounted or IsMounted().
And, btw .IsMounted is number field. It returns 0 or 1. And lua treats 0 as true unless you write explicit condition. So, if .IsMounted then will always be true. If you write if .IsMounted == 1 then, then horseman returns 0 and condition is false.

 

[Infixo]

It’s good to remember that in Lua expression is false ONLY in two cases: it yields NIL or it yields strict boolean FALSE. All other cases, it is alwys TRUE. So, the statement if 0 then xxx is actually always true.

 

[adan_eslavo]

Ok, now I understand. So we wanted o track Range Mounted Units not all Mounted units. Sorry for that. Do needed tweaks then.

That lua "0" also tricked me somehow.

 

[pineappledan]

Oh shoot. No we want to track all mounted units

I didn't know that IsMounted is only set for mounted archery units, but not for mounted melee units. In that case we want both IsMounted and UNITCOMBAT_MOUNTED

When I wrote it, I thought that just checking for IsMounted was killing 2 birds with 1 stone. We definitely want it to affect melee units, or else Sofa will not be buffing Songhai's other UU - the Mandekalu

EDIT: new code:

function HorseDaddy(iPlayer)
local pPlayer = Players[iPlayer]
  for pUnit in pPlayer:Units() do
    if GameInfo.Units[pUnit:GetUnitType()].IsMounted or pUnit:GetUnitCombatType() == UNITCOMBAT_MOUNTED then
      pUnit:SetHasPromotion(iHorseDadEffect, pUnit:IsWithinDistanceOfUnitPromotion(iSofa, 2, true, false))
    end
  end
end

 

[adan_eslavo]

Haha, and thanks to my mistake and lack of knowledge we found out that it was buggy. Nice!

Actually it can be like that:

function HorseDaddy(iPlayer)
local pPlayer = Players[iPlayer]
  for pUnit in pPlayer:Units() do
    if pUnit:IsMounted() or pUnit:GetUnitCombatType() == UNITCOMBAT_MOUNTED then
      pUnit:SetHasPromotion(iHorseDadEffect, pUnit:IsWithinDistanceOfUnitPromotion(iSofa, 2, true, false))
    end
  end
end

 

[Infixo]

I've fixed that.

And it should be

if pUnit:IsMounted() or pUnit:GetUnitCombatType() == GameInfoTypes.UNITCOMBAT_MOUNTED then

 

[adan_eslavo]

@Infixo I'm just curious about those "e" instead of "i" in "eUnitCombatMounted". Do you mark indexes using this letter to differentiate them from standard integers used in calculations?

 

[Infixo]

I've just learned that Dromon changed into Penteconter.
First, I wouldn't expect any changes to VP from the mod that is supposed to add 2 new objects to each civ. I mean - why?
Second, it's quite confusing, because now UNIT_BYZANTINE_DROMON is Penteconter and new unit is UNIT_BYZANTIUM_DROMON. Like, really?

Are there any more cases when standard VP objects get changed? People will ask about that.

 

[adan_eslavo]

https://forums.civfanatics.com/thre...ordination-thread.623355/page-6#post-14901456
I try to update it when possible. It can be outdated somehow.

 

[Infixo]

@Infixo I'm just curious about those "e" instead of "i" in "eUnitCombatMounted". Do you mark indexes using this letter to differentiate them from standard integers used in calculations?

Yup, Hungarian notation. i is for integer, e is for enumeration. Since they are both implemented as integers, it's more a matter of rules and sometimes contex. Values stored in 'e' are usually unique and coming from a well-defined set.

 

[adan_eslavo]

Yup, Hungarian notation. i is for integer, e is for enumeration. Since they are both implemented as integers, it's more a matter of rules and sometimes contex. Values stored in 'e' are usually unique and coming from a well-defined set.

Good to know. I will try to follow that pattern in future. There's no sense in changing current code to that, right?

Sometimes we tweaked VP UCs to match balance issues. The Dromon/Peneteconter is the biggest change. As of naming of those units - I saw worst examples during making promotion overhaul. Believe me.

 

[Infixo]

https://forums.civfanatics.com/thre...ordination-thread.623355/page-6#post-14901456
I try to update it when possible. It can be outdated somehow.

There's a wiki available in GitHub - maybe it's easier to copy all info there? Eventually people will read texts in Civilopedia or one well prepared www page (or that google spreadsheet). I don't think anybody will search posts here (except OP).

 

[adan_eslavo]

Links to that are in 1st post. There's also xls spreadsheet I try to convert into that post. That is temporary solution. I thought about copying that into civfanatics download site when it is ready.

 

[Infixo]

Good to know. I will try to follow that pattern in future. There's no sense in changing current code to that, right?

No. Leave it like it is.

There's actually one more thing related to coding practices that it's worth noting. The names of functions that handle events usually start with "On". Like for EventXXX we have OnEventXXX. You can add some more info like OnEventXXXDoingThis or OnEventXXXDoingThat.

 

[Hinin]

I've just learned that Dromon changed into Penteconter.
First, I wouldn't expect any changes to VP from the mod that is supposed to add 2 new objects to each civ. I mean - why?
Second, it's quite confusing, because now UNIT_BYZANTINE_DROMON is Penteconter and new unit is UNIT_BYZANTIUM_DROMON. Like, really?

Are there any more cases when standard VP objects get changed? People will ask about that.

There are some cases where base VP elements are changed :
- the Pogost is put back at Rifling (so, its Game Info should be changed, since it still claims that the UB is available earlier than the Arsenal)

- the Conquistador benefits from some changes in terms of promotions (you can see it page 6), but the resulting effects are the same

- the Ethiopian Mehal Sefahi get the "Cover I" promotion replaced by "Sentry", because the Shotelai and the MSefahi are in the same unit line, and so the MSefahi was considered as too powerful of a unit with the promotions of the two Ethiopian UMs

- the Carolean becomes a Fusilier replacement (so that all the Swedish UMs don't become available at the same time)

- the Wat has seen the culture bonus transfered toward the Siamese UB2, and gets a new anti-spy bonus instead

- the Polder gets some changes (can be built on all marshes ; drains all MP from hostile units passing on it ; losing the gold bonus toward Villages and Towns)

- the Atlatlist unique promotion is weaker (+33 % CS against wounded units instead of +50 % CS) but not lost on upgrade anymore

- the "Pilum" promotion of the Legion isn't lost on upgrade anymore, and the unit gets a bonus to building speed when within two tiles of a GGeneral

- the Burial tomb gives a bonus to tiles with Flax (the Egyptian unique bonus resource)

- the UA of the Celts gains a bonus toward GMerchant creation : +2 GM points in capital when you obtain a pantheon, found a religion and improve it (so +6 GM points for free, in order to make the Oppidum UI more relevant).

- the Dromon becomes the Byzantium UM2, simply because it is an interesting unit in terms of concept (Greek fire... yeah !). I understand that it can create some compatibility problems, but it was obvious that, if possible, we would choose an already existing unique unit for a civilization instead of having to search for other relevant unique units for that civilization

- some leader flavours and starting bias are also changed (ex. : now the Ottomans have a "coastal starting bias)

- some names are changed (the Nau becomes the Carrack, the Ger becomes the Yurt, and the Naga-Malla becomes the Indian War Elephant, which is odd, since now you have Indian Indian War Elephant => I think we should go back to the original name for this unit, @adan_eslavo).

 

[adan_eslavo]

I would add also:
- grocer giving bonus to Figs
- Mission buffing Haciendas
- unit name changes (4 if I remember correctly)

 

[Hinin]

I would add also:
- grocer giving bonus to Figs
- Mission buffing Haciendas
- unit name changes (4 if I remember correctly)

How yes, the Ottoman "Siege Foundry" becomes the "Imperial Cannon Foundry" in addition to the three above.
As I said, I think the Indian War Elephant needs to recover its original name : it is weird to battle Hunnic Indian War Elephant against Indian Indian War Elephant. We originally changed the name in order to have this 'language harmonization" we wanted for the base modmod (so that we could give "native" names in another version), but I think that the result here is not worth it. What do you think, @adan_eslavo ?