AIs don't have aura information

[wraitii]

Since the introduction of auras, there is a new way to distort stats. AIs can't know which entities are affected.

We probably need something akin to how we handle technologies, with "GetTechModifications()" that's passed to AIs. This is kind of ugly as it could be a lot of information though. So perhaps devise something cleverer using messages? Change the system more fundamentally?

I'm putting this as "Must Have" because it has weird effects: for example if there is an Aura that decreases HP, the AI will notice the decreased HP but won't notice that the max HP is also decreased, and will think the unit/building needs to be healed/repaired. This already happens with Gauls (which for some reason have a CC with base HP 2400 and not 3000), but that's probably because I fail to apply technologies properly.

[sanderd17]

Auras and technologies send the same ValueModification message. So if you just want to know what changed, it's enough to listen to that message for both purposes (and any following modifiers).

[wraitii]

The patch attached above fixes the issues forever for auras and technologies, without requiring any upkeep on the AI part (currently you had to apply manually "GetTechModifiedProperty" which was "I forgot"-prone). I almost feel like ​http://xkcd.com/664/, only there's a catch.

This patch only works if the aura/tech modifications schema keeps the same format it has now:
-each "value" only affects one stat (so, say, "Health/Max" won't modifiy Health->Max and also Armour->Hack. This example is dumb but demonstrative).
-They strictly follow the names and hierarchical guidelines of templates. Ie to modify the max health, you call "Health/Max". If you were to change sounds, say "select", you'd name the value "Sound/SoundGroups/select".

If those two requirements are met (they are now and I think they're good so we should keep them) then this patch will work without any issues, supporting auras and techs, would be fairly fast (there are slight rooms for improvements, too, it's not the definitely committable version), without upkeep for the AI (unless we decide to add technologies that allow to build more buildings, or add preferred classes, then there'll be like a tiny change). This patch even makes the common-api entity.js more readable and reduces the amount of data passed each turn to the AI.
It's basically perfect except for the rigidity above (which is imo good), and for the fact that it adds a basically useless message to auras (AI_AuraChange) because if I make AIProxy listen to ValueModification, then for each tech I'll get it needlessly thousands of times which I do not like. This is up for debate.

[sanderd17]

I don't really understand why you need the AI_AuraChanged message. Maybe it's better to format the message this way" {"ents": [id1, id2, id3], "component": "Health", "valueNames": Health/Max}.

This way, the TechnologyManager could just query all the entities of the player (I can look into that if you want), and put it in a list. While the AuraManager would add it to only one entity. This would mean adding the message to AIProxy isn't needed, as it can be sorted out very efficiently in AIInterface.

With this message type, you wouldn't need to query the player ID. But normally, querying the player id from a player component is done by first querying cmpPlayer on that entity (Engine.QueryInterface(this.entity, IID_Player)), and then, getting the id from the cmpPlayer (cmpPlayer.GetPlayerId()).

Also, I think caching components (as you did in AIProxy) will break saved games. I'm not sure if it's also the case for system components, but for safety, it shouldn't be cached.

About the schema, you can see all implemented possible modifications here: ​http://trac.wildfiregames.com/wiki/TechModifications . So you see there are some proposals that don't follow the schema (like Armour/All), some could be dropped, but I wonder about others (like Technology/ResearchTime). The reason they're currently not implemented is because it's ugly code. But I wonder how we could replace them.

And as last remark, technologies can also cause other values to change, values that are influenced by directly changed values. F.e. if the Health/Max changes, the actual number of hitpoints is also changed in proportion. In border cases, this might even lead to the death of a unit. I don't know if there are other, similar cases, but the code should be checked to allow this.

[sanderd17]

This is how you could get a list of entities owned by the player in the TechnologyManager:

	var cmpPlayer = Engine.QueryInterface(this.entity, IID_Player);
	if (!cmpPlayer || !cmpPlayer.GetPlayerID())
		return;
	var cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager);
	var ents = cmpRangeManager.GetEntitiesByPlayer(cmpPlayer.GetPlayerID());

[wraitii]

Thanks for looking at this… I added the new message because I hadn't looked at the possibility of querying entities from the AIInterface. Basically if I wanted not to have redundant info (player and entities), so auras had to use something else. The thing is I don't want the tech manager to query all entities of a player since I modify templates (it's better style, better for caching too). So I just need to know which player researched that tech.

I'm not sure AIProxy is serialized but I'll look into it. If it's not I'd like to keep it because it's probably going to be faster.

I see about the not following the schema stuff… For Armour/All, this seems extremely redundant to me, we could just add the three kinds (I know it's slightly longer, but not thaaat much and it'll reduce the amount of specific code everywhere). Imo this would be good style even in the JS components since we would have a direct correspondance between tech modifications and template data. About stuff like Technology/ResearchTime: we could either change this to have a basic Rate in ProductionQueue for tech/units/biuldings, and then change that, or make it change technologies data for this player. If that's absolutely not acceptable I can special-case some values, but I'd really rather keep it at a minimum.

(also I've got to say I think the Value Modification code is generally weird, prone to errors since you have to remember to apply modifications, so steamlining it that way seems good to me).

[wraitii]

Updated patch for betterness:
-There are now two messages, "MT_TemplateModification" and "MT_ValueModification". In the C++, those are interchangeable (their memory signatures are identical so it's safe to cast both to the same component), in JS I added a call to this.OnValueModification.
-Auras now choose the proper message and template changes send the good message.
-AI still works perfectly (I've also added a caching for unmodified value)
-a few other unrelated changes are in the patch.

Per discussion with sanderd above, I could special case Technology changes, and I do think that we should keep the "One value, One effect" paradigm. Stuff like "Armour/All" are clutter form a programming standpoint. Tools and GUIs should be created instead to affect all values by writing three different effects imo.

Your last concern which I hadn't addressed (the "Last Remark") is irrelevant, the AI would get other messages if an entity dies.

[sanderd17]

I've attached my proposal. There are still two messages, but regular entity components only need to care about one. The TemplateModification message is only send once to the system entity.

The advantage is that components don't get extra dirty (Philip even thought that letting them listen to one message is dirty), while the AI still has the possibility to differentiate.

Also, not all technology modifications act on numbers (and not all numbers are integer too). So you probably shouldn't convert "item" to a number, and certainly not round it. One example of a string-technology is the athenian wall bonus. And the Actor tech (also string-based) is in the pipeline.

Also, the caching of this.cmpAIInterface really needs to be tested. F.e. I found it broke starting the Atlas simulation on skirmish maps (and that's the first thing I tried).

[wraitii]

In 14588:

AIs now properly receive aura and technology updates. Fixes #2377, Refs #1520 . Consequently reimplement repairing for AIs.
Fix a few style issues and a bug with the gatherer count.
Still need to fix the entity.js file to handle properly some things as this uses raw templates values.
Cache the AIinterface in AIProxy.js, please report if this works properly.