vtols off the ground; bombs expansion #2701
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2701 +/- ##
============================================
- Coverage 10.62% 10.62% -0.01%
- Complexity 3848 3850 +2
============================================
Files 717 717
Lines 99442 99472 +30
Branches 16329 16337 +8
============================================
+ Hits 10563 10565 +2
- Misses 87506 87535 +29
+ Partials 1373 1372 -1
Continue to review full report at Codecov.
|
|
This pull request introduces 1 alert when merging 5e4e651 into ac90643 - view on LGTM.com new alerts:
|
MekHQ/src/mekhq/AtBGameThread.java
Outdated
| @@ -318,7 +318,8 @@ public void run() { | |||
|
|
|||
| // we need to wait until the game has actually started to do transport loading | |||
| // This will load the bot's infantry into APCs | |||
| Thread.sleep(MekHQ.getMekHQOptions().getStartGameDelay()); | |||
| // sometimes it needs more time? | |||
| Thread.sleep(MekHQ.getMekHQOptions().getStartGameDelay() * 2); | |||
There was a problem hiding this comment.
This makes no sense... the option exists so you can set the time based on your setup, and if it is having communication troubles you should just increase the option's value
There was a problem hiding this comment.
Maybe I'm being excessively cynical, but I imagine most people don't touch the default setting for that, and have no idea what it means. Thus, the sequence of events I'm picturing is: 1. Person sees units not loading into transport. 2. Person goes onto github/discord/slack to complain about it. 3. Somebody has to waste their time explaining about the delay setting, maybe successfully maybe not. I'd like to skip parts 1-3 of this process.
Another option is to get rid of this, but set the default delay to 2x or even 3-4x higher. I'm currently leaning towards that.
A third option is to go back to sending entities over one at a time, which will most likely guarantee correct behavior, but will be considerably slower in the general case.
There was a problem hiding this comment.
You probably aren't being cynical but instead realistic... I'd go with 4x the default as a base if this is as much of an issue as it sounds.
There was a problem hiding this comment.
Narrowed down the actual cause of the problem, so now we basically wait for all the bot's units to load before attempting to load transports.
MekHQ/src/mekhq/campaign/mission/AtBDynamicScenarioFactory.java
Outdated
Show resolved
Hide resolved
| int numBombs = (int) ((entity.getWeight() - weightModifier) / | ||
| (BombType.getBombCost(bombIndex) * 5)); |
There was a problem hiding this comment.
Should it be 5.0, to ensure it isn't int / int division? Also, rounding?
There was a problem hiding this comment.
All the numbers involved in the operation are integers. Rounding down (the default behavior) is correct because otherwise it would result in putting more bombs in than the unit has capacity.
There was a problem hiding this comment.
To prevent misreads and IDE / CI issues, can I suggest this be written to use the double and a specified round?
|
Fixed a couple of other issues observed while testing:
|
Three changes: