-
-
Notifications
You must be signed in to change notification settings - Fork 995
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
recruit dialog: Gray out units that will not be able to be recruited #4444
recruit dialog: Gray out units that will not be able to be recruited #4444
Conversation
Testing An Orcish Incursion S5 as mentioned in #4443 and your branch name, this seems to do what is intended. |
Thanks for the review! |
src/gui/dialogs/unit_recruit.hpp
Outdated
@@ -27,11 +27,13 @@ namespace dialogs | |||
class unit_recruit : public modal_dialog | |||
{ | |||
public: | |||
unit_recruit(std::vector<const unit_type*>& recruit_list, team& team); | |||
/// @param recruit_list maps unit typs to strings. The strings are "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no param called recruit_list
.
Should it map to t_string instead of string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no param called
recruit_list
.
Fixed
Should it map to t_string instead of string?
What difference would that make?
Thanks for the review!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel t_string makes it obvious that it's a string for showing to the user, because it's explicitly translatable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Anything else before merging?
424d446
to
bc4613d
Compare
Reproducible in WoV S3 by recruiting a unit and trying to repeat the recruit. Previously it failed silently, now it shows a message.
bc4613d
to
129e53b
Compare
(cherry picked from commit 70a2046)
(cherry picked from commit 70a2046)
(cherry picked from commit 70a2046)
Fixes #4443. This doesn't add or remove any units to the recruit dialog, it simply makes units that will fail to be recruited be listed in gray.
See the individual commits for details.