[PATCH] Let the AI inform allied players when advancing to the next phase

[Stan]

It would be nice to be aware if the AI goes from one phase to another by having a word message just like the one that tells you have opened the developper overlay.

[leper]

Having this for all players sounds like information that should be gathered by scouting. It would be nice to have an allied AI inform you of advancing though.

[maveric]

For I am newbie here, I have a question about implementation. I guess it can be done in ResearchTechnology function in TechnologyManager.js or with triggers and ResearchFinished event precisely (for now I got only these two ways), right? Trigger way seems better for me, but I couldnt find while the event handlers are defined. So, what is the better way to do this and also how can the message be displayed on screen? And the last one. Should AI inform when the research is done or when it is started?

[leper]

As the AI should do this it belongs in the AI code (simulation/ai/petra/chatHelper.js for the message code, researchManager.js in the same directory and possibly something else (see below)), and some more that sends the messages to allies. I guess having two messages (one when planning/starting research; one when done) would be nice.

CCing mimo to possibly point you to other useful/relevant files/functions.

[maveric]

thanks a lot

[mimo]

You have two ways to proceed:

1) either doing everything in the AI code: when a town or city phase research starts, the researchManager calls the functions OnTownPhase and OnCityPhase (from petra/headquarters.js). So you just have to add your starting chat messages there. To know when the research is finished, I guess the simplest is to add a this.currentPhase in petra/headquarters.js which would be updated every AI turn using the gameState.currentPhase() function and you would have to send your messages when it changes.

2) the other solution would be to replace the ResearchFinished trigger message in TechnologyManager by a standard ResearchFinished message which would be listened by the trigger, the EntityLimit and by AIInterface, and add also a ResearchStarted message. Then the AI would listen to these messages (see examples in the function checkEvents of petra/headquarters.js) and react accordingly. That may be overkill for this simple task, but could be useful in the future if the AI needs to be warned of other technologies.

Then, to send your warning messages to allies, you have plenty of examples in petra/chatHelper.js

[mimo]

The patch looks good to me, except passing the number of the phase in the message. I would rather put the full name (town or city instead of 2 or 3). Also you should use a parameter inside the markForTranslation instead of the string concatenation.

Then last point: is it better to send the notification to /team or /allies ? (but that's a question for most of the other notifications in chatHelpers).

[maveric]

Okay, that sounds reasonable. And whats the difference between /team and /allies?

[maveric]

Also I guess calling GameState.currentState() in headquaters.js can be replaced by this.currentState as it updates every turn, right?

[mimo]

Yes, all calls to gamestate.currentState() could be replaced by this.currentState, but i think the difference in performance would be negligible, so not worth.

players in a team can not attack each other, while otherwise, alliance can be changed via the diplomacy window and you can thus attack your (ex-)allies. But diplomacy is still in a very rudimentary state.

[leper]

Regarding /allies and /team: Team is something set once and unchangeable, while allies are changeable using diplomacy, so /allies would be the better choice.

You can just use "phase" no need for "_phase_", the translation of "town" and "city" will likely not work nicely that way. Using the generic name from the tech template (eg: phase_city.json​) would be better and keep the whole thing consistent.

Also 'if (' instead of 'if('. (See the coding conventions)

[mimo]

Looking back at the code, this.currentPhase should be serialized (see the Serialize function).

In addition, I just realized that there was already some logic to know when the new phase was reached, using this.phaseStarted, and it was used to update the territory after a new phase. We should keep only one of the two ways (may-be the new one looks cleaner).

[maveric]

About template. Instead of passing, for example, "town" to message function I should pass something like

gameState.playerData.researchedTechs["phase_town"].genericName

Did I get it right? If I did, than how do I get name in case when tech havent researched yet?

[maveric]

The last one, I think, is a little bit better.

[mimo]

I'm on leave now, so will test it and commit when back home, possibly friday evening. A few additional comment: lines 1736 and 1741 can be moved out of the if, and thus some parenthesis can be removed

[maveric]

Fixed that

[mimo]

In 16573:

petra: inform allied players when advancing to the next phase, patch by maveric, fixes #2757

[mimo]

Thanks for the patch.