CivEffects

[Kailric]

Well, if I knew what to do to help you I would

 

[Nightinggale]

I'm having an issue with the tag CivicTreasuryBonuses.

The problem is that it stores the bonus as a float. However I want to read it into an InfoArray, which is hardcoded to store everything in shorts

Possible solutions:

1. Change to int
2. InfoArray learns to handle floats
3. Store in a JIT array instead

I don't like any of those
1: it makes sense to use float
2: might not be possible. It is fundamentally hardcoded to shorts.
3: InfoArray is designed for quick cache updates. On cache update, a JIT array reads the index and then adds the value to that index. If it's a JIT array, it will add a bunch of 0 to the array, which due to being floats might cause minor rounding errors to accumulate.

Any bright ideas? Should we ban floats in CivEvents? It would be the sane solution from a coding point of view, but what about XML modder point of view?

 

[orlanth]

If the intent is to have it represent a percentage or a small fractional interest rate (this is the interest on treasury iirc), how about storing it as a short int and using something like 0.001 times its value as the in-game interest rate?

 

[Kailric]

The problem is that it stores the bonus as a float. However I want to read it into an InfoArray, which is hardcoded to store everything in shorts

Does it have to be read into an info array? That part can just be left alone for now if not. But perhaps it can be done like orlanth says, put in and int of say 2, and have it convert in the dll to 0.2.

 

[Nightinggale]

Does it have to be read into an info array? That part can just be left alone for now if not.

It does not have to do anything. However because we can end up with a huge amount of CivEffects, I want to cache the combined effect from all CivEffects owned by a player. This will not only be faster than the current code, but also have constant execution time, which is completely independent from how many CivEffects we have in XML.

Also I want to update the cache quickly, which is precisely what InfoArray is designed to do. It skips all zero values and only store the index and values for the rest. Since we can have 30 values and 28 zero values, not even looping the zero values will speed up the performance. Also InfoArray and JIT arrays know each other meaning cache update is just

JITarray.AddCache(iChange, InfoArray*);

iChange is then either 1 or -1, which mean CivEffects can be added or removed with this call.

Having variables, which doesn't follow this standard requires extra coding and will have a hard time keeping up to the performance standard.

But perhaps it can be done like orlanth says, put in and int of say 2, and have it convert in the dll to 0.2.

Yeah, we should probably do something like that, except if we call the tag something with percentage, then 20 becomes 0.2. We base most of our code on either int numbers or int percentages (divided by 100).

 

[Nightinggale]

The promotion setup currently in traits is garbage. It contains a bool array telling which promotions it gives for free. Then it has 3 bool arrays telling which units will get the free promotions. The arrays target unit classes, unit combat types and professions. This mean you can't give promotion A to mounted units and promotion B to traders since they share the promotion array.

Thinking a while on how to make this more flexible without making it an XML setup hell. I came up with this:

<FreeUnitClassPromotions>
   <FreeUnitClassPromotion>
        <UnitClass>UNITCLASS_TRADER</UnitClass>
        <FreePromotions>
                <PromotionType>PROMOTION_A</PromotionType>
                <PromotionType>PROMOTION_B</PromotionType>
                <PromotionType>PROMOTION_C</PromotionType>
        </FreePromotions>
   </FreeUnitClassPromotion>
   <FreeUnitClassPromotion>
        <UnitClass>UNITCLASS_SOMETHING_ELSE</UnitClass>
        <FreePromotions>
                <PromotionType>PROMOTION_D</PromotionType>
        </FreePromotions>
   </FreeUnitClassPromotion>
<FreeUnitClassPromotions>

This should allow any combo of promotions for the 3 types of unit selection and be fairly easy to set up. Next question is to code this. I think it would work, but it would require some testing to ensure it works reliably.

 

[Kailric]

