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 6247:NPE with TacOps Energy Weapons when Changing Firing Mode #6251

Merged

Conversation

psikomonkie
Copy link
Collaborator

@psikomonkie psikomonkie commented Dec 8, 2024

Fixes #6247

Copy link

codecov bot commented Dec 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.99%. Comparing base (1c0e4ba) to head (5fb42e0).
Report is 64 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #6251   +/-   ##
=========================================
  Coverage     28.99%   28.99%           
- Complexity    13982    13985    +3     
=========================================
  Files          2652     2652           
  Lines        268308   268310    +2     
  Branches      47771    47767    -4     
=========================================
+ Hits          77799    77805    +6     
+ Misses       186627   186622    -5     
- Partials       3882     3883    +1     

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

@neoancient
Copy link
Member

neoancient commented Dec 9, 2024

I forgot about the option to change the power level. It's up in EnergyWeapon where I didn't see it.
Rather than hide half the options and assume they will be the correct ones, you could count how many don't start with pulse: return (int) modes.stream().filter(mode -> !mode.getName().startsWith("Pulse")).count(); It has to be cast to int because Stream::count returns long. This requires that the pulse modes are all added last, which is currently the case but it should be pointed out in a comment.

As for adding the modes, I think it might be best for all the removals to be handled by EnergyWeapon. Adding the line removeMode("Pulse Damage " + dmg); after removeMode("Damage " + dmg); would do it and would have no effect if the mode doesn't exist. Then in LaserWeapon::adaptToGameOptions any damage modes have already been added when super was invoked, so we could add the possible pulse modes with

            if (modes.isEmpty()) {
                addMode("");
                addMode("Pulse");
            } else {
                for (var mode : modes) {
                    addMode("Pulse " + mode.getName());
                }
            }

I didn't actually try any of this code to test it.

@psikomonkie
Copy link
Collaborator Author

Thank you so much neoancient! That was really helpful guidance.

@psikomonkie psikomonkie marked this pull request as ready for review December 9, 2024 02:24
@HammerGS HammerGS merged commit 7c8d2bf into MegaMek:master Dec 15, 2024
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
3 participants