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

Spelling - Ring of Fire Protection (EN) #152

Merged
merged 4 commits into from Apr 2, 2021
Merged

Spelling - Ring of Fire Protection (EN) #152

merged 4 commits into from Apr 2, 2021

Conversation

AmProsius
Copy link
Owner

@AmProsius AmProsius commented Apr 1, 2021

Describe the bug
In the English localization the description of the ring Protection of Fire is inconsistent and grammatically incorrect.

Changelog
Changed description of ring "Protection of Fire" to "Ring of Fire Protection"

@AmProsius AmProsius added language dependent validation: validated provided fix labels Feb 25, 2021
@AmProsius AmProsius added this to the v1.1.0 milestone Feb 25, 2021
@AmProsius AmProsius added this to To Do in v1.1.0 via automation Feb 25, 2021
@AmProsius
Copy link
Owner Author

@AmProsius AmProsius commented Feb 25, 2021

changed to

    description     = "Ring of Fire Protection";

@AmProsius AmProsius added opinionated and removed opinionated labels Feb 25, 2021
@AmProsius AmProsius added this to Item description in Fix templates Mar 1, 2021
@AmProsius AmProsius added compatibility: easy type: session fix labels Mar 1, 2021
@AmProsius AmProsius moved this from Item description to Item property in Fix templates Mar 1, 2021
@szapp szapp added the impl: replace assign/push str label Mar 17, 2021
@AmProsius AmProsius self-assigned this Mar 21, 2021
@AmProsius AmProsius removed their assignment Apr 1, 2021
@AmProsius AmProsius requested a review from szapp Apr 1, 2021
@AmProsius AmProsius moved this from To Do to In Progress in v1.1.0 Apr 1, 2021
szapp
szapp approved these changes Apr 2, 2021
Copy link
Collaborator

@szapp szapp left a comment

I updated a few things. Test passes.

@@ -6,6 +6,7 @@
* Fix [#50](https://g1cp.org/issues/50): The inaccessible chest of the crypt below the stonehenge is now correctly positioned and accessible.
* Fix [#52](https://g1cp.org/issues/52): The grindstone in the New Camp now correctly requires a sword blade to use.
* Fix [#149](https://g1cp.org/issues/149): The armor "Improved ore Armor" is now correctly labelled as "Improved Ore Armor".
* Fix [#152](https://g1cp.org/issues/152): The description of the ring "Protection of Fire" is corrected to "Ring of Fire Protection".
Copy link
Collaborator

@szapp szapp Apr 2, 2021

Choose a reason for hiding this comment

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

I updated the text to differentiate that it's the description not the label of the item (compare changelog entries for #149 and #49).

var int symbId; symbId = MEM_GetSymbolIndex("Schutzring_Feuer2");
const string needle = "Protection of Fire";
const string replace = "Ring of Fire Protection";
return (G1CP_ReplaceAssignStr(symbId, "C_ITEM.DESCRIPTION", 0, needle, replace) > 0);
Copy link
Collaborator

@szapp szapp Apr 2, 2021

Choose a reason for hiding this comment

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

I have split this line to stay within 120 character limit of the line.

if (Hlp_StrCmp(item.description, "Ring of Fire Protection")) {
return TRUE;
} else {
var string msg; msg = "Description incorrect: description = '";
Copy link
Collaborator

@szapp szapp Apr 2, 2021

Choose a reason for hiding this comment

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

I set "description" to upper case for consistent test output.

@szapp szapp merged commit 2d109c1 into master Apr 2, 2021
v1.1.0 automation moved this from In Progress to Done Apr 2, 2021
@szapp szapp deleted the bug152 branch Apr 2, 2021
szapp added a commit that referenced this issue Apr 19, 2021
@szapp szapp moved this from Instance variable (string) to Item instance variable (string) in Fix templates Feb 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility: easy impl: replace assign/push str language dependent provided fix type: session fix validation: validated
Projects
Fix templates
Change item instance variable (string)
v1.1.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants