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

Issue 6560: Fix for negative temporary prisoner capacity #6567

Merged

Conversation

goron111
Copy link
Contributor

@goron111 goron111 commented Apr 6, 2025

Fix for #6560

  • prevent temporaryPrisonerCapacity from going below MINIMUM_TEMPORARY_CAPACITY
    negative values should not be possible at all, but with 0% it would still be
    impossible to contain the prisoners with any number of additional forces.
  • MINIMUM_TEMPORARY_CAPACITY = 25 (25% normal prisoner capacity)
    is an arbitrary number. I tried to balance being able to recover from an bad event
    and being to easy to recove. This value should be subject to further testing
  • Changed the formular for degreeOfChange so that a lower value for temporaryPrisonerCapacity
    is not slower degrading than a higher temporaryPrisonerCapacity
  • Updated the UnitTest to reflect the changes in degreeOfChange

@goron111 goron111 marked this pull request as draft April 6, 2025 16:53
@goron111 goron111 marked this pull request as ready for review April 6, 2025 18:33
@IllianiCBT
Copy link
Collaborator

Hi, please update the PR description with a brief summary of the changes you made and (ideally) why. It'll make reviewing a lot easier and means if users click on the PR number in the change log they don't need to understand Java to know what changes were made.

@IllianiCBT
Copy link
Collaborator

Also, sorry for the comment spam, but you really want to be making changes in a working branch and not master. Master should be kept 'pure' at all times.

@IllianiCBT
Copy link
Collaborator

Ignore my last comment. I'm pre-coffee and apparently woke up dumb.

Copy link

codecov bot commented Apr 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 11.57%. Comparing base (e0ebe3a) to head (0d4959b).
Report is 24 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##             master    #6567    +/-   ##
==========================================
  Coverage     11.57%   11.57%            
- Complexity     6401     6402     +1     
==========================================
  Files          1085     1085            
  Lines        139499   139604   +105     
  Branches      21514    21532    +18     
==========================================
+ Hits          16149    16166    +17     
- Misses       121776   121868    +92     
+ Partials       1574     1570     -4     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@IllianiCBT IllianiCBT left a comment

Choose a reason for hiding this comment

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

Thanks for taking a look at this. It was on my TODO, but I'm not sure I'd have done as thorough a job, so great work. :)

If you can update the PR description that would be fantastic. But otherwise, nothing stopping this from being merged.

@IllianiCBT IllianiCBT merged commit c3b6c3d into MegaMek:master Apr 7, 2025
6 checks passed
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.

2 participants