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

Fix #3767: NPE while scouting if AtB tries to generate enemy with SPAs not found in MM #3994

Merged
merged 2 commits into from
Apr 15, 2024

Conversation

Sleet01
Copy link
Collaborator

@Sleet01 Sleet01 commented Apr 14, 2024

Makes this lookup safer if defaultspa.xml contains an SPA that MM doesn't know about.
This appears to happen when AtB tries to generate a bot force during scouting, if the force will have pilots with SPAs.
About 27% of the options in defaultspa.xml will cause an NPE, so this change just forces a re-roll if the current SPA isn't found.

Testing:

  • Ran previously-failing scouting missions from both posted issues.
  • Ran all three projects' unit tests.

Close #3767
Close #3884

@Sleet01 Sleet01 requested review from HammerGS and NickAragua April 14, 2024 23:58
@IllianiCBT
Copy link
Collaborator

You might already done that, but if you haven’t it might be worth adding a log warning to say that MM encountered a SPA it didn’t know about and what SPA it was. I can’t read your code, as I’m on the road, so ignore this if you’ve already got that :)

@Sleet01
Copy link
Collaborator Author

Sleet01 commented Apr 15, 2024

You might already done that, but if you haven’t it might be worth adding a log warning to say that MM encountered a SPA it didn’t know about and what SPA it was. I can’t read your code, as I’m on the road, so ignore this if you’ve already got that :)

Great minds think alike!
I added that as a warning in the MekHQ log.

@AaronGullickson AaronGullickson self-requested a review April 15, 2024 16:44
Copy link
Member

@AaronGullickson AaronGullickson left a comment

Choose a reason for hiding this comment

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

LGTM

@Sleet01 Sleet01 merged commit a57921b into MegaMek:master Apr 15, 2024
4 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
3 participants