Page MenuHomeWildfire Games

Link users with mod.io's privacy policy and terms of use.
AbandonedPublic

Authored by lyv on Jul 30 2018, 8:40 PM.

Details

Reviewers
Itms
Summary

Most users are currently using mod.io's service without having the knowledge that such documents even exist. There is no persisting option for now, because mod.io doesnt offer a way to obtain the documents through its API and they are free to update those when they feel like it.

Test Plan

Read the additional strings carefully for correct language.
See if nothings broken on different screen resolutions.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6311
Build 10470: Vulcan BuildJenkins
Build 10469: arc lint + arc unit

Event Timeline

lyv created this revision.Jul 30 2018, 8:40 PM
Vulcan added a subscriber: Vulcan.Jul 30 2018, 8:44 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/premodio/premodio.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/premodio/premodio.js
|   6|   6| function connect()
|   7|   7| {
|   8|   8| 	Engine.PopGuiPage();
|   9|    |-	Engine.PushGuiPage("page_modio.xml", {"callback": "initMods"});
|    |   9|+	Engine.PushGuiPage("page_modio.xml", { "callback": "initMods"});
|  10|  10| }
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required before '}'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/premodio/premodio.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/premodio/premodio.js
|   6|   6| function connect()
|   7|   7| {
|   8|   8| 	Engine.PopGuiPage();
|   9|    |-	Engine.PushGuiPage("page_modio.xml", {"callback": "initMods"});
|    |   9|+	Engine.PushGuiPage("page_modio.xml", {"callback": "initMods" });
|  10|  10| }
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js
| 350| 350| 
| 351| 351| 	g_ModsEnabled.sort((folder1, folder2) =>
| 352| 352| 		dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 :
| 353|    |-		dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
|    | 353|+			dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
| 354| 354| 
| 355| 355| 	displayModList("modsEnabledList", g_ModsEnabled);
| 356| 356| }

Link to build: https://jenkins.wildfiregames.com/job/differential/699/display/redirect

lyv added a comment.Jul 30 2018, 8:55 PM

https://mod.io/about states that their logo needs to be used. Unfortunately, I’m not sure about the licenses of the files provided. So ommited for now.

lyv added a comment.EditedJul 31 2018, 10:06 AM

(aware of the missing page_premodio.xml file, will fix with next update)

lyv updated this revision to Diff 6831.Jul 31 2018, 3:08 PM

Add the missing file

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/premodio/premodio.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/premodio/premodio.js
|   6|   6| function connect()
|   7|   7| {
|   8|   8| 	Engine.PopGuiPage();
|   9|    |-	Engine.PushGuiPage("page_modio.xml", {"callback": "initMods"});
|    |   9|+	Engine.PushGuiPage("page_modio.xml", { "callback": "initMods"});
|  10|  10| }
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required before '}'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/premodio/premodio.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/premodio/premodio.js
|   6|   6| function connect()
|   7|   7| {
|   8|   8| 	Engine.PopGuiPage();
|   9|    |-	Engine.PushGuiPage("page_modio.xml", {"callback": "initMods"});
|    |   9|+	Engine.PushGuiPage("page_modio.xml", {"callback": "initMods" });
|  10|  10| }
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js
| 350| 350| 
| 351| 351| 	g_ModsEnabled.sort((folder1, folder2) =>
| 352| 352| 		dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 :
| 353|    |-		dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
|    | 353|+			dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
| 354| 354| 
| 355| 355| 	displayModList("modsEnabledList", g_ModsEnabled);
| 356| 356| }

Link to build: https://jenkins.wildfiregames.com/job/differential/700/display/redirect

Thanks a lot for this diff, probably in the rest of the team and related too!

  • Translations: messages.json needs an entry to make it aware of translations.
  • Foldername: premod sounds a bit dull. It would have to be premodio. It could also be modio_terms or similar if we decide to not add anything other than terms to that page prior to the modio page.
  • Path: Don't mix pages in folders where avoidable, modmod/premodio/ -> premodio/ or modio/page1/ + modio/page2/
