felling ax preciousness 254

I meant the assert would go to AI code to avoid similar surprises in the future...

I would prefer a check during tribe loading - if a tribe uses a ware, it also has to define its preciousness.

P.S: we also should compare to INVALID_INDEX rather than 30, if you would like to have an assert in the AI too.

30 or even 20 is arbitrary, but if average preciousness is like 6 and one ware gets 200 it will badly affect balance in AI.
ALTERNATIVELY I can just cap the value for AI's purpose to 20 and no crash (result of assert) but this can gone unnoticed for people who try to manipulate preciousness.

Would you mind official upper limit for preciousness like 20?

Relevant code is here:

int WareDescr::preciousness(const std::string& tribename) const {
        if (preciousnesses_.count(tribename) > 0) {
                return preciousnesses_.at(tribename);
        }
        return kInvalidWare;
}

So you say here in this point the game should trow exemption with detailed description which ware and tribe was culprit?

Preciousness has a big effect on AI as you noticed. Besides training it is one of the biggest levers we have to influence the AI for every tribe. From my perspective the basic ratio of wares needed of a tribe should be reflected in this values. So we have a valid starting point for generating "individdual characters" by the magic numbers, which will not let out of consideration the mechanics of their tribe. Currently these values are only set manually according to what the designers of the tribe "felt" it would be a ggod value. At least that is what I am guessing from having a look into the values. I played with them while making AI handling frisians better and it had great effects. Advantage of setting these values correctly is that AI training values will have mor ethe less the same effect on every tribe. if there is no solid base the traininf would bias the AI values with the weaknesses of the tribes preciousness definitions. this will lead to the values don't fit for a different tribe.
So why am I writing all this stuff? I simply don't know how much range of preciousness values do we need to implement correct ratios. But it is not improbable that we might need more than 50.

I inserted some printf to identify wares without preciousness, and I got many rows like:

Preciousness for Clay for atlanteans not defined in its init.lua file
Preciousness for Brick for atlanteans not defined in its init.lua file
Preciousness for Fruit for atlanteans not defined in its init.lua file
Preciousness for Honey for atlanteans not defined in its init.lua file
.....

So I understand why kInvalidWare is returned, only it is not reliable - sometime author forgets to insert entry for particular tribe and than it returns preciousness 254. We should somehow distinguish if ware is indeed invalid or just forgotten in init.lua files..

So I decided to change on AI, if it will get preciousness above 25, the value is capped with two possible scenarios and it would print one of two logs:

AI WARNING: Preciousness for Felling Ax (barbarians) received as: 254 (kInvalidWare), capping to 1. Perhaps not set in init.lua
AI WARNING: Preciousness for Felling Ax (barbarians) received as: 155, capping to 25. Value in init.lua too high.

I am going to propose the merge, we can discuss it...

from what your log delivered ir seems we should first check whether the ware is used by this tribe.

the lines you posted contain only frisian wares which are not used by the atlanteans at all.

so if the tribe uses this ware (= it is part of tribedescr.wareindex) and Preciousness is not set we should set this to a save default (maybe 5) and write a log that there is something undefined for testing purposes) else we should assing kInvalidWare in my opinion. This can be done while loading the tribe as well as in the current location.
Reason is we will catch and default any error thus making initial development of a tribe or a new ware easier.

Initially I wanted just to throw exemption, but than found out that must such cases are just invalid combinations of ware&tribe...

If you look at ware_descr.cc you can see multiple rows "return kInvalidWare" so the problem can be more widespread.... just going unnoticed...

I am not sure if tribedescr.wareindex is available while working in ware_descr.cc

> Preciousness for Clay for atlanteans not defined in its init.lua file

This is as designed, because Atlanteans don't need clay. Add a check for TribeDescr::hasWare() to get rid of unneeded wares.

> Preciousness for Felling Ax (barbarians) received as: 254

This is a bug and should be caught during tribe loading

> So I understand why kInvalidWare is returned, only it is not reliable - sometime author forgets to insert entry for particular tribe and than it returns preciousness 254. We should somehow distinguish if ware is indeed invalid or just forgotten in init.lua files..

Agreed. We could probably use INVALID_INDEX here, but it probably needs to be implemented into Tribes::postload and throw a GameDataError there. I am writing this from memory though, I haven't looked at the actual code.

I am pushing new branch as a replacement for failing one.

For me the elementary question is:
"Is preciousness mandatory?"
For me not necessarily. Game can provide default value, I propose 1.

I am not familiar with loading of tribes so depending on direction we choose I might not be able to do it. But please review my proposal in ai_too_many_prodsites2 branch

I think that having an arbitrary cap in the AI to circumvent a bug in the Tribes is a very bad idea. It will mask the bug, never to be fixed again. What probably happened here is that the information got lost during the One Tribe engine change when we manually converted the conf files into Lua files. We need to make sure that this never happens again.

Since I am familiar with the Tribes code, I'll take care of it.

Merge request is up. The Barbarian Felling Ax was the only offender. I have retrieved its original value from Build 18.

Fixed in build20-rc1