Yeah, I hated how they set that up, it was lame, so it needs an improvement. Makes me think though, how are these changes effecting in game help text? I also have some Techs that grant a Trait, so we will have to change those. In the Help text, it would get the Tech help and then add to it the Trait help.

 

[Nightinggale]

Makes me think though, how are these changes effecting in game help text?

Currently I comment out the help text if it causes problems with the new design (that's like 3 lines or something). Other than that, I haven't touched it.

My plan is to finish the game mechanics, split the XML file into multiple files and then merge back into develop. I'm on a "timed" task for this because it is so easy to conflict with XML changes made in other branches.

Once everything is up and running, somebody needs to go through the entire help text display function(s) and update them to fit the new design. I have been wondering about concepts on how to display it in a more readable way than just list each change individually, hopefully in a way without too much coding. My plan is to make a function, which takes an InfoArray and TXT_KEYs as arguments since there are access functions to get pointers to all InfoArrays. This would (like the player cache) allow handling everything from a single InfoArray with just one line, which would make GUI updates for additional InfoArrays a lot easier. So far I haven't quite figured out how to do it, but that task haven't had my main focus.

We will also have to make some pedia updates. I would prefer if we can somehow write the effects in one file and then make each page call them. Otherwise we will have to duplicate a lot of display code.

EDIT:
Just spotted another issue. FasterBuildTypes gives an int, which is how long it will take to complete the task (building an improvement). The cache wants to add up all the bonuses. This mean that one CivEffect with a setting of 50 will make the worker complete the task in half the time. Two CivEffects both with 50 will be added into 100, in which case they cancel each other. I would like to change this into the change, meaning a 50% reduction is -50. This way two providing -50 each will add up to -100. However the current (uncached) code will apply them once at a time meaning it ends up with -75.

I'm not sure what to do here

Maybe a float JIT array where all the numbers are multiplied together and divided if a CivEffect is lost. I wonder if rounding errors can cause problems for this approach.

 

[Nightinggale]

After some thinking and some looking up precisely how floats/doubles behaves regarding rounding issues, I think I have come up with a solution.

We write all XML values in ints. That way they are compatible with InfoArray.
CvPlayer then cache in a float JIT array. When a modifier is present in the applied InfoArray, it will divide the modifier by 100 and then multiply it to the array value. Removing the InfoArray the array value is divided by (modifier/100).

When reading (the get function), it should return array value + 0.0001.

This mean 100 is unchanged speed, 50 is half speed, just like it is now. Having two CivEffects, both providing 50% will result in (1/0,5)/0,5 = 0,25 just like it is now.

Adding 0.0001 has to do with rounding errors. Imagine this example:
A task takes 4 turns. There is a 50% boost. 4*0,5 = 2, which mean it will have to take 2 turns.
Due to rounding errors, it might come up with:
4*0.499999 = 1.99999, which rounded to int gives 1 and suddenly we have a 75% speed improvement.
With the +0.0001 modifier, it would be:
4*0.50000999 (or 0.50001) = 2.000005 (or something)
Converting this to int will give 2.

The get modifier will have to be small enough to not affect the gameplay and at the same time significantly bigger than the rounding errors.

We should probably use doubles to ensure that the rounding errors stays as low as possible. Worst case scenario is rounding errors is applying a CivEffect and then remove it and we will not return to the original value.

What's left is figuring out what to do about non array modifiers. They should ideally get the same treatment, but I'm not in the mood to write the code for each of those. Besides I'm thinking of killing off a number of them. We don't need FF point bonus for all and an array with bonuses for each type. If it has to be for all, fill out the array. There are a number of those duplicated ways to do the same.

 

[Kailric]

That's one hell of a project you got going there. Leave it to Night to find the hardest thing to do I think you like being tested that way

 

[Nightinggale]

I just had an interesting idea for a new CivEffect Tag. It should modify iTeachLevel for units. That will allow a unit, which requires universities to suddenly be educated by colleges because you invented something or entered a new era.

Implementation would actually be interesting. CvPlayer should have a unitclass array. If the cache clear function sets it to the XML values rather than just 0, then we would be able to read the teach level from CvPlayer only and ignore XML data during the game.

Another thing which have crossed my mind is to split units.xml into two files. Those files being somewhat identical should have the same tags and be placed in the same vector. The idea being that we have one file for humans and one for non-human units. That way we don't have to loop ships, animals and stuff to see if we can teach them at the school and similar stuff. We can also make arrays, which only fit human units (like we have arrays for cargo yields only).

That's one hell of a project you got going there. Leave it to Night to find the hardest thing to do I think you like being tested that way

I'm not a masochist

The plan was to simply copy the tags from traits into civics, but it turned out that the tags were already a big mess and just adding them would mean half of them would not be used (half the trait tags are unused as it is). I then wanted to improve the cache to prevent this from being a slowdown and then one thing lead to another and had I known the scale of this huge tasks, I likely wouldn't have started. Now I'm happy that I didn't know because I like what I have made so far.

 

[Kailric]

As do the rest of us

 

[Lib.Spi't]

chances are a lot of the trait tags are left overs from civ4, that never 'really' got ported across.

 

[Nightinggale]

We need a naming convention for XML. Right now we have at least 4 systems doing nearly the same.

1. 0=disabled, 100=unchanged, 200 = double
2. -100=disabled, 0=unchanged, 100 = double
3. 0=disabled, 1=unchanged, 2 = double
4. Absolute values as in 2 gives +2

All cleverly named modifier meaning it is a research project in it's own to figure out what to add to a modifier to get the wanted result. Adding 50 to a yield modifier could mean either +50%, -50% or +50 production each turn

I'm thinking of merging 1-3 together into the code for 1 and then add an offset to make it act like 2 to the XML modders. This way -50 will mean -50% and it stacks meaning an additional -50% will affect the remains of the first modifier and end up with -75%.

4 should be unchanged.

Now the naming issue: how should we name the two remaining systems to both identify that they modify existing numbers and tell if it is absolute or percentage numbers? I came up with the postfixes PercentageModifier and AbsoluteModifier. Both are too long as it would mean names like ImprovementBuildingSpeedPercentageModifier (try to say that fast 10 times )

 

[Kailric]

Just shorten it to PModifier, AbModifier, etc. or PercentMod, AbMod, something like that would work.

 

[Commander Bello]

Now the naming issue: how should we name the two remaining systems to both identify that they modify existing numbers and tell if it is absolute or percentage numbers?

In most cases, the Firaxis naming convention seems to be "change" means a change in absolute numbers, while "modifier" means a percentage change.

 

[Kailric]

In most cases, the Firaxis naming convention seems to be "change" means a change in absolute numbers, while "modifier" means a percentage change.

That would make sense actually, me on the other hand went with what ever came to mind when I wrote my code. I'm all about the chaos, about the chaos, about the chaos, no sanity, .

 

[Nightinggale]

In most cases, the Firaxis naming convention seems to be "change" means a change in absolute numbers, while "modifier" means a percentage change.

That sounds reasonable. In that case the main problem is that the current modded tags doesn't follow the naming convention, not the lack of a convention. I will have to go through every single tag now that I have to apply the cache to CvPlayer get functions. I will rename according to this standard and write something about it on the wiki.

 

[Kailric]

TIn that case the main problem is that the current modded tags doesn't follow the naming convention, not the lack of a convention. I will have to go through every single tag now that I have to apply the cache to CvPlayer get functions. I will rename according to this standard and write something about it on the wiki.

Read my previous post . Sorry, Night, . Keep up the

 

[Nightinggale]

me on the other hand went with what ever came to mind when I wrote my code.

That makes two of us

I updated the first post. Even though the TODO list grows, the number of crossed out lines grows faster. It's actually nice to see all the crossed out lines because before I updated all I did was think of the remaining tasks, not how far I have come so far.