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

Garrison fixes #115

Merged
merged 11 commits into from Aug 26, 2015
Merged

Conversation

bwrsandman
Copy link
Contributor

Fix tooltip for in garrison creatures
Show tooltip after moving hero to garrison
Disable creature split button for garrisoned ally
Refactor and simplify Left Click in Garrison
Fix selection as default when action is made
Implement moving all but last creature mechanic
Various documentation addition

/// Type of Garrison for slot (up or down)
enum GarrisonType
{
up=0, ///< 0 - up garrison (Garrisoned)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use capital letters for Enum items

@DjWarmonger
Copy link
Member

Implement moving all but last creature mechanic
This one has been present since forever, unless I'm missing something?

@alexvins
Copy link
Member

This one has been present since forever, unless I'm missing something?

As for now, engine prevents moving last stack (whole last stack, split for last stack works perfectly) on server side. In this PR it will move all except one creature.

@bwrsandman
Copy link
Contributor Author

This one has been present since forever, unless I'm missing something?

Prior, you can't move the last stack of a visiting hero into garrison (or from a garrisoned hero to another hero) because the server stops you with an error.

In vanilla, if you do this, it will move all the creatures except for one to the other slot.

This PR does this and the server no longer complains about something fishy.

@bwrsandman
Copy link
Contributor Author

Fixed the enum style: uppercase values and use the 'E' prefix

Restructure large nested if statements into more shallow blocks
Factor out contents of if statements to functions for better readability
    * viewInfo
    * hightlightOrDropArtifact
    * split
@bwrsandman bwrsandman force-pushed the creature-garisson-tooltip branch 2 times, most recently from ead38ce to 89fb4aa Compare August 25, 2015 13:51
const CArmedInstance * selectedObj = owner->armedObjs[selection->upg];
if (!creature && selectedObj->stacksCount() == 1)
LOCPLINT->cb->splitStack(
selectedObj,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if splitting arguements like that helps. Before rewrite these lines contained complex conditions, but not they are simple and just use up vertical space.

@bwrsandman
Copy link
Contributor Author

@DjWarmonger done

DjWarmonger pushed a commit that referenced this pull request Aug 26, 2015
@DjWarmonger DjWarmonger merged commit ee08e6b into vcmi:develop Aug 26, 2015
ArseniyShestakov added a commit that referenced this pull request Sep 23, 2015
After 8cad239 there was several issues remaining:
* Last creature would remain even for town garrisons.
* Impossibility to move last stack even within same garrison.
* Impossibility of partially merge of last stack into other hero stack.
All fixed. However last stack merge to other hero will now make split window appear instead of auto merge.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants