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

Improved MissionRole Switch Cases & Role List #5289

Merged
merged 5 commits into from
Apr 6, 2024

Conversation

IllianiCBT
Copy link
Collaborator

@IllianiCBT IllianiCBT commented Mar 27, 2024

Problem

Currently there are a number of unused roles listed as viable cases in megamek/src/megamek/client/ratgenerator/MissionRole.java.

There are also missing or incorrect roles listed in megamek/docs/RAT Stuff/rat-generator.txt.

Solutions

MissionRole.java

I have gone through and removed any unused cases.

rat-generator.txt

I have clarified that either infantry_support or inf_support are accepted cases for the infantry support role. Both are in use and, while it might be advantageous to unify everything to use the same case, that is beyond the scope of this PR.

I have replaced 'incindiary' with 'incendiary' as the suggested case for units that can set fires. In #5288 Nick suggested we leave uses of 'incindiary' intact, however replacement in this instance leaves other uses intact while ensuring that future uses of the case are using the correct spelling. As this is user facing, I also wanted to ensure that the spelling was correct.

I have added the missing mechanized_ba role and assigned it the definition eligible to be carried by Omni units as mechanized BA.

I changed pocket ws to pocket_warship, as while pocket ws would have been accepted, its only use is in rat-generator.txt so by changing it to pocket_warship I was able to remove that case from MissionRole.java.

Note

This initially started out as an attempt to investigate #5267. However, upon investigation, I found that existing functions relied upon the cases (when used) as currently implemented. As I was unable to find which functions were reliant on the current role cases, I opted instead to just leave the class intact and to only tidy things up so that we weren't checking for unused cases.

Copy link
Collaborator

@Sleet01 Sleet01 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@HammerGS HammerGS merged commit 03c3c0f into MegaMek:master Apr 6, 2024
4 checks passed
HammerGS added a commit that referenced this pull request Apr 6, 2024
+ PR #5289: Improved MissionRole Switch Cases & Role List
+ Fix #5291: Fix for MML #135  Industrial tripod cockpit
+ Fix #3306: Vehicle Lance Movement prevents full deployment of turrets
repligator pushed a commit to repligator/megamek that referenced this pull request Jun 28, 2024
…. This doesn't actually change the function of anything, just makes troubleshooting easier. See PR MegaMek#5289 and Issue MegaMek#5267.

Ran 317 tests, 312 passed, 5 ignored. Also manually tested Force Generator to make sure the effected units and formations still appear.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants