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

UVD: Remove unnecessary UI elements when the selected unit does not have a special armorType #6523

Merged
merged 6 commits into from
Nov 20, 2024

Conversation

Basilisk3
Copy link
Collaborator

@Basilisk3 Basilisk3 commented Nov 9, 2024

Description of the proposed changes

This is only a small UI tweak for units without a special armorType like, for example, the Striker. The current armorType portion of uvd.lua looks like this for these units:

Armor Type: Normal          (% of damage taken)
Normal - 100.0

This is the new output:

Armor Type: Normal

Both (% of damage taken) and Normal - 100.0 do not provide the player with any relevant information, so they can be hidden.

Testing done on the proposed changes

Only affects units without a special armorType.

Checklist

  • Changes are documented in the changelog for the next game version

@lL1l1
Copy link
Contributor

lL1l1 commented Nov 10, 2024

What if a mod changes the normal armor to take different damage from something else? I think an armor type's properties should not be shown only if they are all 1x damage taken.

@lL1l1 lL1l1 added the area: ui Anything to do with the User Interface of the Game label Nov 10, 2024
@Basilisk3
Copy link
Collaborator Author

Basilisk3 commented Nov 10, 2024

I applied your feedback. Unfortunately, reliably removing (% of damage taken) when nothing is displayed underneath it has proven difficult, so I kept it.

@lL1l1
Copy link
Contributor

lL1l1 commented Nov 12, 2024

If you don't mind I added a way that removes the "% of damage taken" text. Basically table.insert can insert at the first position, making it easy to keep everything the same then just add the header line in the beginning of the lines, after checking if the armor type takes any adjusted damages.

@Basilisk3
Copy link
Collaborator Author

Thanks, tested and works.

@Basilisk3 Basilisk3 requested a review from BlackYps November 17, 2024 16:20
@BlackYps
Copy link
Contributor

Good thinking here so far. While reading the discussion I was wondering if it makes sense to even display the armor type: Normal in this situation? Maybe we should display nothing at all?
I don't really have an opinion on this yet, just wanted to throw it into the discussion.

@lL1l1
Copy link
Contributor

lL1l1 commented Nov 18, 2024

I think it's best to display it so that players and modders know that armor types exist, even if normal armor doesn't have any special modifiers.

@BlackYps
Copy link
Contributor

Makes sense

@lL1l1 lL1l1 merged commit 79dce4b into FAForever:develop Nov 20, 2024
5 checks passed
@Basilisk3 Basilisk3 deleted the uvd_armor branch November 20, 2024 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ui Anything to do with the User Interface of the Game
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants