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

AbstractIcon: Force Icon Rework #2946

Merged

Conversation

Windchild292
Copy link
Contributor

@Windchild292 Windchild292 commented Nov 7, 2021

This PR moves Force Icons to Abstract Icon, and divides them into three separate types. These are StandardForceIcon, LayeredForceIcon, and UnitIcon, all of which have differing functionality. Additionally, this adds LayeredForceIcon export to png, force icon TO&E icon copy/paste, and automated new day force icon operational status updates (based on a MekHQ option).

This PR also replaces the Force Icon images with the far more comprehensive and standardized Kailan pack, with full migration from the old setup and a new MekHQ extras repository containing the current pack and a pack of standard force icons created by Mjolnir06.

The full list of what this does, which I used to keep this organized:

For reviewers:
This file is large because of the ~7k lines of migration, and almost all of the file differences are because of the Force Icon pack swapover. Code-wise most of the work is on the dialog-side, to get the three icon types (with the fourth, internal type used in LayeredForceIcon) selectable and usable.

…tribution is part of the MekHQ Extras instead
@Windchild292 Windchild292 added (RFE) Enhancement Requests for Enhancement, new features or implementations Data Hammertime GUI labels Nov 7, 2021
@Windchild292 Windchild292 self-assigned this Nov 7, 2021
@codecov
Copy link

codecov bot commented Nov 9, 2021

Codecov Report

Merging #2946 (d88588c) into master (ea6a179) will decrease coverage by 0.55%.
The diff coverage is n/a.

❗ Current head d88588c differs from pull request most recent head c4336ca. Consider uploading reports for the commit c4336ca to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2946      +/-   ##
============================================
- Coverage     10.78%   10.22%   -0.56%     
- Complexity     4030     4045      +15     
============================================
  Files           758      773      +15     
  Lines        102144   107928    +5784     
  Branches      16728    16767      +39     
============================================
+ Hits          11013    11038      +25     
- Misses        89752    95515    +5763     
+ Partials       1379     1375       -4     
Impacted Files Coverage Δ
MekHQ/src/mekhq/campaign/universe/Faction.java 65.38% <0.00%> (-0.65%) ⬇️
...ekhq/campaign/universe/RandomFactionGenerator.java 56.17% <0.00%> (-0.40%) ⬇️
MekHQ/src/mekhq/MekHQOptions.java 1.63% <0.00%> (-0.09%) ⬇️
MekHQ/src/mekhq/campaign/Campaign.java 12.01% <0.00%> (-0.03%) ⬇️
MekHQ/src/mekhq/gui/BasicInfo.java 0.00% <0.00%> (ø)
MekHQ/src/mekhq/gui/CampaignGUI.java 0.00% <0.00%> (ø)
MekHQ/src/mekhq/gui/FileDialogs.java 0.00% <0.00%> (ø)
MekHQ/src/mekhq/gui/ForceRenderer.java 0.00% <0.00%> (ø)
MekHQ/src/mekhq/campaign/unit/Unit.java 19.95% <0.00%> (ø)
MekHQ/src/mekhq/gui/CommandCenterTab.java 0.00% <0.00%> (ø)
... and 34 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea6a179...c4336ca. Read the comment docs.

@HammerGS
Copy link
Member

Kinda like to see this one in for 49.6. But I can't even filter to see the changed code. It crashes Chrome for me. How about two separate PR's one for the data stuff and one for the code.

Otherwise I think this will never happen.

@Windchild292
Copy link
Contributor Author

I'd have separated the two if they weren't mutually required. As for GitHub's handling... that's annoyingly bad code on their end.

@NickAragua
Copy link
Member

Bottom line is, there is no way to meaningfully review the code changes. Feel free to spin out a separate PR with just the code changes for review purposes only.

@Windchild292
Copy link
Contributor Author

I've opened #3014, which is just the code for this PR.

@Windchild292 Windchild292 merged commit d21f7a3 into MegaMek:master Dec 24, 2021
@Windchild292 Windchild292 deleted the dev_Windchild_ForceIconRework branch December 24, 2021 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Data Hammertime GUI (RFE) Enhancement Requests for Enhancement, new features or implementations
Projects
None yet
3 participants