patchFreeciv - Patches: patch #4400, Tidy up requirement descriptions

 
 
Show feedback again

patch #4400: Tidy up requirement descriptions

Submitted by:  Jacob Nevins <jtn>
Submitted on:  Fri 03 Jan 2014 04:55:30 AM UTC  
 
Category: docsPriority: 5 - Normal
Status: DonePrivacy: Public
Assigned to: Jacob Nevins <jtn>Open/Closed: Closed
Planned Release: 2.4.3, 2.5.0, 2.6.0

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Wed 23 Apr 2014 08:20:41 PM UTC, SVN revision 24794:

Fix numerous issues in textual descriptions of requirements.

See gna patch #4400.

(Browse SVN revision 24794)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Wed 23 Apr 2014 08:12:15 PM UTC, SVN revision 24792:

Fix numerous issues in textual descriptions of requirements.

See gna patch #4400.

(Browse SVN revision 24792)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Wed 23 Apr 2014 08:10:53 PM UTC, SVN revision 24791:

Fix numerous issues in textual descriptions of requirements.

See gna patch #4400.

(Browse SVN revision 24791)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Tue 22 Apr 2014 12:10:40 AM UTC, comment #7:

Last tweak: I had a sudden worry about "the The Pyramids wonder". Turns out that our rulesets call it just Pyramids, but I've removed articles and the word 'wonder' on general principles.

Also, S2_4 version attached. I remember why I was keen to do this -- this touches many of the same strings as patch #3841 did, so if I don't backport this, then translators still on S2_4 see two sets of churn.
However, this patch introduces significant churn of its own -- cumulative with patch #3841, it touches 81 strings on S2_4, as opposed to just 33 for patch #3841 alone.

(file #20551, file #20552, file #20553)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Mon 21 Apr 2014 08:13:06 PM UTC, comment #6:

VUT_IMPROVEMENT/REQ_RANGE_CONTINENT: Indeed, this just adds a yak.

Articles: this is probably an artefact of having dealt with i18n too much, but yes, there is no good solution without ruleset integration (another yak).

PL_(): Thanks for the pointer: I retract my criticism entirely :)

VUT_TERRAINALTER: The current code is very prone to false positives, but upon further investigation, the gen_extras stuff hasn't made it completely obsolete yet, so probably another yak (perhaps to be handled as part of thought around patch #3756).

Version 4 of the trunk patch looks good to me now. I'm not confident in my ability to carefully review the S2_5 one after looking at the trunk one, but it at least passes cursory examination.

Emmet Hikory <persia>
Project Member
Mon 21 Apr 2014 05:24:03 PM UTC, comment #5:

New versions addressing these comments + some others.
Commit candidates, although I haven't tested them thoroughly.

