{"action":"created","comment":{"url":"https://api.github.com/repos/scummvm/scummvm/pulls/comments/192598097","pull_request_review_id":125403853,"id":192598097,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDE5MjU5ODA5Nw==","diff_hunk":"@@ -196,16 +195,19 @@ static const CRUISEGameDescription gameDescriptions[] = {\n class CruiseMetaEngine : public AdvancedMetaEngine {\n public:\n \tCruiseMetaEngine() : AdvancedMetaEngine(Cruise::gameDescriptions, sizeof(Cruise::CRUISEGameDescription), cruiseGames) {\n-\t\t_singleId = \"cruise\";\n \t\t_guiOptions = GUIO2(GUIO_NOSPEECH, GUIO_NOMIDI);\n \t}\n \n+\tconst char *getEngineId() const {\n+\t\treturn \"cruise\";\n+\t}\n+\n \tvirtual const char *getName() const {\n-\t\treturn \"CruisE\";\n+\t\treturn \"Cinematique evo 2\";\n \t}\n \n \tvirtual const char *getOriginalCopyright() const {\n-\t\treturn \"Cruise for a Corpse (C) Delphine Software\";\n+\t\treturn \"Cinematique evo 2 (C) Delphine Software\";","path":"engines/cruise/detection.cpp","position":27,"original_position":27,"commit_id":"03e20dcd85d1bbbff9728943b30af683264e03d3","original_commit_id":"03e20dcd85d1bbbff9728943b30af683264e03d3","user":{"login":"sev-","id":365188,"node_id":"MDQ6VXNlcjM2NTE4OA==","avatar_url":"https://avatars2.githubusercontent.com/u/365188?v=4","gravatar_id":"","url":"https://api.github.com/users/sev-","html_url":"https://github.com/sev-","followers_url":"https://api.github.com/users/sev-/followers","following_url":"https://api.github.com/users/sev-/following{/other_user}","gists_url":"https://api.github.com/users/sev-/gists{/gist_id}","starred_url":"https://api.github.com/users/sev-/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sev-/subscriptions","organizations_url":"https://api.github.com/users/sev-/orgs","repos_url":"https://api.github.com/users/sev-/repos","events_url":"https://api.github.com/users/sev-/events{/privacy}","received_events_url":"https://api.github.com/users/sev-/received_events","type":"User","site_admin":false},"body":"Interesting. Above you were changing Chewy to the game name, and here you're doing the opposite. Is there some reasoning behind this inconsistency?","created_at":"2018-06-03T18:16:21Z","updated_at":"2018-06-03T18:29:58Z","html_url":"https://github.com/scummvm/scummvm/pull/1210#discussion_r192598097","pull_request_url":"https://api.github.com/repos/scummvm/scummvm/pulls/1210","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/scummvm/scummvm/pulls/comments/192598097"},"html":{"href":"https://github.com/scummvm/scummvm/pull/1210#discussion_r192598097"},"pull_request":{"href":"https://api.github.com/repos/scummvm/scummvm/pulls/1210"}}},"pull_request":{"url":"https://api.github.com/repos/scummvm/scummvm/pulls/1210","id":192239059,"node_id":"MDExOlB1bGxSZXF1ZXN0MTkyMjM5MDU5","html_url":"https://github.com/scummvm/scummvm/pull/1210","diff_url":"https://github.com/scummvm/scummvm/pull/1210.diff","patch_url":"https://github.com/scummvm/scummvm/pull/1210.patch","issue_url":"https://api.github.com/repos/scummvm/scummvm/issues/1210","number":1210,"state":"open","locked":false,"title":"ENGINES: Add an engine id","user":{"login":"bgK","id":52294,"node_id":"MDQ6VXNlcjUyMjk0","avatar_url":"https://avatars1.githubusercontent.com/u/52294?v=4","gravatar_id":"","url":"https://api.github.com/users/bgK","html_url":"https://github.com/bgK","followers_url":"https://api.github.com/users/bgK/followers","following_url":"https://api.github.com/users/bgK/following{/other_user}","gists_url":"https://api.github.com/users/bgK/gists{/gist_id}","starred_url":"https://api.github.com/users/bgK/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bgK/subscriptions","organizations_url":"https://api.github.com/users/bgK/orgs","repos_url":"https://api.github.com/users/bgK/repos","events_url":"https://api.github.com/users/bgK/events{/privacy}","received_events_url":"https://api.github.com/users/bgK/received_events","type":"User","site_admin":false},"body":"Currently games are uniquely identified by their gameId. This is good because a single value can be used to identify a game across all the engines. The gameId is stored in the configuration file under the game target section.\r\n\r\nHowever, as more game engines have been added to ScummVM, the concern of gameId clashes across engines became apparent. A feature was added to the Advanced Detector to work around this issue: singleId. For engines that enable the singleId feature, the gameId is always the same for all the games supported by the engine, allowing to avoid collisions.\r\nHowever, storing the singleId instead of the gameId in the configuration file is a loss of information that causes a few problems:\r\n* The `--list-games` command line command no longer lists all the games supported by the engines using singleId.\r\n* When multiple games using the same engine are stored in the same folder, ScummVM doesn't know which one to launch, and starts the first one (This is annoying for Myst because both the game and the making of are in the same folder on the game CD).\r\n* In the MetaEngines, in code that is executed before detection happens, it's not possible to know which game the currently active target is for. This is a problem for the Mohawk engine because saved game handling is entirely separate across the games. Currently we have to rely on the target name to detect the game (https://github.com/scummvm/scummvm/blob/61f9398b04a4bce397a8be6ae96491a2015a6da2/engines/mohawk/detection.cpp#L241). This is not robust because the target name is user configurable and could very well not contain the game name.\r\n\r\nIn this PR, I propose to use a different approach. Engines have a new engineId property that can be used to uniquely identify them. That way, the gameIds for an engine are namespaced by the engineId. This removes the need for the singleId feature which I propose to remove. The engineIds are stored in the configuration file under the game targets sections along with the gameIds.\r\n\r\nThis pull request contains code allowing to automatically upgrade the game targets to add an engineId and set the appropriate gameId when trying to launch a game (and other operations that require an active game target context such as editing the game options or loading a saved game).\r\n\r\nOn the command line, I have added the ability to use qualified game names where gameIds were previously used. Qualified game names have the following form: `engineId:gameId`. It is now recommended to say:\r\n`scummvm --path=/my/games/riven mohawk:riven` instead of `scummvm --path=/my/games/riven riven`.\r\nNon-qualified game names still work as long as there are no gameId conflicts but are deprecated. (They are deprecated because looking up a game by its gameId only has become a slow path when using dynamic plugins. This is because the gameId => plugin path cache stored in the configuration file has become a engineId => plugin path cache to allow the now more common operation of looking up an engine by its name to be a fast path.)\r\n\r\nOn the command line, I have also added a new `--list-engines` command allowing to print the list of compiled engines along with their ids.\r\n\r\nPlease review.","created_at":"2018-06-03T09:42:08Z","updated_at":"2018-06-03T18:29:58Z","closed_at":null,"merged_at":null,"merge_commit_sha":"d6a1effb43b1c192bd96969418b78aa8292ce66e","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/scummvm/scummvm/pulls/1210/commits","review_comments_url":"https://api.github.com/repos/scummvm/scummvm/pulls/1210/comments","review_comment_url":"https://api.github.com/repos/scummvm/scummvm/pulls/comments{/number}","comments_url":"https://api.github.com/repos/scummvm/scummvm/issues/1210/comments","statuses_url":"https://api.github.com/repos/scummvm/scummvm/statuses/03e20dcd85d1bbbff9728943b30af683264e03d3","head":{"label":"bgK:add-engineid5","ref":"add-engineid5","sha":"03e20dcd85d1bbbff9728943b30af683264e03d3","user":{"login":"bgK","id":52294,"node_id":"MDQ6VXNlcjUyMjk0","avatar_url":"https://avatars1.githubusercontent.com/u/52294?v=4","gravatar_id":"","url":"https://api.github.com/users/bgK","html_url":"https://github.com/bgK","followers_url":"https://api.github.com/users/bgK/followers","following_url":"https://api.github.com/users/bgK/following{/other_user}","gists_url":"https://api.github.com/users/bgK/gists{/gist_id}","starred_url":"https://api.github.com/users/bgK/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bgK/subscriptions","organizations_url":"https://api.github.com/users/bgK/orgs","repos_url":"https://api.github.com/users/bgK/repos","events_url":"https://api.github.com/users/bgK/events{/privacy}","received_events_url":"https://api.github.com/users/bgK/received_events","type":"User","site_admin":false},"repo":{"id":1836318,"node_id":"MDEwOlJlcG9zaXRvcnkxODM2MzE4","name":"scummvm","full_name":"bgK/scummvm","owner":{"login":"bgK","id":52294,"node_id":"MDQ6VXNlcjUyMjk0","avatar_url":"https://avatars1.githubusercontent.com/u/52294?v=4","gravatar_id":"","url":"https://api.github.com/users/bgK","html_url":"https://github.com/bgK","followers_url":"https://api.github.com/users/bgK/followers","following_url":"https://api.github.com/users/bgK/following{/other_user}","gists_url":"https://api.github.com/users/bgK/gists{/gist_id}","starred_url":"https://api.github.com/users/bgK/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bgK/subscriptions","organizations_url":"https://api.github.com/users/bgK/orgs","repos_url":"https://api.github.com/users/bgK/repos","events_url":"https://api.github.com/users/bgK/events{/privacy}","received_events_url":"https://api.github.com/users/bgK/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/bgK/scummvm","description":"ScummVM main repository","fork":true,"url":"https://api.github.com/repos/bgK/scummvm","forks_url":"https://api.github.com/repos/bgK/scummvm/forks","keys_url":"https://api.github.com/repos/bgK/scummvm/keys{/key_id}","collaborators_url":"https://api.github.com/repos/bgK/scummvm/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/bgK/scummvm/teams","hooks_url":"https://api.github.com/repos/bgK/scummvm/hooks","issue_events_url":"https://api.github.com/repos/bgK/scummvm/issues/events{/number}","events_url":"https://api.github.com/repos/bgK/scummvm/events","assignees_url":"https://api.github.com/repos/bgK/scummvm/assignees{/user}","branches_url":"https://api.github.com/repos/bgK/scummvm/branches{/branch}","tags_url":"https://api.github.com/repos/bgK/scummvm/tags","blobs_url":"https://api.github.com/repos/bgK/scummvm/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/bgK/scummvm/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/bgK/scummvm/git/refs{/sha}","trees_url":"https://api.github.com/repos/bgK/scummvm/git/trees{/sha}","statuses_url":"https://api.github.com/repos/bgK/scummvm/statuses/{sha}","languages_url":"https://api.github.com/repos/bgK/scummvm/languages","stargazers_url":"https://api.github.com/repos/bgK/scummvm/stargazers","contributors_url":"https://api.github.com/repos/bgK/scummvm/contributors","subscribers_url":"https://api.github.com/repos/bgK/scummvm/subscribers","subscription_url":"https://api.github.com/repos/bgK/scummvm/subscription","commits_url":"https://api.github.com/repos/bgK/scummvm/commits{/sha}","git_commits_url":"https://api.github.com/repos/bgK/scummvm/git/commits{/sha}","comments_url":"https://api.github.com/repos/bgK/scummvm/comments{/number}","issue_comment_url":"https://api.github.com/repos/bgK/scummvm/issues/comments{/number}","contents_url":"https://api.github.com/repos/bgK/scummvm/contents/{+path}","compare_url":"https://api.github.com/repos/bgK/scummvm/compare/{base}...{head}","merges_url":"https://api.github.com/repos/bgK/scummvm/merges","archive_url":"https://api.github.com/repos/bgK/scummvm/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/bgK/scummvm/downloads","issues_url":"https://api.github.com/repos/bgK/scummvm/issues{/number}","pulls_url":"https://api.github.com/repos/bgK/scummvm/pulls{/number}","milestones_url":"https://api.github.com/repos/bgK/scummvm/milestones{/number}","notifications_url":"https://api.github.com/repos/bgK/scummvm/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/bgK/scummvm/labels{/name}","releases_url":"https://api.github.com/repos/bgK/scummvm/releases{/id}","deployments_url":"https://api.github.com/repos/bgK/scummvm/deployments","created_at":"2011-06-02T09:05:33Z","updated_at":"2017-06-17T17:22:48Z","pushed_at":"2018-06-03T10:40:30Z","git_url":"git://github.com/bgK/scummvm.git","ssh_url":"git@github.com:bgK/scummvm.git","clone_url":"https://github.com/bgK/scummvm.git","svn_url":"https://github.com/bgK/scummvm","homepage":"http://www.scummvm.org/","size":283827,"stargazers_count":2,"watchers_count":2,"language":"C++","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"other","name":"Other","spdx_id":null,"url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":0,"open_issues":0,"watchers":2,"default_branch":"master"}},"base":{"label":"scummvm:master","ref":"master","sha":"56d2bf77bae50bb1cb0eab01841cd1a9e7057f30","user":{"login":"scummvm","id":365181,"node_id":"MDEyOk9yZ2FuaXphdGlvbjM2NTE4MQ==","avatar_url":"https://avatars3.githubusercontent.com/u/365181?v=4","gravatar_id":"","url":"https://api.github.com/users/scummvm","html_url":"https://github.com/scummvm","followers_url":"https://api.github.com/users/scummvm/followers","following_url":"https://api.github.com/users/scummvm/following{/other_user}","gists_url":"https://api.github.com/users/scummvm/gists{/gist_id}","starred_url":"https://api.github.com/users/scummvm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/scummvm/subscriptions","organizations_url":"https://api.github.com/users/scummvm/orgs","repos_url":"https://api.github.com/users/scummvm/repos","events_url":"https://api.github.com/users/scummvm/events{/privacy}","received_events_url":"https://api.github.com/users/scummvm/received_events","type":"Organization","site_admin":false},"repo":{"id":1358940,"node_id":"MDEwOlJlcG9zaXRvcnkxMzU4OTQw","name":"scummvm","full_name":"scummvm/scummvm","owner":{"login":"scummvm","id":365181,"node_id":"MDEyOk9yZ2FuaXphdGlvbjM2NTE4MQ==","avatar_url":"https://avatars3.githubusercontent.com/u/365181?v=4","gravatar_id":"","url":"https://api.github.com/users/scummvm","html_url":"https://github.com/scummvm","followers_url":"https://api.github.com/users/scummvm/followers","following_url":"https://api.github.com/users/scummvm/following{/other_user}","gists_url":"https://api.github.com/users/scummvm/gists{/gist_id}","starred_url":"https://api.github.com/users/scummvm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/scummvm/subscriptions","organizations_url":"https://api.github.com/users/scummvm/orgs","repos_url":"https://api.github.com/users/scummvm/repos","events_url":"https://api.github.com/users/scummvm/events{/privacy}","received_events_url":"https://api.github.com/users/scummvm/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/scummvm/scummvm","description":"ScummVM main repository","fork":false,"url":"https://api.github.com/repos/scummvm/scummvm","forks_url":"https://api.github.com/repos/scummvm/scummvm/forks","keys_url":"https://api.github.com/repos/scummvm/scummvm/keys{/key_id}","collaborators_url":"https://api.github.com/repos/scummvm/scummvm/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/scummvm/scummvm/teams","hooks_url":"https://api.github.com/repos/scummvm/scummvm/hooks","issue_events_url":"https://api.github.com/repos/scummvm/scummvm/issues/events{/number}","events_url":"https://api.github.com/repos/scummvm/scummvm/events","assignees_url":"https://api.github.com/repos/scummvm/scummvm/assignees{/user}","branches_url":"https://api.github.com/repos/scummvm/scummvm/branches{/branch}","tags_url":"https://api.github.com/repos/scummvm/scummvm/tags","blobs_url":"https://api.github.com/repos/scummvm/scummvm/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/scummvm/scummvm/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/scummvm/scummvm/git/refs{/sha}","trees_url":"https://api.github.com/repos/scummvm/scummvm/git/trees{/sha}","statuses_url":"https://api.github.com/repos/scummvm/scummvm/statuses/{sha}","languages_url":"https://api.github.com/repos/scummvm/scummvm/languages","stargazers_url":"https://api.github.com/repos/scummvm/scummvm/stargazers","contributors_url":"https://api.github.com/repos/scummvm/scummvm/contributors","subscribers_url":"https://api.github.com/repos/scummvm/scummvm/subscribers","subscription_url":"https://api.github.com/repos/scummvm/scummvm/subscription","commits_url":"https://api.github.com/repos/scummvm/scummvm/commits{/sha}","git_commits_url":"https://api.github.com/repos/scummvm/scummvm/git/commits{/sha}","comments_url":"https://api.github.com/repos/scummvm/scummvm/comments{/number}","issue_comment_url":"https://api.github.com/repos/scummvm/scummvm/issues/comments{/number}","contents_url":"https://api.github.com/repos/scummvm/scummvm/contents/{+path}","compare_url":"https://api.github.com/repos/scummvm/scummvm/compare/{base}...{head}","merges_url":"https://api.github.com/repos/scummvm/scummvm/merges","archive_url":"https://api.github.com/repos/scummvm/scummvm/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/scummvm/scummvm/downloads","issues_url":"https://api.github.com/repos/scummvm/scummvm/issues{/number}","pulls_url":"https://api.github.com/repos/scummvm/scummvm/pulls{/number}","milestones_url":"https://api.github.com/repos/scummvm/scummvm/milestones{/number}","notifications_url":"https://api.github.com/repos/scummvm/scummvm/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/scummvm/scummvm/labels{/name}","releases_url":"https://api.github.com/repos/scummvm/scummvm/releases{/id}","deployments_url":"https://api.github.com/repos/scummvm/scummvm/deployments","created_at":"2011-02-12T15:50:57Z","updated_at":"2018-06-03T17:43:48Z","pushed_at":"2018-06-03T17:43:43Z","git_url":"git://github.com/scummvm/scummvm.git","ssh_url":"git@github.com:scummvm/scummvm.git","clone_url":"https://github.com/scummvm/scummvm.git","svn_url":"https://github.com/scummvm/scummvm","homepage":"http://www.scummvm.org/","size":287152,"stargazers_count":897,"watchers_count":897,"language":"C++","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":439,"mirror_url":null,"archived":false,"open_issues_count":24,"license":{"key":"other","name":"Other","spdx_id":null,"url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":439,"open_issues":24,"watchers":897,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/scummvm/scummvm/pulls/1210"},"html":{"href":"https://github.com/scummvm/scummvm/pull/1210"},"issue":{"href":"https://api.github.com/repos/scummvm/scummvm/issues/1210"},"comments":{"href":"https://api.github.com/repos/scummvm/scummvm/issues/1210/comments"},"review_comments":{"href":"https://api.github.com/repos/scummvm/scummvm/pulls/1210/comments"},"review_comment":{"href":"https://api.github.com/repos/scummvm/scummvm/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/scummvm/scummvm/pulls/1210/commits"},"statuses":{"href":"https://api.github.com/repos/scummvm/scummvm/statuses/03e20dcd85d1bbbff9728943b30af683264e03d3"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"sev-"}}