-
Notifications
You must be signed in to change notification settings - Fork 170
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
Issue #416: AtB turret generation and base attack scenario fix #417
Issue #416: AtB turret generation and base attack scenario fix #417
Conversation
AtB used to generate turrets using a crude hard-coded method based tables provided by Makinus, who wrote the AtB rules. I've requested code review by kipstafoo since he has be reworking the AtB scenario generation code. |
So, two comments:
|
The AtB rules state that the "base force" during a base attack gets 8 civilian units and 6 turrets. So, I'm following the text. It doesn't really matter either way, as the pilots are generated at the opfor skill and tech level. I don't think there is a reason to keep the randomGunEmplacement() method around. I'll get rid of it. |
I think I'd like to keep the randomGunEmplacement() method around but commented out for historical purposes. |
Ok, I'm good with it being part of the civs. Like I said, it really doesn't matter as long as the turrets target the right 'enemy'. For the randomGunEmplacement(), let's nuke it please instead of commenting it out. Git maintains history for us, so no sense in muddying the code with unreachable/unused code. I'm a stickler for clean code. ;) And that you for removing the unused imports earlier... that drives me utterly bonkers. |
No idea why the diff looks so weird, but commented out code removed. |
Using my amazing powers of prediction (wooooooooo), I see a future where Bloodwolf has created more turret RATs, rendering the original fix obsolete. The new fix accounts for the possibility of many different combinations of turret RATs, just as long as they follow the pattern of "Turret YYYY Q", where YYYY is a year and Q is the single-letter quality level. |
K, Yeah when I get around to grinding through the clan turret RATs they will all be in the same format. |
A somewhat crude implementation of a turret generation mechanism for base attack scenarios (and now usable for any other scenarios in the future which may want turrets). I'm not aware of a way that turrets can be generated from the standard RAT generator, so I chose to imitate the mechanism used to add civilian units instead.
Notes: the AtB rules state that the civilian unit count on base attack missions should be 8, so 8 it is.
Also: This fix is dependent on pull request #415 , as it uses the updated unitGenerator.getUnitRatingName() function.