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

Crit unification #971

Merged
merged 3 commits into from
Dec 7, 2021
Merged

Crit unification #971

merged 3 commits into from
Dec 7, 2021

Conversation

SJuliez
Copy link
Member

@SJuliez SJuliez commented Dec 4, 2021

  • Crit Cells on various units now use a more unified style and use a standard size font instead of a reduced size font. The width of crit cells adapts to the number of columns used by the unit (5 for meks, 3 for BA)
  • The number of shots for ammo is now displayed at the start of the cell text to (finally) always keep it in sight no matter the name of the ammo
  • Adds experimental mouse click functionality on Crit Cells (add & remove weapons & ammo on Ctrl-Click) to ProtoMeks (for now)

Adds a CritCellUtil class that combines general Crit Cell display functionality. Lots of code cleanup without much visual effect. Especially in the various CriticalView classes loads of code is removed that had little or no effect or was just (to my eye) in need of some updated organization.

image
image

Copy link
Member

@Windchild292 Windchild292 left a comment

Choose a reason for hiding this comment

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

A few things first, mostly just naming conventions.

The code looks great, and is a huge reduction in complexity.

src/megameklab/com/ui/util/CritCellUtil.java Outdated Show resolved Hide resolved
src/megameklab/com/ui/BattleArmor/views/CriticalView.java Outdated Show resolved Hide resolved
src/megameklab/com/ui/util/ProtomekMountList.java Outdated Show resolved Hide resolved
src/megameklab/com/ui/util/ProtomekMountList.java Outdated Show resolved Hide resolved
src/megameklab/com/util/UnitUtil.java Outdated Show resolved Hide resolved
src/megameklab/com/util/UnitUtil.java Outdated Show resolved Hide resolved
src/megameklab/com/ui/util/ProtomekMountList.java Outdated Show resolved Hide resolved
src/megameklab/com/ui/util/ProtomekMountList.java Outdated Show resolved Hide resolved
@SJuliez
Copy link
Member Author

SJuliez commented Dec 7, 2021

All in. We could kinda come up with an agreement on using ProtoMek or ProtoMech in the code ;)

@HammerGS
Copy link
Member

HammerGS commented Dec 7, 2021

I think we should stick with the Mek as opposed to Mech in general.

@SJuliez
Copy link
Member Author

SJuliez commented Dec 7, 2021

OK so let's make it ProtoMek (including the Capital) from now on.

@Windchild292
Copy link
Member

Do you plan on swapping the ones here as part of this PR @SJuliez?

@SJuliez
Copy link
Member Author

SJuliez commented Dec 7, 2021

Nah. I'll do it as I go along.

@Windchild292 Windchild292 merged commit cd00d23 into MegaMek:master Dec 7, 2021
@SJuliez SJuliez deleted the Crit_Unification branch December 26, 2021 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants