Page MenuHomeWildfire Games

gamesetup panel should indicate the AI level without having to open the AI-config panel
ClosedPublic

Authored by mimo on Nov 25 2017, 4:14 PM.

Details

Summary

We should have a way to indicate the chosen AI level without having to open the AI-config panel which is quite cumbersome when lot of Ais.
A possibility would be to change the background of the gear icon depending on the current AI level (if somebody has nice proposition about it), but here i've used a more basic way by adding the level in the AI_config tooltip such that we can know the level by only hovering the mouse over the gear icon.

Test Plan

Check if you agree with the changes

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mimo created this revision.Nov 25 2017, 4:14 PM
bb added a subscriber: bb.Nov 25 2017, 4:22 PM

Shouldn't we also mention the ai name? (as that is choosable too) so we get something like Configure AI settings (current: {name}, {level})

elexis requested changes to this revision.Nov 25 2017, 4:25 PM
elexis added a subscriber: elexis.

Broken translation.
(Ah right, period at the end of tooltips, Polakrity wanted those)

I would also add the Petra name, even if we only have one AI currently.
(We do that in the lobby and replay menu (gamedescription.js) too for instance.)

You likely want these:
translateAIName(playerData.AI)
translateAIDifficulty(playerData.AIDiff)

Maybe "Configure AI settings (Very Hard Petra AI)."?

This revision now requires changes to proceed.Nov 25 2017, 4:25 PM
Vulcan added a subscriber: Vulcan.Nov 25 2017, 4:47 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Executing section Default...
Executing section Source...
Executing section JS...
mimo added a comment.Nov 25 2017, 4:51 PM

Agreed about the translation (i forgot it), but is the name really needed? it is already displayed just on the left of the gear icon that you have to hover. I would rather not overcrowd this tooltip and keep room to add additional info if we ever have it (like ai agressivity and so on).

In D1066#42209, @mimo wrote:

name really needed? it is already displayed just on the left

Oh, you are right.

Perhaps we could also add the difficulty to the tooltip of the dropdown. (Adding it to the label won't work for 1024x768, otherwise I would have suggested that).

If there is going to be more than one AI property, then adding these with newlines shouldn't be a problem.

mimo added a comment.Nov 26 2017, 12:44 PM

In fact, as we don't have much to display in that tooltip, i've put the name in it. We can always remove it later when we have more properties to display.
And translation is fixed.

In D1066#42212, @elexis wrote:

Perhaps we could also add the difficulty to the tooltip of the dropdown. (Adding it to the label won't work for 1024x768, otherwise I would have suggested that).

We currently don't have any tooltip for it. I'd be fine with adding the same tooltip for the dropdown, but i'm afraid it must be misleading as the dropdown does not allow to change the difficulty.

If there is going to be more than one AI property, then adding these with newlines shouldn't be a problem.

The way this tooltip is displayed only allows for 2 lines i think (but could be changed when we will need it)

mimo updated this revision to Diff 4387.Nov 26 2017, 12:45 PM

update patch

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Executing section Default...
Executing section Source...
Executing section JS...
elexis accepted this revision.Nov 26 2017, 1:10 PM
  • Better end the period too for consistency before Polakrity notices ;-) Oh thats not committed yet D594.
  • Should we use the same order used in replay menu, load game dialog and lobby? (That is Very Hard Petra Bot, all loaded from gamedescription.js)
  • Not fully sure about "Current:".
  • Perhaps level -> difficulty.

That'd be Configure AI: %(difficulty)s %(name)s.

Your choice.

This revision is now accepted and ready to land.Nov 26 2017, 1:10 PM
elexis added inline comments.Nov 26 2017, 1:11 PM
binaries/data/mods/public/gui/gamesetup/gamesetup.js
961 ↗(On Diff #4387)

-1 tab for the three lines judging from phabriator

mimo added a comment.Nov 26 2017, 3:39 PM
In D1066#42441, @elexis wrote:
  • Better end the period too for consistency before Polakrity notices ;-) Oh thats not committed yet D594.
  • Should we use the same order used in replay menu, load game dialog and lobby? (That is Very Hard Petra Bot, all loaded from gamedescription.js)

I'd keep the current way (with a separation as "Petra Bot - Very Hard") otherwise it may look strange in some languages as french where "Moyen Petra Bot" is weird.

  • Not fully sure about "Current:".
  • Perhaps level -> difficulty.

ok

That'd be Configure AI: %(difficulty)s %(name)s.

Your choice.

This revision was automatically updated to reflect the committed changes.
In D1066#42493, @mimo wrote:

it may look strange in some languages as french where "Moyen Petra Bot" is weird.

For that reason we have the sprintf, in english it can be 1 2 and in french 2 1 and somewhere else foo 1 bar 2

elexis added inline comments.Nov 27 2017, 5:12 PM
ps/trunk/binaries/data/mods/public/gui/gamesetup/gamesetup.js
953

We could rename playerConfig to aiConfig to better describe what that button is about.
Could also leave it as is to inspire the addition of more player-dependent config options there, such as selecting the regicide hero.
In that case we could maximize the inspiration rate by renaming the aiconfig GUI page, directory, filenames to playerconfig.