adding hero button to wagon and other recruit objects

[shprotru]

fheroes2 brings enhancement to dismiss troops while someone is recruiting, but it doesn't work everywhere.

before:

after:

related issue: #5249

[github-actions]

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

[zenseii]

Hi @shprotru!

The warnings have to do with the fact that you need to use the fheroes2::Text specific functions/methods now that you are using that instead of class Text.

Take this example from the warnings:

text.Blit( pos.x + 320 - text.w() / 2, pos.y + 64 );

fheroes2::Text doesn't have a Blit member function so you need to call its respective draw function. In addition that function takes the additional parameter Image &. You'll also notice that fheroes2::Text has its width function named as width instead of just w. The same goes for h and height. Last but not least, the new fheroes2::Text shifts the text up by 2 pixels compared to the old Text which means we need to move down two pixels and therefore add 2 pixels in the Y position parameter. In the above example this gives us this change:

text.draw( pos.x + 320 - text.width() / 2, pos.y + 66, display );

[zenseii]

Hi @shprotru !

I just tested this and would it be possible to add this for dwellings that give you creatures for free too? Like the Peasant hut, fearie trees etc. ?

Also we have a slightly similar issue regarding creatures that join you due to alliances like dwarves and dragons. This would require a bit more work though since some extra dialog logic needs to be added if you read the discussion in that issue #5249 .

If you want to go ahead and add that too but need some clarification then don't hesitate to ask either me or the other members that contributed to that issue's discussion.

[github-actions]

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

[zenseii]

Hi @shprotru ! I left one comment. Please take a look when you should have the time 😊

[zenseii]

Right now there is no way to make this message about ranks being full appear since you cannot press "Yes":
.

I suggest to remove this message and make the else statement further down the default action for this dialog. What do you think?

[zenseii]

I didn't ask if you could revert this change, but the logic needs to reflect that the message will not appear anymore.
The code I'm asking for would be something like this.

if ( Dialog::YES == Dialog::ArmyJoinFree( title, message, troop, hero ) ) {
                tile.MonsterSetCount( 0 );
                hero.GetArmy().JoinTroop( troop );
                hero.MovePointsScaleFixed();
                Interface::Basic::Get().GetStatusWindow().SetRedraw();
}

What do you think?

[ihhub]

Hi @shprotru , before proceeding with the review could you please explain why we need this feature if a player can easily hit a Space button one more time? I think we have similar discussion here.

Enhancements are good but we need to be cautious about them and not overuse them. I would say that in cases when a hero cannot repeat the same action they might help to improve user experience.

Let's do this way: we listen the opinion of @oleg-derevenetz , @Branikolog , @oleg-derevenetz , @zenseii and @idshibanov about this enhancement and then decide what we want to do.

Remember: look at this enhancement with closed eyes on similar buttons in other dialogs and question whether it is useful to have it.

[zenseii]

Hi @ everyone in this PR 😅

I've been thinking a bit about this PR after @oleg-derevenetz's point that he made in another issue about adding the marketplace button. To summarize I believe we should only introduce such buttons to the dialogs if these dialogs cannot be exited and reentered again. Existing examples of these are level up, creature joining/"followers" etc.

That's why when it comes to the dialogs covered by this PR, which are the dwelling dialogs, I do not think we need the added button because the player can simply exit and reenter with the space bar after having made necessary changes (or just make them before entering).

[oleg-derevenetz]

Let's do this way: we listen the opinion of @oleg-derevenetz , @Branikolog , @oleg-derevenetz , @zenseii and @idshibanov about this enhancement and then decide what we want to do.

I believe that if the same action can be repeated later using spacebar, then there is no need to add a shortcut for it - it just clutters up the UI.

[Branikolog]

Hi, everyone.

In my personal point of view, we can add here this button, since this window 99% is the same to common joining window:

We can modify text here, as @zenseii proposed.
We can use a part of text from the next window, appearing after player accepts. Something like: "Your ranks are full, so you cannot join new troops" and add it under the question. So, the next window (You are unable to recruit at this time, your ranks are full.`) will never appear anymore.

[AAS]

Guys, could you please just merge it as is?
It is 3+ years old now :(