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.

Sign up for GitHub

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

Spelling - Improved Ore Armor (DE/EN) #149

Merged
merged 5 commits into from Mar 21, 2021
Merged

Spelling - Improved Ore Armor (DE/EN) #149

merged 5 commits into from Mar 21, 2021

Conversation

AmProsius
Copy link
Owner

@AmProsius AmProsius commented Mar 20, 2021 •
edited

Describe the bug
In the German and English localization there's a typo in the Improved Ore Armor's name.

Changelog

  • Den Namen "verbesserte Erzrüstung" korrigiert zu "Verbesserte Erzrüstung".
  • Changed the name "Improved ore Armor" to "Improved Ore Armor".

@AmProsius AmProsius added the validation: required This issue needs validation from one of the validators. label Feb 24, 2021
@AmProsius
Copy link
Owner Author

AmProsius commented Feb 24, 2021 •
edited by szapp

gothic-1-community-patch/scriptbase/_work/Data/Scripts/Content/Items/Armor.d

Line 1127 in ba24fbc

name = "verbesserte Erzrüstung";

changed to

name 					=	"Verbesserte Erzrüstung";

@AmProsius AmProsius added provided fix This issue has a fix provided in the comments. language dependent This issue only occurs in certain localizations. validation: validated This issue is still present even with Systempack/Union. and removed validation: required This issue needs validation from one of the validators. labels Feb 24, 2021
@AmProsius AmProsius added this to To Do in v1.1.0 via automation Feb 24, 2021
@AmProsius AmProsius added this to the v1.1.0 milestone Feb 24, 2021
@AmProsius AmProsius added this to Item name in Fix templates Mar 1, 2021
@szapp szapp added the impl: replace assign/push str This issue requires replacing pushed strings or string assignments in the scripts. label Mar 17, 2021
@szapp szapp mentioned this pull request Mar 17, 2021
Closed
@szapp szapp added the type: session fix The fix for this issues is persistent across a session. label Mar 17, 2021
@AmProsius AmProsius changed the title Spelling - Improved Ore Armor (DE) Spelling - Improved Ore Armor (DE/EN) Mar 20, 2021
@AmProsius AmProsius requested a review from szapp March 20, 2021 22:00
v1.1.0 automation moved this from To Do to In Progress Mar 20, 2021
szapp
szapp approved these changes Mar 20, 2021
View reviewed changes
src/Ninja/G1CP/Content/Fixes/Session/fix149_DE_EN_ImprovedOreArmorName.d Show resolved Hide resolved
src/Ninja/G1CP/Content/Tests/test149.d Outdated
*
* The text of the item "ORE_ARMOR_H" is inspected programmatically.
*
* Expected behavior: The armor will have the correct description (checked for English and German localization only).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here it says "description" but it is the name in this fix. Maybe we should make these comments more generic. I think it is very tedious to adjust these comments every time (or potentially have to go back and fix them if we forgot). We can keep that in mind going forward.

@AmProsius AmProsius requested a review from szapp March 21, 2021 08:41
szapp
szapp requested changes Mar 21, 2021
View reviewed changes
Copy link
Collaborator

@szapp szapp left a comment

Choose a reason for hiding this comment

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

The test fails for German localization. It seems the file test149.d is not encoded in Windows-1252. And the string "Verbesserte Erzrüstung" (including the umlaut) won't match. Otherwise all good.

@AmProsius AmProsius requested a review from szapp March 21, 2021 13:49
szapp
szapp approved these changes Mar 21, 2021
View reviewed changes
Copy link
Collaborator

@szapp szapp left a comment

Choose a reason for hiding this comment

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

Test passes for German and English localization

@szapp szapp merged commit e0f8ed2 into master Mar 21, 2021
v1.1.0 automation moved this from In Progress to Done Mar 21, 2021
@szapp szapp deleted the bug149 branch March 21, 2021 14:29
@szapp szapp restored the bug149 branch March 21, 2021 14:30
@szapp szapp deleted the bug149 branch March 21, 2021 14:30
@szapp szapp added the compatibility: easy This issue is easy to make compatible. label Mar 21, 2021
@szapp szapp mentioned this pull request Apr 2, 2021
Merged
szapp added a commit that referenced this pull request Apr 20, 2021
@szapp
refactor: simplify test of #149
4d561ad
@szapp szapp mentioned this pull request Dec 26, 2021
Open
@szapp szapp moved this from Instance variable (string) to Item instance variable (string) in Fix templates Feb 5, 2022
@AmProsius AmProsius moved this from Change item instance variable (string) to Change item instance name in Fix templates Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szapp szapp szapp approved these changes

Assignees
No one assigned
Labels
compatibility: easy This issue is easy to make compatible. impl: replace assign/push str This issue requires replacing pushed strings or string assignments in the scripts. language dependent This issue only occurs in certain localizations. provided fix This issue has a fix provided in the comments. type: session fix The fix for this issues is persistent across a session. validation: validated This issue is still present even with Systempack/Union.
Projects
Fix templates
Change item instance name
v1.1.0
  
Done
Milestone
v1.1.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@AmProsius @szapp