-
Notifications
You must be signed in to change notification settings - Fork 176
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
Reworked Monthly XP in Education Module #4365
Merged
IllianiCBT
merged 12 commits into
MegaMek:master
from
IllianiCBT:education_monthlyXpBug
Jul 12, 2024
Merged
Reworked Monthly XP in Education Module #4365
IllianiCBT
merged 12 commits into
MegaMek:master
from
IllianiCBT:education_monthlyXpBug
Jul 12, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The education checks in EducationController.java were refactored for improved readability and maintainability. The code was broken down into individual methods each handling a specific type of check: weekly, monthly, and yearly.
The text for the "barely graduated" education status in the Education.properties file was updated. The revised version provides a more specific phrase that includes the name of the institution from which the individual has barely graduated.
Refactored the EducationController class, specifically the methods which generate prefixes, suffixes, and types. Switch-case statements were refactored for quick assignment of strings. Removed code that checked for the start of a new month to allocate training XP, which is no longer required. Improvements were also made to add bonus XP. The "graduatedBarely" and "graduatedChild" conditions were streamlined, thus removing duplicated code. Finally, removed the redundant use of Collectors.toList().
The code now checks if the person's highest education level is less than the academy's education level before awarding bonus XP. This prevents a potential exploit in XP distribution.
The method 'checkForAcademyClosure' in EducationController was refactored. The return type was changed from boolean to void as the returned boolean value was not used elsewhere in the code.
The random XP option in the campaign was replaced with a more flexible bonus XP multiplier. Furthermore, all references to the old system were removed from the UI and the underlying codebase.
The calculation for xpRate in the addBonusXp method of EducationController was updated. The formula was adjusted to factor in the faculty skill level and operate over a different range by dividing the time spent in education by 600, rather than 150.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4365 +/- ##
=========================================
Coverage 10.27% 10.27%
+ Complexity 5810 5809 -1
=========================================
Files 925 925
Lines 125997 125959 -38
Branches 18646 18615 -31
=========================================
- Hits 12941 12938 -3
+ Misses 111781 111746 -35
Partials 1275 1275 ☔ View full report in Codecov by Sentry. |
The variable "bonusXpRate" used in different places in CampaignOptions class was renamed to "facultyXpRate". Accompanying changes were made in labels, Spinner and other related methods and comments as well to maintain consistency. This refactoring was made to provide more clarity in identifying the purpose of the variable.
The method 'addBonusXp' was renamed to 'addFacultyXp' to more accurately reflect its functionality. The related variable and calculations were updated accordingly. Moreover, a base XP rate is now also awarded when the person's highest education level is equal or greater than the academy's education level.
In the EducationController class, a line of code has been added for granting faculty XP when a person drops out of education.
Implemented additional checks in the addFacultyXp method to account for an academy's type when calculating experience points. Specifically, logic now differentiates whether the academy is a PrepSchool or not.
The bonus calculation has been refactored in the EducationController. Specifically, we've introduced a bonusPercentage that is determined by the bonusCount divided by 5. Then, this bonusPercentage is utilized to calculate the bonusAmount, which gets factored into the final XP award.
HammerGS
approved these changes
Jul 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses a number of issues with the education module:
Addressing the second issue, I discussed this over on Discord and we collectively opted to change 'random monthly xp' to a fixed value gained at the conclusion of a qualification. To this end I reworked the graduation/drop-out related methods and campaign options. This dramatically reduces the amount of XP gained from the education module to more reasonable levels, but still allows for user control (to accommodate campaigns with custom skill purchase values).
Closes #4357
Partially addresses #4359
Note: As there are a few changes coming to the education module I opted to update the documentation separately.