Scaling purchase costs for faith/culture/science/etc. with speed

[SeventhM]

Also, added culture to statCostModifiers so it scales as expected

[yairm210]

Instead of adding it to each item in the yield, we should add this once at the bottom, to the final minOrNull()

And for the sequence of ints - do we really need it there? I think that should also be covered by a one-time multiplication instead of per-item

[SeventhM]

we should add this once at the bottom

Can't be done. Some of the checks already are multiplied by game speed, such as the gold costs or the BuyBuildingsByProductionCost check. We can't really rip the speed multiplier from those checks cleanly either thanks to how specifically the gold cost works

And for the sequence of ints - do we really need it there?

I can double check this. I remember it being weird that we got a sequence of ints in the first place as it's usually immediately turned into a float/double elsewhere. Given the intent, though, I'm not sure there's necessarily a better way to handle it besides having a sequence of some sorts (either a sequence of floats or a sequence of ints)

[SeventhM]

I can double check this. I remember it being weird that we got a sequence of ints in the first place as it's usually immediately turned into a float/double elsewhere. Given the intent, though, I'm

Looks like no. Because of how some of these functions are done, no matter how you do it you need to cast at least something. The best we can do is switch this to a float, which avoids needing to cast literally everything to an int and disrupts the least amount of things. But the only way to avoid casting some of these things is to cast them on a one by one scale

Also, because of the intent (getting the lowest value out of a selection of values), I'm not sure there's a good way to avoid all of these casts and in general of these ints/floats without disrupting a lot. Maybe we could avoid using a sequence by just returning the first value in a lot of these cases, but you'd still need to minimal either cast things on a one by one basis or change the type for this function to, say, an Any?, or just uproot most of the code to work with doubles

[yairm210]

I still really don't like the 'apply function to each item individually' pattern, and Now that I've seen in-code how these functions differ between baseunit and building, I have a solution.

Both of them have a list of possible stat costs, taking the super.getBaseBuyCost into account.
The difference is that BaseUnit has a separate function for generating this sequence of possible costs, while Building does it all in the same function.

So my suggestion is:

• Convert Building to act like BaseUnit - creating a getBaseBuyCosts:Sequence function
• Both getBaseBuyCosts (Building and Baseunit) will have the gamespeed changes at the bottom of the function - like `sequence {...}.map{ it * civ.gameInfo.speed.statCostModifiers[stat]!!}

I think this will make the code clearer, but maybe I'm just making it worse by adding another layer of indirection. What do you think?

[SeventhM]

So, I have 2 problems with this. The first is I actually like how buildings handle it more than how units handle it. unit.getBaseStatCost calls unitCostFunctions.getBaseStatCost, which calls unit.getBaseBuyCost which calls unitCostFunctions.getBaseBuyCost. I not sure what adding this system would do for buildings besides makes it messy and bounce around

The second is you are missing my actual point. So let's take buildings for example. It finds the lowest cost between multiple different uniques. All of these possible prices needs to get multiplied by the game, except 2:

• super.getBaseBuyCost, as it includes the the base gold cost, which explicitly does not scale the same across game speeds the way the rest of these do and
• UniqueType.BuyBuildingsByProductionCost. This one calls getProductionCost, which is already scaled by game speed by default

Best case scenario, you build the sequence explicitly not including these two, multiply that by game speed, then build another sequence for these two plus what's already been worked on. That simplifies everything but at the cost of adding an extra sequence in there, which makes the unit version look even more complicated. I guess technically, you can have BuyBuildingByProdiction use the base cost of the unit/building instead of getProductionCost. That would allow you to multiply everything except super.getBaseBuyCost by the game speed, then simply find the lowest of the two left. But that would mean that the cost wouldn't be modified by the difficulty as intended

[yairm210]

:/
I see
How annoying