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

Codechange: Spell 'Viewport' consistently #8260

Merged
merged 1 commit into from Jul 27, 2020

Conversation

techgeeknz
Copy link
Contributor

Some places in the codebase misspell 'Viewport' as 'ViewPort' or 'view_port'.
This patch makes everything consistent.

There are several strings that are named VIEW_PORT and/or VIEWPORT. I have left these alone because I do not know what will break.

@LordAro
Copy link
Member

LordAro commented Jul 2, 2020

Nothing will break as far as I know, go ahead and do the strings as well :)
(Though take care of the alignment)

@techgeeknz
Copy link
Contributor Author

Nothing will break as far as I know, go ahead and do the strings as well :)
(Though take care of the alignment)

It won't break the translation/localisation system?

@LordAro
Copy link
Member

LordAro commented Jul 2, 2020

Nah.

Probably.

@glx22
Copy link
Contributor

glx22 commented Jul 2, 2020

Translations should be safe if everything match. The translation system always reads the updated strings.
Oh and usually we change english.txt in the main commit, then update all other language in a second one.

@techgeeknz
Copy link
Contributor Author

Translations should be safe if everything match. The translation system always reads the updated strings.
Oh and usually we change english.txt in the main commit, then update all other language in a second one.

I thought that only applied to translations? What we are proposing here is to change the symbolic identifiers; things would break horribly if they weren't consistent all the way through, yeah?

src/script/api/game/game_window.hpp.sq Outdated Show resolved Hide resolved
src/toolbar_gui.cpp Outdated Show resolved Hide resolved
src/toolbar_gui.cpp Outdated Show resolved Hide resolved
LordAro
LordAro previously approved these changes Jul 3, 2020
@techgeeknz
Copy link
Contributor Author

Oops, didn‘t need to rebase this one. Sorry about that. (I wonder if I can undo it? 🤔)

@LordAro
Copy link
Member

LordAro commented Jul 3, 2020

Only by (effectively) rebasing again. Why would you want to anyway?

@techgeeknz
Copy link
Contributor Author

Only by (effectively) rebasing again. Why would you want to anyway?

Habit, I guess. Most of my PRs seem to end up conflicting with each other one way or another.
Either that, or I may have used it as a base for the GUI refactoring commits and wanted Github to show me a nice, pretty HTML5 diff against master + this.

@techgeeknz
Copy link
Contributor Author

techgeeknz commented Jul 7, 2020

Presumably, we are waiting for a rebase upon #8192 before continuing here?

Edit: I have reverted the changes to WC_EXTRA_VIEW_PORT and put these into #8267

LordAro
LordAro previously approved these changes Jul 10, 2020
Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

Not especially waiting on anything

Copy link
Contributor

@glx22 glx22 left a comment

Choose a reason for hiding this comment

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

You can include the changes from #8267, as #8192 is merged.

src/script/api/game/game_window.hpp.sq Outdated Show resolved Hide resolved
src/script/api/script_window.hpp Outdated Show resolved Hide resolved
Some places in the codebase misspell 'Viewport' as 'ViewPort' or 'view_port'.
This patch makes everything consistent.
@techgeeknz techgeeknz requested a review from LordAro July 22, 2020 05:49
@LordAro LordAro merged commit a10013d into OpenTTD:master Jul 27, 2020
@techgeeknz techgeeknz deleted the master_viewport_rename branch July 28, 2020 19:33
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

4 participants