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

Fix: USA Battle Plan tool tip text details #1559

Merged
merged 1 commit into from
Jan 31, 2023
Merged

Conversation

xezon
Copy link
Collaborator

@xezon xezon commented Jan 17, 2023

This change fixes wrong and inaccurate information in USA Battle Plan tool tip texts.

TODO

  • Fix comment slashes when so possible

@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Text Is text, string, localization related labels Jan 17, 2023
@commy2
Copy link
Collaborator

commy2 commented Jan 18, 2023

It's a better description, but "All ground units" is technically wrong too. It doesn't increase the firepower of Drones like the Sentry Drone. It also doesn't increase the damage caused by Demogen Workers suicide ability, while it works on all other GLA unit's suicide ability.

@xezon
Copy link
Collaborator Author

xezon commented Jan 18, 2023

Do we consider drones as ground units? They are floating above ground, so it seems they are excluded from "ground".

The Demogen Worker damage is a bug?

@Stubbjax
Copy link
Collaborator

Do we consider drones as ground units?

Sentry Drones.

Just rewrite it to "Vehicles and Infantry". Drones are not vehicles and Workers are not Infantry.

@commy2
Copy link
Collaborator

commy2 commented Jan 18, 2023

The Demogen Worker damage is a bug?

Oversight I would say. He's a DOZER kind, but DOZER is on the forbidden list for Strat Center bonuses. Just removing DOZER from the forbidden list would make Hold The Line give extra HP to the USA Dozer though.

Just ignore that as a quirk.

@xezon
Copy link
Collaborator Author

xezon commented Jan 19, 2023

Fixed.

Copy link
Collaborator

@Stubbjax Stubbjax left a comment

Choose a reason for hiding this comment

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

The collocation "Infantry and Vehicles" feels unnatural. In English, the semantically larger "vehicles" would typically come first, e.g. "Vehicles and Infantry", and the lighter word often comes first as well.

@xezon xezon force-pushed the loca-tooltip-battleplan branch from 9a3740b to 10d7945 Compare January 26, 2023 22:29
@xezon
Copy link
Collaborator Author

xezon commented Jan 26, 2023

Fixed. Also added change comments to str file. I made a mistake in gametextcompiler tool and comments start with \\ instead of //. I will correct this mistake in future version.

@Stubbjax
Copy link
Collaborator

I made a mistake in gametextcompiler tool and comments start with \\ instead of //. I will correct this mistake in future version.

What are the implications of this? Can't you just commit a fix?

@xezon
Copy link
Collaborator Author

xezon commented Jan 29, 2023

I would need to hook up new gametextcompiler version to Mod Builder first.

@Stubbjax
Copy link
Collaborator

I would need to hook up new gametextcompiler version to Mod Builder first.

But it is fine to be in the main branch and won't cause issues?

@xezon
Copy link
Collaborator Author

xezon commented Jan 29, 2023

This change as is right now does work with the backslashes.

@xezon xezon force-pushed the loca-tooltip-battleplan branch from 10d7945 to ee626a5 Compare January 31, 2023 20:17
@xezon
Copy link
Collaborator Author

xezon commented Jan 31, 2023

Comments now use forward slashes. Everything works.

@xezon xezon merged commit 6be243c into main Jan 31, 2023
@xezon xezon deleted the loca-tooltip-battleplan branch January 31, 2023 20:21
@xezon xezon added the USA Affects USA faction label Mar 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Minor Severity: Minor < Major < Critical < Blocker Text Is text, string, localization related USA Affects USA faction
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants