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

Fix 1989 cavaliers -> champions in stables #183

Merged
merged 2 commits into from Feb 2, 2016

Conversation

@vmarkovtsev
Copy link
Contributor

vmarkovtsev commented Jan 30, 2016

@ArseniyShestakov

This comment has been minimized.

Copy link
Member

ArseniyShestakov commented Jan 31, 2016

Small comment about H3 behaviour: InfoWindow with text must appear as well even if it 2nd+ visit of stables and you just get an upgrade without any movement bonus.

Currently you just showing text that object already visited even if upgrade been done.

@vmarkovtsev

This comment has been minimized.

Copy link
Contributor Author

vmarkovtsev commented Jan 31, 2016

Damn, it will be much harder to display 2 InfoWindows - it is hardcoded to be a single window per CGBonusingObject, so there are going to be quite a few ugly hacks.
Besides, I need to find out the corresponding text ID. Can you post a screenshot of that window from OH3 please?

@ArseniyShestakov

This comment has been minimized.

Copy link
Member

ArseniyShestakov commented Jan 31, 2016

It's uses same text ID in H3 so there is no special message that only include information about upgrade.

As you note there was object refactoring, but unfortunately Ivan didn't find way to make objects fully configurable back then. So if adding infowindow on upgrade will require dirty hacks in code then just leave it as and post bug about unfinished UI feature on bug tracker.

@vmarkovtsev

This comment has been minimized.

Copy link
Contributor Author

vmarkovtsev commented Jan 31, 2016

Am I right that if we visit stables for the first time and have cavaliers, the message says "... blah-blah you got champions", and if we visit stables for the second time and have cavaliers, exactly the same message must be shown (not the usual "you have already visited this")? Or should we concatenate "you have already visited this" with "you got champions" text and picture? The third variant is first display "you have already visited this" and then display "... blah-blah you got champions" (the bad one - hard to implement).

@ArseniyShestakov

This comment has been minimized.

Copy link
Member

ArseniyShestakov commented Jan 31, 2016

Just show exactly same message as it's what H3 do.

@vmarkovtsev

This comment has been minimized.

Copy link
Contributor Author

vmarkovtsev commented Jan 31, 2016

Got it, it's easy then

@vmarkovtsev vmarkovtsev force-pushed the vmarkovtsev:issue/1989 branch from 9c09efa to 88bc219 Feb 1, 2016
@vmarkovtsev

This comment has been minimized.

Copy link
Contributor Author

vmarkovtsev commented Feb 1, 2016

Done

ArseniyShestakov added a commit that referenced this pull request Feb 2, 2016
Fix 1989 cavaliers -> champions in stables
@ArseniyShestakov ArseniyShestakov merged commit eabdc0f into vcmi:develop Feb 2, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.