search

FreeCol / freecol

FreeCol: FreeCol is a turn-based strategy game based on the old game Colonization, and similar to Civilization. The objective of the game is to create an independent nation.
GNU General Public License v2.0
585 stars 170 forks source link

Adding "New" town name functionality #75

Closed AdamClobes closed 3 years ago

AdamClobes commented 3 years ago

The purpose of this change is to get the best of both worlds and to create a more realistic experience when naming towns names that have already been used. Added functionality to settlement.java that calls a recursive function to concatenate "New " onto any town name that has already been used. In the unusual case that a player wants to use a name for a third time, it will continue to add "New " to the name, but this is to avoid repeated names while also allowing users to continue to use names they have before.

This was the ticket I made the fix for. https://sourceforge.net/p/freecol/improvement-requests/187/

Calebrw commented 3 years ago

@AdamClobes In looking at the code, it looks like this is for two different things, adding the Names functionality and adding a ToDo list. It seems to me that these should be two separate commits.

mpope042 commented 3 years ago

The New-prefix idea is not unreasonable, although I prefer the work from Marcin that greatly expands the settlement name list such that adding New will be less needed. However I can not support this patch in the current form:

  1. Did you really mean to add a list of all settlement names for a player to instances of the Settlement class? I do not think that does what you want. The right place for this logic is .../common/i18n/NameCache.java.
  2. Note the .../i18n/... We have put a lot of work into internationalization. Adding a prefix of "New" only works in English. Worse still, are you sure that adding whatever-new-is-in-language-x at the front is grammatical in all languages?

I definitely want a Todo list in FreeCol. However that patch needs more work ATM. If you are still interested on working on a Todo list, please open a new pull request with only relevant code to that feature.

Apologies for the very slow response to this. Closing.