(file #20545, file #20546)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Mon 21 Apr 2014 01:04:40 PM UTC, comment #4:

> S2_4 would benefit from it [...] Probably only worth it if
> there is vast player outcry about the help being subtly wrong.

Indeed, and reviewing the diffs, it doesn't make significant improvement with common rulesets (unlike patch #3841).

> "surviving or non-wonder continent-ranged requirements not
> supported": is the survival part not a yet-unimplemented part
> of the sources cache, or is this intended for some reason?

I don't think it's deliberately left out. Supported surviving requirements seem to generally be those that were trivial to implement (world wonders and player buildings are simple arrays indexed by improvement ID).
(Anyway, I'm not sure how a surviving continent-ranged requirement would work, given that continents can change due to terrain change.)

> "Requires Airbase on the tile" feels like it dropped an
> article

[...]

> Also, I think [VUT_STYLE] could benefit from an article in
> many cases (at least for "Asian" and "Classical").

Articles are tricky -- even in English, we can't get "a/an" to agree with a ruleset item. So I avoid them.
Fixing this sort of thing properly would require a much more exciting i18n system than the one we have, much more tied into the rulesets; I don't have effort to design such a thing and I'm not aware of an off-the-shelf design with a well-supported workflow that we can integrate.
So, I generally punt on this; the fact that ruleset items are capitalised means I can pretend they're proper nouns, removing articles and thus defeating English's epenthesis at the cost of sounding slightly awkward (but still better than "a Airbase" or "an Fort").
(Well, this works in my head -- hopefully that's not just due to a lifetime of exposure to badly-localised computer programs.)
Actual inflected languages presumably have it much worse; this is one reason I'm putting in "?extra:" type qualification, to allow translators to use different language hacks around this for each kind of ruleset object.
(...all of which means my S2_5 patch needs rework, because forgetting this argument, I did include articles for road/base requirements in that version.)

> (yes, this is a problem with English: most languages either
> always or never have articles).

Oh good; that hopefully reduces the l10n impact of us merging continuous ("Irrigation") and discrete ("Airbase") things into the single category of 'extras'.

> "...playing the %s nation": should this be "...playing as the %s nation"?

[...]

> is the "nation" semantically useful here?

I'll think about this.

> VUT_STYLE/*: With the change in how city styles work, is
> is this the correct set of ranges? [...]

Possibly not, but I'm trying not to accrue any more yaks; my aim right now is to get the strings matching the code capabilities.

> VUT_DIPLREL/*: These read awkwardly to me [...]

Me too, but I have no better ideas either.

> uses of PL_(): To confirm, these duplicate lines are
> to support multiple pluralisations? What about languages
> with dual number [...]?

Yes, this is for pluralisation. gettext has a standard approach to this to accommodate different pluralisation rules; see here.

> VUT_TERRAINALTER: Can this be safely removed [...]

See above re yaks. That said, the sentence construction is pretty ugly (with elements like "CanRoad"), so it would be nice to remove it from that point of view. (Note to self: this area has regressed grammatically since S2_5, I think?)
(I suspect removing this would complicate rulesets by requiring authors to multiply out effects-allowing-irrigation and effects-of-irrigation.)

> VUT_AI_LEVEL: Could this be "Applies to %s AI
> players" and "Does not apply to %s AI players"?

Mm, maybe -- I suppose current AI levels are adjectival and likely to stay that way.

> VUT_MINYEAR: Perhaps "...the game has/has not yet
> reached the year..."?

That is a bit less awkward, yes.

> requirements.c:
> VUT_MAXTILEUNITS: Why use a non-verbal construction here
> (especially with a verbal hint to translators)?

For me the overriding requirement of universal_name_translation() is that it be short, because of its use in list constructions like 'Allows Hermitage (with Mountains, "Loner" tech, <=2 units)'. So the existing string was too verbose for me.
(Note to self: there is more work to be done in universal_name_translation() on S2_5.)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Sun 20 Apr 2014 07:22:08 PM UTC, comment #3:

This patch looks great. S2_4 would benefit from it, but it is a lot of work to backport it, and then a lot of translation effort to make a difference. Probably only worth it if there is vast player outcry about the help being subtly wrong. Some specific comments on the trunk patch:

helpdata.c:
VUT_IMPROVEMENT/REQ_RANGE_CONTINENT: "surviving or non-wonder continent-ranged requirements not supported": is the survival part not a yet-unimplemented part of the sources cache, or is this intended for some reason? (yes, this text matches the current code in is_building_in_range()).

VUT_EXTRA/*: Extras strings have frustrating grammar: "Requires Irrigation on the tile" makes perfect sense. "Requires Airbase on the tile" feels like it dropped an article (yes, this is a problem with English: most languages either always or never have articles).

VUT_NATION/REQ_RANGE_PLAYER: "...playing the %s nation": should this be "...playing as the %s nation"? The first seems like it could match the construction used in "Is Alice playing freeciv?", "She's playing Bob" (implying Alice and Bob are competing players).

VUT_NATION/*: In parallel to Resources and Extras, is the "nation" semantically useful here?

VUT_STYLE/*: With the change in how city styles work, is this the correct set of ranges? (This may be a different bug in a different ticket, but it occurs to me during review of this patch). Also, I think it could benefit from an article in many cases (at least for "Asian" and "Classical").

VUT_DIPLREL/*: These read awkwardly to me: perhaps something like "Requires you to be at War with at least one living player"(REQ_RANGE_PLAYER/present), or "Requires that no living players are at Peace with each other"(REQ_RANGE_WORLD/!present). That said, being "at Armstice" or "at Cease Fire" or "at Alliance" all make almost no sense, so this suggestion can't be used directly.

uses of PL_(): To confirm, these duplicate lines are to support multiple pluralisations? What about languages with dual number (e.g. Russian), where one may need slightly different construction for one, two, and many (I don't actually know if Russian happens to require different constructions for any of these strings, but I do know that Russian isn't the only language with dual numbers and I do know that I have no practical knowledge of some languages having dual number, so am a poor counterparty to say that something is sufficient).

VUT_AI_LEVEL: Could this be "Applies to %s AI players" and "Does not apply to %s AI players"?

VUT_MINYEAR: Perhaps "...the game has/has not yet reached the year..."?

VUT_TERRAINALTER: Can this be safely removed, with the transition to extras and the various *_POSSIBLE effects? The helptext matches the current code, but I'm not sure we want the current code.

requirements.c:
VUT_MAXTILEUNITS: Why use a non-verbal construction here (especially with a verbal hint to translators)?

cityturn.c:
The prior comment about PL_() applies here as well, although I've nothing to add to it.

Emmet Hikory <persia>
Project Member
Sun 20 Apr 2014 03:37:49 PM UTC, comment #2:
  • New trunk version is commit candidate.
  • S2_5 version also included; only reviewed / tested very lightly so far, but may become commit candidate if I find no issues.
  • Now not sure whether I'll bother with S2_4.

(file #20534, file #20535)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Sat 04 Jan 2014 01:08:21 AM UTC, comment #1:

Newer version fixing more issues. Still not a commit candidate.

(file #19661)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Fri 03 Jan 2014 04:55:30 AM UTC, original submission:

Following on from patch #3841, a patch to rework the text generated by insert_requirement() to fix various issues.
Also a couple of other places like worklist_change_build_target().

  • New text for "survives" requirements (wonders and nations).
  • Acknowledges that wonders can go obsolete and describes the effect of this.
  • Distinguishes Player/Alliance/World ranges for achievements.
  • Distinguishes Local/CAdjacent/Adjacent ranges for CityTile.
  • Use textyear() for MinYear requirements.
  • Fixes i18n plural issues of bug #21269.
  • Fix format string issues.
  • Remove the words "extra" and "resource" -- I'd like "extra" not to appear in the UI (the ruleset names should stand on their own in context). e.g. "Requires the %s extra on the tile" -> "Requires %s on the tile"
    • However, I have qualified the strings for i18n, on the basis that resources tend to be plural words and extras tend to be singular.
  • Technology: "requires you to have researched" => "requires to to know" (research is not the only way to meet tech requirements)
  • Bails out for more unsupported ranges rather than generating inappropriate text.
  • Typos and grammar.
Jacob Nevins <jtn>
Project AdministratorIn charge of this item.

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #20551:  trunk-rework-req-descs-5.patch added by jtn (80kB - text/x-patch - trunk/S2_5/S2_4 r24787 (+ patch #3841 for S2_4))
file #20552:  S2_5-rework-req-descs-5.patch added by jtn (58kB - text/x-patch - trunk/S2_5/S2_4 r24787 (+ patch #3841 for S2_4))
file #20553:  S2_4-rework-req-descs-5.patch added by jtn (39kB - text/x-patch - trunk/S2_5/S2_4 r24787 (+ patch #3841 for S2_4))
file #20545:  trunk-rework-req-descs-4.patch added by jtn (78kB - text/x-diff - trunk/S2_5 r24786)
file #20546:  S2_5-rework-req-descs-4.patch added by jtn (57kB - text/x-diff - trunk/S2_5 r24786)
file #20536:  4400-trunk-output.diff added by jtn (330kB - text/x-diff - Example changes to help output with various rulesets)
file #20537:  4400-S2_5-output.diff added by jtn (306kB - text/x-diff - Example changes to help output with various rulesets)
file #20534:  trunk-rework-req-descs-ter.patch added by jtn (74kB - text/x-diff - trunk/S2_5 r24782 + bug #21470)
file #20535:  S2_5-rework-req-descs-ter.patch added by jtn (53kB - text/x-diff - trunk/S2_5 r24782 + bug #21470)
file #19661:  trunk-rework-req-descs-bis.patch added by jtn (68kB - text/x-diff - WIP: trunk r24004 + various other patches, probably)
file #19631:  trunk-rework-req-descs.patch added by jtn (61kB - text/x-diff - trunk r23987)

 

Digest:
   bug dependencies, patch dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by persia (Posted a comment)
  • -unavailable- added by jtn (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 18 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 23 Apr 2014 08:22:46 PM UTCjtnStatusReady For Test=>Done
      Open/ClosedOpen=>Closed
      Planned Release2.4.3,2.5.0,2.6.0=>2.4.3, 2.5.0, 2.6.0
    Tue 22 Apr 2014 12:10:40 AM UTCjtnAttached File-=>Added trunk-rework-req-descs-5.patch, #20551
      Attached File-=>Added S2_5-rework-req-descs-5.patch, #20552
      Attached File-=>Added S2_4-rework-req-descs-5.patch, #20553
    Mon 21 Apr 2014 05:24:03 PM UTCjtnAttached File-=>Added trunk-rework-req-descs-4.patch, #20545
      Attached File-=>Added S2_5-rework-req-descs-4.patch, #20546
    Sun 20 Apr 2014 03:38:29 PM UTCjtnAttached File-=>Added 4400-trunk-output.diff, #20536
      Attached File-=>Added 4400-S2_5-output.diff, #20537
    Sun 20 Apr 2014 03:37:49 PM UTCjtnAttached File-=>Added trunk-rework-req-descs-ter.patch, #20534
      Attached File-=>Added S2_5-rework-req-descs-ter.patch, #20535
      StatusIn Progress=>Ready For Test
    Wed 22 Jan 2014 09:55:24 PM UTCjtnPlanned Release=>2.4.3,2.5.0,2.6.0
    Sat 11 Jan 2014 07:11:29 PM UTCjtnDependencies-=>Depends on bugs #21470
    Sat 04 Jan 2014 01:08:21 AM UTCjtnAttached File-=>Added trunk-rework-req-descs-bis.patch, #19661
    Fri 03 Jan 2014 07:32:23 PM UTCjtnDependencies-=>Depends on patch #3841
    Fri 03 Jan 2014 05:21:59 AM UTCjtnAttached File-=>Added trunk-rework-req-descs.patch, #19631
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup