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

Feature: Added Spanish town names #8146

Closed
wants to merge 5 commits into from

Conversation

rasputino
Copy link

@rasputino rasputino commented May 14, 2020

I added Spanish (from Spain) town names with its own town name generator. I did the differentiation of Latinamerican and Spanish cities.

2020-05-14 11_03_27-Window

@James103
Copy link
Contributor

James103 commented May 14, 2020

Commit message format needs to be followed, since otherwise commit checker has failed.

@rasputino rasputino changed the title Added Spanish town names Feature: Added Spanish town names May 14, 2020
@rasputino
Copy link
Author

rasputino commented May 14, 2020

I'll fix the problems and try again

@rasputino rasputino closed this May 14, 2020
@LordAro
Copy link
Member

LordAro commented May 14, 2020

This pull request will do fine - just needs some rebasing

@LordAro LordAro reopened this May 14, 2020
@nielsmh
Copy link
Contributor

nielsmh commented May 14, 2020

Shouldn't this be a NewGRF town name set?

@planetmaker
Copy link
Contributor

planetmaker commented May 14, 2020

Shouldn't this be a NewGRF town name set?

I'm also strongly in favour of the NewGRF approach. The integrated townname lists are somewhat deprecated for a decade.

@nielsmh nielsmh mentioned this pull request May 14, 2020
@rasputino
Copy link
Author

rasputino commented May 14, 2020

This pull request will do fine - just needs some rebasing

Could you help me with some tips? thanks

@rasputino
Copy link
Author

rasputino commented May 14, 2020

Shouldn't this be a NewGRF town name set?

There is one, but has no MakeTownGenerator

@glx22
Copy link
Contributor

glx22 commented May 14, 2020

PRs should be based on master, not on release branches

@LordAro
Copy link
Member

LordAro commented May 14, 2020

Didn't even spot that the first time round. You can't fix that from this PR, so closing. Also see the above comments - we're generally opposed to adding further town name generators (they're old and creaky and should be extended using NewGRFs instead)

@LordAro LordAro closed this May 14, 2020
@James103
Copy link
Contributor

James103 commented May 14, 2020

we're generally opposed to adding further town name generators (they're old and creaky and should be extended using NewGRFs instead

Will all the existing town name generators (except for a fallback one) eventually be converted to NewGRFs that are shipped with the base game? This could even apply to old save games - just load the NewGRF that corresponds to the town name generator used in the savegame.

@rasputino
Copy link
Author

rasputino commented May 15, 2020

PRs should be based on master, not on release branches

Thanks, I didn't know it

@rasputino
Copy link
Author

rasputino commented May 15, 2020

Didn't even spot that the first time round. You can't fix that from this PR, so closing. Also see the above comments - we're generally opposed to adding further town name generators (they're old and creaky and should be extended using NewGRFs instead)

The problem is that is surprising to find Catalan town names and not Spanish town names. Catalunya is just a region of Spain, thus from my point of view, Catalan town names should be replaced by Spanish town names or just add the Spanish town names.

@Yexo
Copy link
Contributor

Yexo commented May 16, 2020

The Catalan town name generator was the last town generator added directly to OpenTTD in August 2006. I'm not going to bother find exactly when NewGRF support for new town names was added, but I suspect that was shortly afterwards (definitely before 2008).

Catalan town names should be replaced by Spanish town names

That'll break existing savegames.

or just add the Spanish town names.

That's your goal, but not an argument in itself.

The list of hardcoded town name generators is a historical artifact from before the NewGRF approach was finished (or even started). At this point we're simply not accepting any new hardcoded town name generators. There is a perfectly fine alternative: write a NewGRF that generates the town names you want (and make it available via the online content service).

Will all the existing town name generators (except for a fallback one) eventually be converted to NewGRFs that are shipped with the base game? This could even apply to old save games - just load the NewGRF that corresponds to the town name generator used in the savegame.

While this might be theoretically possible, it's definitely going to be hard to do without changing town names in existing savegames. There doesn't seem to be a large advantage to doing so: it's basically a lot of work for very little benefit. If you or someone else wants to take this on, feel free to try, but please discuss you approach first (not here, rather on IRC/new issue/forums).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants