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 This issue only occurs in certain localizations. validation: validated This issue is still present even with Systempack/Union. provided fix This issue has a fix provided in the comments. 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

changed to

    description     = "Ring of Fire Protection";

@AmProsius AmProsius added opinionated This issue or this issues' fix is opinionated and not predefined by the scripts. and removed opinionated This issue or this issues' fix is opinionated and not predefined by the scripts. labels Feb 25, 2021
@AmProsius AmProsius added this to Item description in Fix templates Mar 1, 2021
@AmProsius AmProsius added compatibility: easy This issue is easy to make compatible. type: session fix The fix for this issues is persistent across a session. 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 This issue requires replacing pushed strings or string assignments in the scripts. 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 April 1, 2021 08:15
@AmProsius AmProsius moved this from To Do to In Progress in v1.1.0 Apr 1, 2021
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.

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

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

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

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 April 2, 2021 08:08
szapp added a commit that referenced this pull request Apr 19, 2021
@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 description in Fix templates Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 description
v1.1.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants