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

Jump to bottom

Codechange: Spell 'Viewport' consistently #8260

Merged
merged 1 commit into from Jul 27, 2020
Merged

Codechange: Spell 'Viewport' consistently #8260

merged 1 commit into from Jul 27, 2020

Conversation

@techgeeknz
Copy link
Contributor

@techgeeknz techgeeknz commented Jul 2, 2020

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 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

@techgeeknz techgeeknz 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)

It won't break the translation/localisation system?

@LordAro
Copy link
Member

@LordAro LordAro commented Jul 2, 2020

Nah.

Probably.

@glx22
Copy link
Contributor

@glx22 glx22 commented Jul 2, 2020 •
edited

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

@techgeeknz techgeeknz commented Jul 3, 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.

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?

@techgeeknz techgeeknz force-pushed the master_viewport_rename branch from 17eead3 to 6dcde46 Jul 3, 2020
techgeeknz
techgeeknz reviewed Jul 3, 2020
View changes
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
techgeeknz
techgeeknz reviewed Jul 3, 2020
View changes
src/toolbar_gui.cpp Show resolved Hide resolved
@techgeeknz techgeeknz force-pushed the master_viewport_rename branch from 6dcde46 to bea33f1 Jul 3, 2020
@techgeeknz techgeeknz force-pushed the master_viewport_rename branch from bea33f1 to 96937df Jul 3, 2020
@techgeeknz
Copy link
Contributor Author

@techgeeknz techgeeknz commented Jul 3, 2020

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

@techgeeknz techgeeknz force-pushed the master_viewport_rename branch from 96937df to bea33f1 Jul 3, 2020
@LordAro
Copy link
Member

@LordAro LordAro commented Jul 3, 2020

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

@techgeeknz
Copy link
Contributor Author

@techgeeknz techgeeknz commented Jul 3, 2020

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 techgeeknz commented Jul 7, 2020 •
edited

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

@techgeeknz techgeeknz force-pushed the master_viewport_rename branch from bea33f1 to b823268 Jul 7, 2020
@techgeeknz techgeeknz mentioned this pull request Jul 7, 2020
Closed
LordAro
LordAro reviewed Jul 10, 2020
View changes
Copy link
Member

@LordAro LordAro left a comment

Not especially waiting on anything

glx22
glx22 requested changes Jul 16, 2020
View changes
Copy link
Contributor

@glx22 glx22 left a comment

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

@techgeeknz techgeeknz force-pushed the master_viewport_rename branch from b823268 to f2ba109 Jul 19, 2020
techgeeknz
techgeeknz reviewed Jul 19, 2020
View changes
src/script/api/game/game_window.hpp.sq Outdated Show resolved Hide resolved
src/script/api/script_window.hpp Outdated Show resolved Hide resolved
@techgeeknz
Codechange: Spell 'Viewport' consistently
69fdaa5 
Some places in the codebase misspell 'Viewport' as 'ViewPort' or 'view_port'.
This patch makes everything consistent.
@techgeeknz techgeeknz force-pushed the master_viewport_rename branch from f2ba109 to 69fdaa5 Jul 19, 2020
@techgeeknz techgeeknz requested a review from glx22 Jul 19, 2020
glx22
glx22 approved these changes Jul 19, 2020
View changes
@techgeeknz techgeeknz mentioned this pull request Jul 21, 2020
Closed
@techgeeknz techgeeknz requested a review from nielsmh Jul 22, 2020
@techgeeknz techgeeknz requested a review from LordAro Jul 22, 2020
@LordAro LordAro merged commit a10013d into OpenTTD:master Jul 27, 2020
8 checks passed
@techgeeknz techgeeknz deleted the master_viewport_rename branch Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glx22 glx22

@LordAro LordAro

@nielsmh nielsmh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet
4 participants
@techgeeknz @LordAro @glx22 @nielsmh