ModelRecord updates #5549
ModelRecord updates #5549
Conversation
…infantry, fixed wing, and space craft. Pulled AP evaluation out into its own method. Changed evaluation to numerical instead of straight boolean.
… types, and don't bother once set.
…ce. Exclude conventional infantry weapons, including AP weapons on battle armor. Give Streak-based missile systems a discount.
…not very effective AP weapons
…nd indirect fire capability. Pulled bulk of work out into separate method.
…infantry support weapon. Extract testing to method.
…role isn't provided use the MIXED_ARTILLERY role.
…g/short range and other factors
… checks for fixed wing range checks
…for thresholding even well armored targets
…y other abilities
…ode testing and optimization
…licate ammoFactor application
| } | ||
|
|
||
| public int getSpeed() { | ||
| return speed; | ||
| } | ||
|
|
||
| public boolean getJump() { | ||
| return canJump; |
There was a problem hiding this comment.
Could this possibly refer to the MechSummary ms, like return ms.getJumpMp() > 0; ?
There was a problem hiding this comment.
I didnt check if canJump is used elsewhere.
There was a problem hiding this comment.
This follows a similar style to the other getters, referring to a backing field rather than calling the MechSummary object, for consistency. Easy enough to change, though.
There was a problem hiding this comment.
It's alright as it is. Just something I thought while reading.
| * Proportion of total weapons BV that is best at short range | ||
| * @return between zero (none) and 1.o (all weapons) | ||
| */ | ||
| public double getSRProportion() { |
There was a problem hiding this comment.
Would be lovely if the comment on longRange/SR indicated what they mean. It does not seem self-evident.
Also, 1.o should be 1.0 ;)
There was a problem hiding this comment.
Darn copy-paste errors, one always slips through. Not sure what you're after on the descriptions though - do you want additional comments on the private variables, or a more detailed description on the methods?
Side note: getLongRange() is original and is used in a number of other files, and single-file updates are about all I'm aiming for as a version control noob. Later on I'll put in another PR to clean up the old method names to be more consistent and more evident like the newly added ones.
There was a problem hiding this comment.
What I meant was that, reading the old getLongRange and the new getSRProportion and their comments I could not imagine what they would return. What weapon is worse at its own short range than at its own long range? Is this a fixed comparison with, say, range 10? Meaning a medium laser would be bad at long range and a PPC would be good? But why would the PPC then be bad at short range? Minimum range? No clue. If I ever had to use these methods I would be forced to read the analysis code. The long range one is a legacy method, I saw that. It's ok if you don't want to do that atm.
There was a problem hiding this comment.
Gotcha. I'll put in something more descriptive for those, then that should be it.
| * @param checkWeapon Weapon to check | ||
| * @return between zero (not a short ranged weapon) and 1 | ||
| */ | ||
| private double getShortRangeModifier (EquipmentType checkWeapon) { |
There was a problem hiding this comment.
Our style guide suggests no double spaces and no space between method name and opening ( and opening { not on their own line (line 1071). A bit much :)
There was a problem hiding this comment.
Also it seems that WeaponType suggests itself as the parameter here.
There was a problem hiding this comment.
Same in l.1004: WeaponType and possibly others.
|
|
||
| if (checkWeapon instanceof InfantryWeapon) { | ||
| return notEffective; | ||
| } else if (checkWeapon instanceof megamek.common.weapons.battlearmor.BAMGWeapon) { |
ModelRecord provides a convenient summary of unit capabilities for a number of uses, similar to explicitly declared roles in the RATGen/force generator but more general and automatically generated. For example, when calling for random generation of units with the ANTI_AIRCRAFT role there may simply not be enough of the given unit type/weight class/etc. So the ModelRecord entries provide a property to indicate how well other units do even without the role.
This update adds more detail to the existing analysis process, and adds a few additional points which may be useful e.g. remote drone and robot drone flags to assist with potentially generating all-drone forces at some point, and some basic primitive/retrotech checking. Fills out a couple of legacy FIXME/TODOs. Analysis has been pulled out of the constructor to it's own method, so quickly checking code flow from outside can just step over it. Because this involves looping through all equipment on large numbers of units, extra work is done on loop optimization to avoid unnecessary checks and improve performance.