Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unit_recall: Show unrecallable units grayed out. #4362

Merged
merged 10 commits into from Sep 26, 2019

Conversation

@jostephd
Copy link
Member

jostephd commented Sep 18, 2019

Part of issue #1282.

Looks like this:

2019-09-18-154113_1024x768_scrot

  • add a tooltip with the reason the unit is grayed out
test case
diff --git a/data/campaigns/An_Orcish_Incursion/scenarios/01_Defend_the_Forest.cfg b/data/campaigns/An_Orcish_Incursion/scenarios/01_Defend_the_Forest.cfg
index 53ea5f5be8e..596689ec975 100644
--- a/data/campaigns/An_Orcish_Incursion/scenarios/01_Defend_the_Forest.cfg
+++ b/data/campaigns/An_Orcish_Incursion/scenarios/01_Defend_the_Forest.cfg
@@ -50,7 +50,7 @@
     [side]
         side=1
         controller=human
-        {GOLD 200 150 100}
+        gold=20
         income=0
         team_name=Elves
         user_team_name= _ "Elves"
@@ -61,6 +61,10 @@
 
         facing=nw
 
+        {UNIT 1 Mage recall recall ()}
+        {UNIT 1 Mage recall recall (recall_cost=25)}
+        {UNIT 1 Peasant recall recall ()}
+
         [unit]
             side=1
             type=Elvish Rider
@jostephd jostephd self-assigned this Sep 18, 2019
@soliton-

This comment has been minimized.

Copy link
Member

soliton- commented Sep 18, 2019

Perhaps label should still be set in the cfg file and C++ would just conditionally append ~GS?

@jostephd

This comment has been minimized.

Copy link
Member Author

jostephd commented Sep 18, 2019

@soliton- I wanted to do that, actually, but couldn't figure out how. Would that be listbox.get_row_grid(listbox.get_item_count() - 1).find("gold_icon", false)?

@soliton-

This comment has been minimized.

Copy link
Member

soliton- commented Sep 18, 2019

Sorry don't know much about GUI2. I just assumed you have access to the cfg file via a config object somewhere.

@gfgtdf

This comment has been minimized.

Copy link
Contributor

gfgtdf commented Sep 18, 2019

From.looking at the code this only checks for costs and.nit for [filter_recall] of the leader?

@gfgtdf

This comment has been minimized.

Copy link
Contributor

gfgtdf commented Sep 18, 2019

I wonder whether it should also check for free tiles.

@jostephd

This comment has been minimized.

Copy link
Member Author

jostephd commented Sep 18, 2019

From.looking at the code this only checks for costs and.nit for [filter_recall] of the leader?

Units that don't match the [filter_recall] do not appear in the dialog in the first place. (The list of units passed to unit_recall's constructor doesn't include them.)

I wonder whether it should also check for free tiles.

Maybe... 🤔 but I don't consider this a blocker.

@jostephd jostephd force-pushed the jostephd:gray-out-unrecallable-expensive branch from d34060a to 3e7005d Sep 18, 2019
@jostephd

This comment has been minimized.

Copy link
Member Author

jostephd commented Sep 18, 2019

@soliton- That worked, so pushed.

jostephd added 2 commits Sep 18, 2019
Copied from unit_recruit.
@jostephd

This comment has been minimized.

Copy link
Member Author

jostephd commented Sep 18, 2019

Recruit dialog updated too:

2019-09-18-190446_509x346_scrot

@gfgtdf

This comment has been minimized.

Copy link
Contributor

gfgtdf commented Sep 18, 2019

Wasn't the gold already shown in red before? I currently tend to say I like the previous way better because it makes clearer why exactly you cannot recruit the unit.

@jostephd

This comment has been minimized.

Copy link
Member Author

jostephd commented Sep 18, 2019

Wasn't the gold already shown in red before?

Old behavior: the gold is shown in red if the unit has a recall_cost higher than the team's recall_cost, regardless of how much gold the team has in the bank.

New behavior: if the unit's recall cost is higher than the team's gold, the line is gray. Otherwise, same as old behavior.

…hiteboard.

This makes it so if you try to recall and your recall list is empty when
accounting for the whiteboard, you get the "There are no troops" error
rather than the "You currently can't recall" error below it.
@jostephd jostephd merged commit c9ba3ee into wesnoth:master Sep 26, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jostephd added a commit that referenced this pull request Sep 26, 2019
@jostephd jostephd referenced this pull request Oct 18, 2019
Wedge009 added a commit to Wedge009/wesnoth that referenced this pull request Nov 12, 2019
(cherry picked from commit 28d1db5)
Wedge009 added a commit to Wedge009/wesnoth that referenced this pull request Nov 13, 2019
(cherry picked from commit 28d1db5)
Wedge009 added a commit that referenced this pull request Nov 13, 2019
(cherry picked from commit 28d1db5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.