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

Move Generic Prefixes to yaml and add prefix for Neutral #14404

Merged
merged 1 commit into from Dec 28, 2017

Conversation

Projects
None yet
4 participants
@MustaphaTR
Member

MustaphaTR commented Nov 22, 2017

Fixes #14402.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Nov 22, 2017

Member

Omitting the "Neutral" here was a deliberate choice. The idea is that an is X (building/infantry/whatever) in its default state should not need any additional or redundant qualifiers. Allied or enemy Xes have special properties that make them different to the default state, and so they are described differently.

I believe this also for consistency with the original RA and TD.

Member

pchote commented Nov 22, 2017

Omitting the "Neutral" here was a deliberate choice. The idea is that an is X (building/infantry/whatever) in its default state should not need any additional or redundant qualifiers. Allied or enemy Xes have special properties that make them different to the default state, and so they are described differently.

I believe this also for consistency with the original RA and TD.

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Nov 22, 2017

Member

I believe this also for consistency with the original RA and TD.

Not D2K (it show Neutral prefix for Smuggler Units), maybe we should move the Prefixes to yaml.

Member

MustaphaTR commented Nov 22, 2017

I believe this also for consistency with the original RA and TD.

Not D2K (it show Neutral prefix for Smuggler Units), maybe we should move the Prefixes to yaml.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Nov 22, 2017

Member

If you want to override this for D2K then you could make these stance prefixes configurable. Replace the GenericStancePrefix flag with strings for AlliedPrefix = "Allied ", NeutralPrefix = null, EnemyPrefix = "Enemy ", and replace the GenericStancePrefix checks with a non-null check for the appropriate prefix.

Member

pchote commented Nov 22, 2017

If you want to override this for D2K then you could make these stance prefixes configurable. Replace the GenericStancePrefix flag with strings for AlliedPrefix = "Allied ", NeutralPrefix = null, EnemyPrefix = "Enemy ", and replace the GenericStancePrefix checks with a non-null check for the appropriate prefix.

@MustaphaTR MustaphaTR changed the title from Add Neutral GenericStancePrefix to Replace GenericStancePrefix with Ally/Neutral/EnemyPrefix Nov 22, 2017

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Nov 22, 2017

Member

Updated. Replaced GenericStancePrefix with Ally/Neutral/EnemyPrefix. Also added a upgrade rule for it.

Member

MustaphaTR commented Nov 22, 2017

Updated. Replaced GenericStancePrefix with Ally/Neutral/EnemyPrefix. Also added a upgrade rule for it.

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Dec 10, 2017

Member

It looks like the Fieldloader trims the white spaces, so:

02:21:11 (abcdefg30) does #14404 even work properly for custom names?
02:21:32 (abcdefg30) i.e. if I define "Ordos " as prefix, do we cut the trailing space away?
02:22:03 (abcdefg30) (-> the pr relies on the fieldloader not doing that)
02:22:25 (abcdefg30) if the fieldloader indeed trims the string
02:22:31 (abcdefg30) we need to hardcode a " " anyway
02:22:35 (abcdefg30) so we need that boolean again
02:22:47 (abcdefg30) to tell it "use a prefix + a whitespace" or "nothing at all"
02:23:08 (abcdefg30) (with an empty prefix we'd still have the hardcoded whitespace, if you get what I mean)

Member

abcdefg30 commented Dec 10, 2017

It looks like the Fieldloader trims the white spaces, so:

02:21:11 (abcdefg30) does #14404 even work properly for custom names?
02:21:32 (abcdefg30) i.e. if I define "Ordos " as prefix, do we cut the trailing space away?
02:22:03 (abcdefg30) (-> the pr relies on the fieldloader not doing that)
02:22:25 (abcdefg30) if the fieldloader indeed trims the string
02:22:31 (abcdefg30) we need to hardcode a " " anyway
02:22:35 (abcdefg30) so we need that boolean again
02:22:47 (abcdefg30) to tell it "use a prefix + a whitespace" or "nothing at all"
02:23:08 (abcdefg30) (with an empty prefix we'd still have the hardcoded whitespace, if you get what I mean)

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Dec 10, 2017

Member

Updated, this now just adds neutral and makes the prefixes settable on yaml. Generic boolean is still there, upgrade rule and changes to main mods are no longer required and removed. Also hardcoded a space into the name as @abcdefg30 said.

Member

MustaphaTR commented Dec 10, 2017

Updated, this now just adds neutral and makes the prefixes settable on yaml. Generic boolean is still there, upgrade rule and changes to main mods are no longer required and removed. Also hardcoded a space into the name as @abcdefg30 said.

@MustaphaTR MustaphaTR changed the title from Replace GenericStancePrefix with Ally/Neutral/EnemyPrefix to Move Generic Prefixes to yaml and add prefix for Neutral Dec 10, 2017

@GraionDilach

👍 after that, however.

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Dec 27, 2017

Member

Updated.

Member

MustaphaTR commented Dec 27, 2017

Updated.

@pchote

pchote approved these changes Dec 28, 2017

@pchote pchote merged commit 7014393 into OpenRA:bleed Dec 28, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MustaphaTR MustaphaTR deleted the MustaphaTR:neutral-prefix branch Dec 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment