{"action":"created","comment":{"url":"https://api.github.com/repos/scummvm/scummvm/pulls/comments/48888495","id":48888495,"diff_hunk":"@@ -1139,6 +1146,257 @@ void GfxPalette::unloadClut() {\n \t_clutTable = 0;\n }\n \n+inline void GfxPalette::_clearCycleMap(const uint16 fromColor, const uint16 numColorsToClear) {\n+\tbool *mapEntry = _cycleMap + fromColor;\n+\tconst bool *lastEntry = _cycleMap + numColorsToClear;\n+\twhile (mapEntry < lastEntry) {\n+\t\t*mapEntry++ = false;\n+\t}\n+}\n+\n+inline void GfxPalette::_setCycleMap(const uint16 fromColor, const uint16 numColorsToSet) {\n+\tbool *mapEntry = _cycleMap + fromColor;\n+\tconst bool *lastEntry = _cycleMap + numColorsToSet;\n+\twhile (mapEntry < lastEntry) {\n+\t\tif (*mapEntry != false) {\n+\t\t\terror(\"Cycles intersect\");\n+\t\t}\n+\t\t*mapEntry++ = true;\n+\t}\n+}\n+\n+inline PalCycler *GfxPalette::_getCycler(const uint16 fromColor) {\n+\tconst int numCyclers = ARRAYSIZE(_cyclers);\n+\n+\tfor (int cyclerIndex = 0; cyclerIndex < numCyclers; ++cyclerIndex) {\n+\t\tPalCycler *cycler = _cyclers[cyclerIndex];\n+\t\tif (cycler != nullptr && cycler->fromColor == fromColor) {\n+\t\t\treturn cycler;\n+\t\t}\n+\t}\n+\n+\treturn nullptr;\n+}\n+\n+inline void _doCycle(PalCycler *cycler, const int16 speed) {\n+\tint16 currentCycle = cycler->currentCycle;\n+\tconst uint16 numColorsToCycle = cycler->numColorsToCycle;\n+\n+\tif (cycler->direction == 0) {\n+\t\tcurrentCycle = (currentCycle - (speed % numColorsToCycle)) + numColorsToCycle;\n+\t} else {\n+\t\tcurrentCycle = currentCycle + speed;\n+\t}\n+\n+\tcycler->currentCycle = (uint8) (currentCycle % numColorsToCycle);\n+}\n+\n+inline void _applyCycleToPalette(PalCycler *cycler, Palette *palette) {\n+\tconst int16 currentCycle = cycler->currentCycle;\n+\tconst uint16 numColorsToCycle = cycler->numColorsToCycle;\n+\n+\tSci::Color tempPalette[numColorsToCycle];\n+\tSci::Color *sourceColor = palette->colors + cycler->fromColor;\n+\tmemcpy(tempPalette, sourceColor, sizeof(tempPalette));\n+\n+\tSci::Color *targetColor = sourceColor;\n+\tfor (int numColorsCycled = 0; numColorsCycled < numColorsToCycle; ++numColorsCycled) {\n+\t\tSci::Color sourceColor = *(tempPalette + ((currentCycle + numColorsCycled) % numColorsToCycle));\n+\t\t*(targetColor + numColorsCycled) = sourceColor;\n+\t}\n+}\n+\n+void GfxPalette::applyAllCycles() {\n+\tfor (int cyclerIndex = 0, numCyclers = ARRAYSIZE(_cyclers); cyclerIndex < numCyclers; ++cyclerIndex) {\n+\t\tPalCycler *cycler = _cyclers[cyclerIndex];\n+\t\tif (cycler != nullptr) {\n+\t\t\tcycler->currentCycle = (uint8) ((((int) cycler->currentCycle) + 1) % cycler->numColorsToCycle);\n+\t\t\t// Disassembly was not fully evaluated to verify this is exactly the same as the code from applyCycles,\n+\t\t\t// but it appeared to be at a glance\n+\t\t\t_applyCycleToPalette(cycler, &_sysPalette);\n+\t\t}\n+\t}\n+}\n+\n+void GfxPalette::applyCycles() {\n+\tfor (int i = 0, len = ARRAYSIZE(_cyclers); i < len; ++i) {\n+\t\tPalCycler *cycler = _cyclers[i];\n+\t\tif (cycler == nullptr) {\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (cycler->delay != 0 && cycler->numTimesPaused == 0) {\n+\t\t\twhile ((cycler->delay + cycler->lastUpdateTick) < g_sci->getTickCount()) {\n+\t\t\t\t_doCycle(cycler, 1);\n+\t\t\t\tcycler->lastUpdateTick += cycler->delay;\n+\t\t\t}\n+\t\t}\n+\n+\t\t_applyCycleToPalette(cycler, &_sysPalette);\n+\t}\n+}\n+\n+int16 GfxPalette::setCycle(const uint16 fromColor, const uint16 toColor, const int16 direction, const int16 delay) {\n+\tassert(fromColor <= UINT8_MAX);\n+\tassert(toColor <= UINT8_MAX);\n+\tassert(fromColor < toColor);\n+\n+\tint cyclerIndex;\n+\tconst int numCyclers = ARRAYSIZE(_cyclers);\n+\n+\tPalCycler *cycler = _getCycler(fromColor);\n+\n+\tif (cycler != nullptr) {\n+\t\tdebug(\"Resetting existing cycler\");\n+\t\t_clearCycleMap(fromColor, cycler->numColorsToCycle);\n+\t} else {\n+\t\tfor (cyclerIndex = 0; cyclerIndex < numCyclers; ++cyclerIndex) {\n+\t\t\tif (_cyclers[cyclerIndex] == nullptr) {\n+\t\t\t\tcycler = new PalCycler;\n+\t\t\t\t_cyclers[cyclerIndex] = cycler;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t// SCI engine overrides the first oldest cycler that it finds where “oldest” is determined by\n+\t// the difference between the tick and now\n+\tif (cycler == nullptr) {\n+\t\tint maxUpdateDelta = -1;\n+\t\t// SCI engine gets the tick twice on each iteration through the loop,\n+\t\t// which seems quite unnecessary\n+\t\tconst uint32 now = g_sci->getTickCount();\n+\n+\t\tfor (cyclerIndex = 0; cyclerIndex < numCyclers; ++cyclerIndex) {\n+\t\t\tPalCycler *candidate = _cyclers[cyclerIndex];\n+\n+\t\t\tconst int32 updateDelta = now - candidate->lastUpdateTick;\n+\t\t\tif (updateDelta >= maxUpdateDelta) {\n+\t\t\t\tmaxUpdateDelta = updateDelta;\n+\t\t\t\tcycler = candidate;\n+\t\t\t}\n+\t\t}\n+\n+\t\t_clearCycleMap(cycler->fromColor, cycler->numColorsToCycle);\n+\t}\n+\n+\tconst uint16 numColorsToCycle = 1 + ((uint8) toColor) - fromColor;\n+\tcycler->fromColor = (uint8) fromColor;\n+\tcycler->numColorsToCycle = (uint8) numColorsToCycle;\n+\tcycler->currentCycle = (uint8) fromColor;\n+\tcycler->direction = direction < 0 ? PalCycleBackward : PalCycleForward;\n+\tcycler->delay = delay;\n+\tcycler->lastUpdateTick = g_sci->getTickCount();\n+\tcycler->numTimesPaused = 0;\n+\n+\t_setCycleMap(fromColor, numColorsToCycle);\n+\n+\t// TODO: Validate return value","path":"engines/sci/graphics/palette.cpp","position":179,"original_position":179,"commit_id":"f96671780fa88e628535b41cba6aa661cf24a4f2","original_commit_id":"f96671780fa88e628535b41cba6aa661cf24a4f2","user":{"login":"bluegr","id":510395,"avatar_url":"https://avatars.githubusercontent.com/u/510395?v=3","gravatar_id":"","url":"https://api.github.com/users/bluegr","html_url":"https://github.com/bluegr","followers_url":"https://api.github.com/users/bluegr/followers","following_url":"https://api.github.com/users/bluegr/following{/other_user}","gists_url":"https://api.github.com/users/bluegr/gists{/gist_id}","starred_url":"https://api.github.com/users/bluegr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bluegr/subscriptions","organizations_url":"https://api.github.com/users/bluegr/orgs","repos_url":"https://api.github.com/users/bluegr/repos","events_url":"https://api.github.com/users/bluegr/events{/privacy}","received_events_url":"https://api.github.com/users/bluegr/received_events","type":"User","site_admin":false},"body":"Validate it... how?","created_at":"2016-01-05T19:54:26Z","updated_at":"2016-01-05T19:54:26Z","html_url":"https://github.com/scummvm/scummvm/pull/644#discussion_r48888495","pull_request_url":"https://api.github.com/repos/scummvm/scummvm/pulls/644","_links":{"self":{"href":"https://api.github.com/repos/scummvm/scummvm/pulls/comments/48888495"},"html":{"href":"https://github.com/scummvm/scummvm/pull/644#discussion_r48888495"},"pull_request":{"href":"https://api.github.com/repos/scummvm/scummvm/pulls/644"}}},"pull_request":{"url":"https://api.github.com/repos/scummvm/scummvm/pulls/644","id":54931115,"html_url":"https://github.com/scummvm/scummvm/pull/644","diff_url":"https://github.com/scummvm/scummvm/pull/644.diff","patch_url":"https://github.com/scummvm/scummvm/pull/644.patch","issue_url":"https://api.github.com/repos/scummvm/scummvm/issues/644","number":644,"state":"open","locked":false,"title":"SCI32: palette management (full kPalCycle, partial kSetPalStyleRange and kPalFade)","user":{"login":"csnover","id":229244,"avatar_url":"https://avatars.githubusercontent.com/u/229244?v=3","gravatar_id":"","url":"https://api.github.com/users/csnover","html_url":"https://github.com/csnover","followers_url":"https://api.github.com/users/csnover/followers","following_url":"https://api.github.com/users/csnover/following{/other_user}","gists_url":"https://api.github.com/users/csnover/gists{/gist_id}","starred_url":"https://api.github.com/users/csnover/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/csnover/subscriptions","organizations_url":"https://api.github.com/users/csnover/orgs","repos_url":"https://api.github.com/users/csnover/repos","events_url":"https://api.github.com/users/csnover/events{/privacy}","received_events_url":"https://api.github.com/users/csnover/received_events","type":"User","site_admin":false},"body":"This PR includes implementation of the kPalCycle opcode, partial transcription of kSetPalStyleRange (commented out, since more work needs to be done in order to architect palette code to match the SCI32 architecture before it can be implemented), and some stubs for other parts of palette code from SCI32. This work is based on SQ6DOS disassembly.\r\n\r\nThere is also a small patch to scummsys.h to fix some warnings clang was emitting about redefined constants.\r\n\r\nI’ll accept any feedback to improve the PR/fix problems in the code. Apologies that some things are interleaved into commits a little weirdly; if needed I can try to rewrite the commits to group functionality a little bit better. This was just an artefact of getting stuck and switching between multiple different features in the code.\r\n\r\nThanks!","created_at":"2016-01-03T19:59:44Z","updated_at":"2016-01-05T19:54:26Z","closed_at":null,"merged_at":null,"merge_commit_sha":"32392179e69060eae0b5128e19fb9501d252f891","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/scummvm/scummvm/pulls/644/commits","review_comments_url":"https://api.github.com/repos/scummvm/scummvm/pulls/644/comments","review_comment_url":"https://api.github.com/repos/scummvm/scummvm/pulls/comments{/number}","comments_url":"https://api.github.com/repos/scummvm/scummvm/issues/644/comments","statuses_url":"https://api.github.com/repos/scummvm/scummvm/statuses/f96671780fa88e628535b41cba6aa661cf24a4f2","head":{"label":"csnover:sci32-kPalCycle","ref":"sci32-kPalCycle","sha":"f96671780fa88e628535b41cba6aa661cf24a4f2","user":{"login":"csnover","id":229244,"avatar_url":"https://avatars.githubusercontent.com/u/229244?v=3","gravatar_id":"","url":"https://api.github.com/users/csnover","html_url":"https://github.com/csnover","followers_url":"https://api.github.com/users/csnover/followers","following_url":"https://api.github.com/users/csnover/following{/other_user}","gists_url":"https://api.github.com/users/csnover/gists{/gist_id}","starred_url":"https://api.github.com/users/csnover/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/csnover/subscriptions","organizations_url":"https://api.github.com/users/csnover/orgs","repos_url":"https://api.github.com/users/csnover/repos","events_url":"https://api.github.com/users/csnover/events{/privacy}","received_events_url":"https://api.github.com/users/csnover/received_events","type":"User","site_admin":false},"repo":{"id":40680302,"name":"scummvm","full_name":"csnover/scummvm","owner":{"login":"csnover","id":229244,"avatar_url":"https://avatars.githubusercontent.com/u/229244?v=3","gravatar_id":"","url":"https://api.github.com/users/csnover","html_url":"https://github.com/csnover","followers_url":"https://api.github.com/users/csnover/followers","following_url":"https://api.github.com/users/csnover/following{/other_user}","gists_url":"https://api.github.com/users/csnover/gists{/gist_id}","starred_url":"https://api.github.com/users/csnover/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/csnover/subscriptions","organizations_url":"https://api.github.com/users/csnover/orgs","repos_url":"https://api.github.com/users/csnover/repos","events_url":"https://api.github.com/users/csnover/events{/privacy}","received_events_url":"https://api.github.com/users/csnover/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/csnover/scummvm","description":"ScummVM main repository","fork":true,"url":"https://api.github.com/repos/csnover/scummvm","forks_url":"https://api.github.com/repos/csnover/scummvm/forks","keys_url":"https://api.github.com/repos/csnover/scummvm/keys{/key_id}","collaborators_url":"https://api.github.com/repos/csnover/scummvm/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/csnover/scummvm/teams","hooks_url":"https://api.github.com/repos/csnover/scummvm/hooks","issue_events_url":"https://api.github.com/repos/csnover/scummvm/issues/events{/number}","events_url":"https://api.github.com/repos/csnover/scummvm/events","assignees_url":"https://api.github.com/repos/csnover/scummvm/assignees{/user}","branches_url":"https://api.github.com/repos/csnover/scummvm/branches{/branch}","tags_url":"https://api.github.com/repos/csnover/scummvm/tags","blobs_url":"https://api.github.com/repos/csnover/scummvm/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/csnover/scummvm/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/csnover/scummvm/git/refs{/sha}","trees_url":"https://api.github.com/repos/csnover/scummvm/git/trees{/sha}","statuses_url":"https://api.github.com/repos/csnover/scummvm/statuses/{sha}","languages_url":"https://api.github.com/repos/csnover/scummvm/languages","stargazers_url":"https://api.github.com/repos/csnover/scummvm/stargazers","contributors_url":"https://api.github.com/repos/csnover/scummvm/contributors","subscribers_url":"https://api.github.com/repos/csnover/scummvm/subscribers","subscription_url":"https://api.github.com/repos/csnover/scummvm/subscription","commits_url":"https://api.github.com/repos/csnover/scummvm/commits{/sha}","git_commits_url":"https://api.github.com/repos/csnover/scummvm/git/commits{/sha}","comments_url":"https://api.github.com/repos/csnover/scummvm/comments{/number}","issue_comment_url":"https://api.github.com/repos/csnover/scummvm/issues/comments{/number}","contents_url":"https://api.github.com/repos/csnover/scummvm/contents/{+path}","compare_url":"https://api.github.com/repos/csnover/scummvm/compare/{base}...{head}","merges_url":"https://api.github.com/repos/csnover/scummvm/merges","archive_url":"https://api.github.com/repos/csnover/scummvm/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/csnover/scummvm/downloads","issues_url":"https://api.github.com/repos/csnover/scummvm/issues{/number}","pulls_url":"https://api.github.com/repos/csnover/scummvm/pulls{/number}","milestones_url":"https://api.github.com/repos/csnover/scummvm/milestones{/number}","notifications_url":"https://api.github.com/repos/csnover/scummvm/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/csnover/scummvm/labels{/name}","releases_url":"https://api.github.com/repos/csnover/scummvm/releases{/id}","created_at":"2015-08-13T20:29:41Z","updated_at":"2015-08-13T20:30:07Z","pushed_at":"2016-01-04T04:44:30Z","git_url":"git://github.com/csnover/scummvm.git","ssh_url":"git@github.com:csnover/scummvm.git","clone_url":"https://github.com/csnover/scummvm.git","svn_url":"https://github.com/csnover/scummvm","homepage":"http://www.scummvm.org/","size":177550,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"scummvm:master","ref":"master","sha":"453afd7bbc7b83506638bf87b830384078f729e1","user":{"login":"scummvm","id":365181,"avatar_url":"https://avatars.githubusercontent.com/u/365181?v=3","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,"name":"scummvm","full_name":"scummvm/scummvm","owner":{"login":"scummvm","id":365181,"avatar_url":"https://avatars.githubusercontent.com/u/365181?v=3","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}","created_at":"2011-02-12T15:50:57Z","updated_at":"2015-12-30T06:49:15Z","pushed_at":"2016-01-05T19:48:09Z","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":212288,"stargazers_count":558,"watchers_count":558,"language":"C++","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":279,"mirror_url":null,"open_issues_count":23,"forks":279,"open_issues":23,"watchers":558,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/scummvm/scummvm/pulls/644"},"html":{"href":"https://github.com/scummvm/scummvm/pull/644"},"issue":{"href":"https://api.github.com/repos/scummvm/scummvm/issues/644"},"comments":{"href":"https://api.github.com/repos/scummvm/scummvm/issues/644/comments"},"review_comments":{"href":"https://api.github.com/repos/scummvm/scummvm/pulls/644/comments"},"review_comment":{"href":"https://api.github.com/repos/scummvm/scummvm/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/scummvm/scummvm/pulls/644/commits"},"statuses":{"href":"https://api.github.com/repos/scummvm/scummvm/statuses/f96671780fa88e628535b41cba6aa661cf24a4f2"}}}}