In D1601#64178, @smiley wrote:

https://mod.io/about states that their logo needs to be used. Unfortunately, I’m not sure about the licenses of the files provided. So ommited for now.

I don't think it really needs to, especially if it isn't in their terms and conditions. Also they didn't complain.
I thought to include it because it might look nice, but it would have to be compatible with our LICENSE.txt.
(Also would be cool to host that service on our own server with own code, but I guess not in this diff.)

binaries/data/mods/mod/gui/modmod/premodio/modiodisclaimer.txt
8

This is a disclaimer, so no questions but only explanations of the terms. The question could be moved outside of the disclaimer (in the code and the UI), or we could have the "Connect" button and state that the user agrees with the terms by using the service?

"security risk": is ambiguous, could be a possible personal data breach or damage to the computer (https://www.slashgear.com/steam-on-linux-bug-can-delete-all-users-files-16364945/).
Disclaimers often have an exlusion of liability clause which this one is likely supposed to be, it could be precised a bit and mention the personal data aspect independently. The latter would relate to the "not under the control" sentence, perhaps consider rephrasing or amend thatto state that WFG is not processing any data and that personal data requests would have to go to them.

Also some day we might be able to process data, usage statistics.

"Terms and Conditions" and "Privacy Policy" (capitals, singular)

binaries/data/mods/mod/gui/modmod/premodio/premodio.js
10

{space space}

binaries/data/mods/mod/gui/modmod/premodio/premodio.xml
5

2nd include wrong?

25

-1 tab

Also would be good to inform the user that the browser is opened, the window might not switch automatically. A message box should probably ask the user whether to open that url in the browser, here and in any other place that uses OpenURL (so probably a common helper function)? Could be done elsewhere or here

lyv added inline comments.Jul 31 2018, 4:24 PM
binaries/data/mods/mod/gui/modmod/premodio/premodio.xml
5

need initMods() from modmod.js

elexis added inline comments.Jul 31 2018, 5:24 PM
binaries/data/mods/mod/gui/modmod/premodio/premodio.xml
5

"initMods" != initMods

lyv added a comment.Aug 2 2018, 8:30 PM

(I would try updating this tommorow)

lyv updated this revision to Diff 6837.Aug 3 2018, 6:11 PM

Change directory structure as elexis suggested. Added tooltips communicating that the pages would be opened in the browser. Tooltip instead of a message box for now, because the other links from elsewhere should be changed as well. So it could be done in another diff. However, can add if a messagebox is needed.

Vulcan added a comment.Aug 3 2018, 6:14 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js
| 350| 350| 
| 351| 351| 	g_ModsEnabled.sort((folder1, folder2) =>
| 352| 352| 		dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 :
| 353|    |-		dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
|    | 353|+			dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
| 354| 354| 
| 355| 355| 	displayModList("modsEnabledList", g_ModsEnabled);
| 356| 356| }

Link to build: https://jenkins.wildfiregames.com/job/differential/704/display/redirect

elexis added a comment.Aug 9 2018, 3:44 PM

Hold on,

bb wrote a patch to add a terms UI that works for all 3 current online services. This way this patch doesnt have to introduce a new file.

All current active revision proposals with regards to GDPR are currently in https://github.com/bb-bb/0ad/tree/terms (except D1590, but that will come later)

Itms added a comment.Aug 9 2018, 4:35 PM
In D1601#64362, @elexis wrote:

Hold on,

bb wrote a patch to add a terms UI that works for all 3 current online services. This way this patch doesnt have to introduce a new file.

All current active revision proposals with regards to GDPR are currently in https://github.com/bb-bb/0ad/tree/terms (except D1590, but that will come later)

Ah, wonderful!

elexis abandoned this revision.Sep 14 2018, 5:09 PM

Nuking in favor of bbs refactoring in D1602 which reuses one UI page for all 3 online platforms, rather than introducing a new UI page here. Still thanks for the participation!