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

Fixed Missing Site Location Change Code & Turnover Information Storage #4490

Merged
merged 3 commits into from
Jul 31, 2024

Conversation

IllianiCBT
Copy link
Collaborator

@IllianiCBT IllianiCBT commented Jul 27, 2024

Somehow I managed to accidentally all but wipe #4462 prior to it getting merged. It's definitely a case of error between keyboard and chair, but either way, this PR restores the code.

Furthermore, an oversight in the storing of turnover results across days caused it to never be forgotten so users would get spammed with their turnover results every day. On the bright side, this means it was currently storing the information across new day events! Not ideal though, so this PR also fixes that.

I also spotted that the leasing of DropShips was using a lossy implicit cast in a number of places, so I corrected that. DropShip need calculations should now be a little more accurate. Eventually we probably want to move this from Campaign.java and give it a full review to make sure everything is being handled correctly. Especially as some of this code hasn't been touched for 7 years.

@IllianiCBT IllianiCBT added Bug Personnel Personnel-related Issues Refactoring labels Jul 27, 2024
@IllianiCBT IllianiCBT self-assigned this Jul 27, 2024
Corrected a bug in the handling of leased large mech dropships where the integer part of mech reduction was not properly cast. Also fixed incorrect cumulative cargo capacity to ensure accurate leasing operations.
Updated several logging statements to use placeholders {} instead of string concatenation for better performance and readability. Also changed an equality check to use Objects.equals() to handle potential null values safely.
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 10.33%. Comparing base (0ecc4a0) to head (51dbe8a).
Report is 4 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4490      +/-   ##
============================================
- Coverage     10.33%   10.33%   -0.01%     
+ Complexity     5824     5823       -1     
============================================
  Files           935      935              
  Lines        128395   128391       -4     
  Branches      18889    18891       +2     
============================================
- Hits          13270    13268       -2     
+ Misses       113849   113846       -3     
- Partials       1276     1277       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@IllianiCBT IllianiCBT merged commit 20d49fd into MegaMek:master Jul 31, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Personnel Personnel-related Issues Refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants