Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#3157 closed defect (fixed)

[PATCH] Coordination of attack with an AI

Reported by: mimo Owned by: mimo
Priority: Nice to Have Milestone: Alpha 19
Component: AI Keywords:
Cc: Patch:

Description

When allied with an AI, the player should be able to coordinate attacks with its AI ally. In the attached patch, this is done through an additional button in the diplomacy window which sends a request to the allied AIs to attack a given player. Most of the patch is internal petra changes, but feedback on the gui part (+ corresponding changes in AIInterface.js and Commands.js) would be welcomed.

Attachments (2)

attackRequest.diff (11.0 KB ) - added by mimo 9 years ago.
attackRequest-v2.diff (15.1 KB ) - added by mimo 9 years ago.

Download all attachments as: .zip

Change History (9)

by mimo, 9 years ago

Attachment: attackRequest.diff added

comment:1 by leper, 9 years ago

Don't we have the isAI data in the GUI? We could just use that to detect if the player is an AI (or just send a chat message in any case) (see Sander's comments on IRC).

"Sorry, I have not enough soldiers to attack %(name)s right now, but my next attack will target him." would be better as "Sorry, I do not have enough soldiers to attack %(name)s currently, but my next attack will target %(name)."

"[...], as I have another [...]"

if (!foo)
    return;
oneliner();
return;

is a bit ugly, just do if(foo)oneliner();

Also quite some let usage you have there. I thought we decided to only switch to that when performance of it in SpiderMonkey is improved (at least where perf matters).

by mimo, 9 years ago

Attachment: attackRequest-v2.diff added

comment:2 by mimo, 9 years ago

I've just uploaded a new version of the patch in which a chat message is also send to non AI players. In addition, I've added some formatting of the ai chat messages (when mentionning a player, its name appears now in its player color).

comment:3 by sanderd17, 9 years ago

Looks very good. The only thing I wonder is if the player name can be translated that way.

I guess it will try to translate strings like _player_2, and thus always fail. So if you want it translated, maybe look for a different way. But I'm not sure if player names are currently translated.

EDIT: Also, you can reuse the "agressive" icon for now, but it would be better if you copy the icon, and reference it with its own name. That way, it will be easier for artists to replace the icon with something more suited.

Last edited 9 years ago by sanderd17 (previous) (diff)

comment:4 by mimo, 9 years ago

Thanks for the comments. I agree for the icon, I'll copy it when committing the patch. For the translation of the player name, the name displayed will be the same as the name displayed in front of chat messages. So that will be consistant.

comment:5 by leper, 9 years ago

/allies would be better than /team, wouldn't it? The tooltip in for the button isn't translated and could be worded better.

Does that code assume no player will name itself _player_4? We should just pass the parameter as something else than "name" and do the matching to a playername somewhere else.

comment:6 by mimo, 9 years ago

Owner: set to mimo
Resolution: fixed
Status: newclosed

In 16533:

new button in diplomacy to request an attack to allies, works also for AIs, fix #3157

comment:7 by mimo, 9 years ago

Keywords: patch review removed
Milestone: BacklogAlpha 19

Thanks sanderd17 and leper for the reviews.

Note: See TracTickets for help on using tickets.