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

Add all attacking units at battle site for retreat consideration #1827

Merged
merged 1 commit into from
Jun 8, 2017

Conversation

ron-murhammer
Copy link
Member

Fixes #442.

Need to consider units such as infra that were removed from fighting in the battle when retreating.

@DanVanAtta
Copy link
Member

Looks good. I tested this on BW v3, Feudal Japan and Domination 1914 No mans land.
Feudal Japan is where we saw the problem, horses would be given away if you attacked with them and retreated. That no longer happens with this branch, horses retreat with the attacking army.

Nice work finding the fix for this @ron-murhammer , that ticket (#442) has been open for some time.

@DanVanAtta DanVanAtta merged commit 49095de into master Jun 8, 2017
@DanVanAtta DanVanAtta deleted the Fix_Retreating_For_Attacking_Infra branch June 8, 2017 17:49
@panguitch
Copy link

Yes, thanks @ron-murhammer, just tested this for Greyhawk Wars and looks good. This will allow me to delete the "hack" support attachment that enabled bowmen to retreat. And when they retreat if you brought troops from multiple territories the bowmen can choose which territory to retreat to, not just the one they came from--which is as it should be and I'm glad to see it work that way since I could see that nuance getting lost.

I do notice that it no longer shows AA units in the battle window the way it used to. Not really a problem but it's a little weird since it shows their rolls against attacking aircraft but doesn't show them. Not sure if this was part of the fix or a side effect.

@ron-murhammer
Copy link
Member Author

@panguitch This change shouldn't have any effect on what units display in the battle window since all it does is add infra units to the possible units to consider for retreat. If you test a version right before this change then you should see the same issue. Feel free to open an issue for it and what version you've tested that have or don't have the problem.

@panguitch
Copy link

Nevermind. Can't recreate this problem in the release where I first saw it, so I must have been imagining things to begin with!

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.

3 participants