-
Notifications
You must be signed in to change notification settings - Fork 293
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
Princess - improved AMS handling #5617
Princess - improved AMS handling #5617
Conversation
// Set default to on/automatic and test to see if it should be off or manual instead | ||
EquipmentMode newAMSMode = EquipmentMode.getMode(Weapon.MODE_AMS_ON); | ||
|
||
boolean isOverheating = (curEntity instanceof Mech) && (curEntity.getHeat() >= 14); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This heat threshold number should either be:
A) a constant declared at the top of the file, or
B) a value passed in to the function / read from some member, or
C) a reference to any other existing heat limit variable already used by Princess
so we don't have conflicting behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, ideally it would tie into the existing code that handles heat. The big problem is the existing code is during the movement/weapons fire phases, and like the stealth armor control this is happening during the end phase (which is why it can have a look at the heat level rather than trying to predict it). So all that fire control objects/information is no longer valid or available.
I'll put in a constant for now. I would like something a little more... dynamic, I guess, for what constitutes 'overheating'. But keeping the complexity down seems to be the better option to start with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, ideally it would tie into the existing code that handles heat. The big problem is the existing code is during the movement/weapons fire phases, and like the stealth armor control this is happening during the end phase (which is why it can have a look at the heat level rather than trying to predict it). So all that fire control objects/information is no longer valid or available.
I'll put in a constant for now. I would like something a little more... dynamic, I guess, for what constitutes 'overheating'. But keeping the complexity down seems to be the better option to start with.
I might be misremembering, but I think there's a heat limit value that's used in the "with/without overheating" firing plan code that might be usable. IIRC we calculate something based on aggression level; if it's not immediately available, that might be nice to pull out to a method or instance member.
|
||
// Turn off laser AMS to help with overheating problems | ||
if (curAMS.getType().hasFlag(WeaponType.F_ENERGY)) { | ||
if (isOverheating) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above. Also, while we apply stricter overheating checks to ASF than to 'mechs in other sections of the Princess code, it looks like isOverheating would ignore ASFs, causing them to be less likely to disable their LAMS units.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deliberate choice - fighters are more vulnerable to damage so having AMS active, even when overheating, is considered useful. I considered included them at first but decided just to do Mechs and keep the complexity down. Can look into adding something if warranted, though.
boolean conserveAmmo = curAMS.getLinkedAmmo().getUsableShotsLeft() <= (int) Math.floor(curAMS.getOriginalShots() * | ||
behaviorSettings.getSelfPreservationValue() / 100.0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be wrapped into the existing ammo conservation code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, but there's a distinct difference. The existing ammo conservation code is a shoot/no shoot determination. For AMS it is actually a mode change that prevents automatic operation in the following turn. Keeps the existing code about determining whether to fire, and this on whether to turn this specific system off or on.
// Consider turning off AMS to conserve ammo unless it's needed for infantry | ||
if (conserveAmmo && !newAMSMode.equals(Weapon.MODE_AMS_MANUAL)) { | ||
|
||
int ammoTN = 12 - behaviorSettings.getBraveryIndex(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise; this could probably be incorporated into the existing conservation / threshold calculation code.
if (ammoTN < 10) { | ||
if (Compute.d6(2) >= ammoTN) { | ||
newAMSMode = EquipmentMode.getMode(Weapon.MODE_AMS_OFF); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the choice to change mode randomized rather than deterministic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While it's good to have the bulk of the behavior calculated rather than random, I'm not entirely sold on a fully deterministic Princess; for one, it is too easy to predict. There will be circumstances like this, where there is no clear 'good' outcome that can be easily calculated vs the impact the decision will have. Sometimes it's best all 'round to have it to do a simple gut check and hope for the best.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of questions, and a couple nits I'd like picked, but otherwise looks very promising.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Currently, Princess will use AMS as a manual weapon without first switching it to the proper mode (this might warrant some checks inside ToHitData, similar to those which prevent combing called and aimed shots). This PR ensures that Princess will only consider using it manually if the system has the correct mode set.
It also adds some code to the end phase to select an appropriate mode for the AMS systems each unit may be carrying. Most of the time this will be on/automatic, but laser AMS may be shut off to assist with heat management on seriously overheating units and relatively undamaged units may turn off conventional AMS to conserve ammo when running low. Manual/use as a weapon mode is restricted to when the unit comes into close contact with infantry; once it breaks contact with the infantry and stops firing it the mode will reset to on/automatic.
Will close #5084 .