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

Allow printing unit list from MM #6190

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pavelbraginskiy
Copy link
Collaborator

Requires MegaMek/megameklab#1656.

Adds a button to print the units of the selected player, without needing to save the list and opening it in MML:
image

This works by saving the unit list to a temporary file and then opening it in MML automatically.

If MegaMek and MegaMekLab are located in same directory (i.e., from a typical MekHQ distribution), MegaMek will automatically detect the MML executable and launch it. Otherwise, MegaMek will have to be told where MML is in the client settings:
image

This can be the actual MML executable (MegaMekLab.exe or MegaMekLab.sh as appropriate) or the gradle wrapper of the MegaMekLab git repo (gradlew.bat for windows, gradlew otherwise). If it's a path to a MegaMekLab executable everything will function as one might expect, if it's a gradle wrapper it will try to compile and run MML from source.

This allows this feature to be used even with a development version of MegaMekLab.

Copy link

codecov bot commented Nov 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 29.17%. Comparing base (a7bb1b4) to head (91a30a4).
Report is 33 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6190      +/-   ##
============================================
- Coverage     29.17%   29.17%   -0.01%     
- Complexity    13971    13981      +10     
============================================
  Files          2628     2628              
  Lines        266564   266641      +77     
  Branches      47575    47584       +9     
============================================
+ Hits          77771    77792      +21     
- Misses       184910   184965      +55     
- Partials       3883     3884       +1     

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

@SJuliez
Copy link
Member

SJuliez commented Nov 9, 2024

I suggest moving over the printing code from MML. That is eventually going to happen anyway.

@pavelbraginskiy
Copy link
Collaborator Author

I suggest moving over the printing code from MML.

That's a huge refactoring. Why do you say it's something that's eventually going to happen anyway?

@SJuliez
Copy link
Member

SJuliez commented Nov 9, 2024

because there's no reason to keep it in MML when MM can make use of it And for MML it's just a big delete and changing imports. Was on my todo list p.12

@HammerGS HammerGS added the For New Dev Cycle This PR should be merged at the beginning of a dev cycle label Nov 9, 2024
@pavelbraginskiy
Copy link
Collaborator Author

I'll let you work on that then, but I think it's going to be harder than just move and change imports.
You'll have to figure out how to set up record sheet settings to be configurable in both MM and MML, how to handle the sheet template data, probably other stuff I'm missing.

@SJuliez SJuliez reopened this Nov 10, 2024
@SJuliez
Copy link
Member

SJuliez commented Nov 10, 2024

No idea when I'll get to it. Or if.

Copy link
Member

@SJuliez SJuliez 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. Suggest calling the button "Print Record Sheets" or at least note this in the tooltip. It's probably not that self-evident in MM.
Edit: or maybe it is, I don't know. @HammerGS you decide and merge.


try {
// Save unit list to a temporary file
var unitFile = File.createTempFile("MegaMekPrint", ".mul");

Check warning

Code scanning / CodeQL

Local information disclosure in a temporary directory Medium

Local information disclosure vulnerability due to use of file readable by other local users.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For New Dev Cycle This PR should be merged at the beginning of a dev cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants