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

Language rework #58

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Language rework #58

wants to merge 5 commits into from

Conversation

comradekingu
Copy link

D-pad, as per https://en.wikipedia.org/wiki/D-pad.
Plate- and chainmail, (to avoid compond word errors), as per https://en.wikipedia.org/wiki/Mail_(armour).

@@ -183,7 +183,7 @@
<string name="about_button2">Authors</string>
<string name="about_button3">License</string>
<string name="about_contents1">
Welcome to Andor\'s Trail, an open-source roguelike RPG on Android.&lt;br /&gt;
Welcome to Andor\'s Trail, a copylefted libre roguelike RPG on Android.&lt;br /&gt;

Choose a reason for hiding this comment

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

Hmmm I think this wording is confusing. While the actual source code is fully GPL the project as a whole is only partial copyleft at best. Each content creator retains their copyright over their work and whatever license they choose to use with it. We have explicit written approval from the author(s) of the content to use them in this project only.

Copy link
Owner

Choose a reason for hiding this comment

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

I agree with @sdevaney. This change is not desirable. Moreover, despite being insufficient, the term of open-source is more widely recognized, and best suited for this high level description.

Copy link
Owner

@Zukero Zukero left a comment

Choose a reason for hiding this comment

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

Overall a nice PR. I'd merge this once the "copyleft" one is reverted and the "(requires restart)" changes are consistent.

@@ -654,7 +654,7 @@ Every skill level increases the attack chance of weapons with %1$d %% of their o

<string name="preferences_ui_use_localized_resources_title">Use localized resources</string>
<string name="preferences_ui_use_localized_resources">Use translation of interface and content, where available. (requires restart)</string>
Copy link
Owner

Choose a reason for hiding this comment

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

Missed this one :)

@Rijackson
Copy link

"The file is created with a newer version than is currently running" should be "The file was created with a newer version than is currently running", since the file was clearly created in the past. Similarly, "Andor's Trail was unable to load the savegame." should be "Andor's Trail is unable to load the savegame.", since the problem is clearly a current one. These errors are not due to the changes made for this PR, but it would be a good time to fix them.

I would also question "Do you want to attack…?". I do not think an ellipsis is correct here. An ellipsis is meant convey missing words, or a lost train of thought, or similar. This is a simple question, and should therefore be just "Do you want to attack?". Again, this is not a result of changes made for this PR, but it would be a good time to address this.

Aside from the ones already noted by sdevaney and Zukero, I agree with all the changes.

@comradekingu
Copy link
Author

@Rijackson The actual attempt is made in the past, I think one more-so suggests there is no use trying again. Went with standard fare "Could not".

Everything should be in order now.

@Rijackson
Copy link

Rijackson commented Aug 17, 2018

Looks good to me :)

Copy link
Owner

@Zukero Zukero left a comment

Choose a reason for hiding this comment

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

Getting close :)

@@ -277,12 +277,12 @@

<string name="preferences_display_category">Display</string>
<string name="preferences_display_fullscreen_title">Fullscreen</string>
<string name="preferences_display_fullscreen">Displays the game in fullscreen mode. (Requires restart)</string>
<string name="preferences_display_fullscreen">Program restart required to display the game in fullscreen mode.</string>
Copy link
Owner

Choose a reason for hiding this comment

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

The fact that it requires a program restart is not the essential fact here. This is the description of the option item. I prefered the previous way, either with parenthesis or as a second sentence.

@@ -653,8 +653,8 @@ Every skill level increases the attack chance of weapons with %1$d %% of their o
<!-- Added in v0.7.1 -->

<string name="preferences_ui_use_localized_resources_title">Use localized resources</string>
<string name="preferences_ui_use_localized_resources">Use translation of interface and content, where available. (requires restart)</string>
<string name="change_locale_requires_restart">Changing locale requires restart. Andor\'s Trail has been closed.</string>
<string name="preferences_ui_use_localized_resources">Program restart required to use translation of interface and content, where available.</string>
Copy link
Owner

Choose a reason for hiding this comment

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

Same for this one. The restart is an indication, not the main information.

<string name="preferences_ui_use_localized_resources">Use translation of interface and content, where available. (requires restart)</string>
<string name="change_locale_requires_restart">Changing locale requires restart. Andor\'s Trail has been closed.</string>
<string name="preferences_ui_use_localized_resources">Program restart required to use translation of interface and content, where available.</string>
<string name="change_locale_requires_restart">Program restart required to change language. Andor\'s Trail has been closed.</string>
Copy link
Owner

Choose a reason for hiding this comment

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

This is fine, as we explain the player why the app just closed.

@@ -696,11 +696,11 @@ Every skill level increases the attack chance of weapons with %1$d %% of their o
<string name="skill_sort_unlocked">Unlocked</string>

<string name="preferences_display_theme_title">Theme</string>
<string name="preferences_display_theme">Choose the UI theme. (requires restart)</string>
<string name="preferences_display_theme">Program restart required to choose UI theme.</string>
Copy link
Owner

Choose a reason for hiding this comment

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

Same for this one. The restart is an indication, not the main information.

<string name="preferences_display_theme_blue">Cobalt</string>
<string name="preferences_display_theme_green">Malachite</string>
<string name="preferences_display_theme_charcoal">Obsidian</string>
<string name="change_theme_requires_restart">Changing UI theme requires restart. Andor\'s Trail has been closed.</string>
<string name="change_theme_requires_restart">Program restart required to change UI theme. Andor\'s Trail has been closed.</string>
Copy link
Owner

Choose a reason for hiding this comment

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

This is fine too.

Copy link

@Rijackson Rijackson left a comment

Choose a reason for hiding this comment

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

I don't think this addresses the point Zukero made. For example, we had "Displays the game in fullscreen mode. (Requires restart)" The main point is that the user has an option to display the game in fullscreen mode. The fact that it requires a restart is secondary information.

@comradekingu
Copy link
Author

@Rijackson Made some amends to that effect.

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.

4 participants