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

[After #5706] Carryable objects scenario v2 language #5816

Merged
merged 8 commits into from
Aug 3, 2024

Conversation

SJuliez
Copy link
Member

@SJuliez SJuliez commented Jul 28, 2024

This moves part of the carryable code up from Game to AbstractGame as I don't see why other games types wouldnt use this. Also, adds MM scenario language support and a test setup scenario (can be found in the scenario chooser in the "Test Setups" tab.

Copy link

codecov bot commented Jul 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.99%. Comparing base (37a465c) to head (6a651eb).
Report is 15 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5816      +/-   ##
============================================
- Coverage     29.01%   28.99%   -0.02%     
- Complexity    13928    13930       +2     
============================================
  Files          2511     2512       +1     
  Lines        267123   267279     +156     
  Branches      47787    47833      +46     
============================================
- Hits          77510    77509       -1     
- Misses       185666   185820     +154     
- Partials       3947     3950       +3     

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

@Sleet01
Copy link
Collaborator

Sleet01 commented Jul 31, 2024

I've noticed that loading pre-#5706 in-progress games into the latest code causes NPEs when mousing over the map, because the loaded Game's groundObjects Map isn't initialized and every tooltip is now trying to get info from it.

Do we care enough to safety the calls that refer to it, or are we willing to live with this due to the short time before 0.50.0 goes out?

@SJuliez SJuliez marked this pull request as ready for review July 31, 2024 20:00
Copy link
Member

@NickAragua NickAragua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine, but I'll need to adapt the heavy lifter PR once this is merged.

@HammerGS
Copy link
Member

HammerGS commented Aug 1, 2024

This has picked up some conflicts.

@SJuliez
Copy link
Member Author

SJuliez commented Aug 2, 2024

@NickAragua I updated this a bit. I added some commenting to ICarryable and changed the damage() method to return as a boolean, if the object was destroyed. My thought was that there could be object classes that dont detract damage from tonnage at all (or at a low rate) but are destroyed when one damage block is big enough (hit by AC/20). Other objects might be destroyed by any damage regardless of their tonnage. I worded the comments accordingly.

@HammerGS
Copy link
Member

HammerGS commented Aug 2, 2024

I'm going to let either Nick or Juliez merge this.

@SJuliez SJuliez merged commit df9dc13 into MegaMek:master Aug 3, 2024
5 checks passed
@SJuliez SJuliez deleted the carry_objects-scenario-v2 branch October 15, 2024 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants