GitHub PullRequestReviewCommentEvent at 2014-09-04 16:56:22
Repository: hifi
Actor: ZappoMan
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/highfidelity/hifi/pulls/comments/17126817","id":17126817,"diff_hunk":"@@ -1486,112 +1487,125 @@ void MyAvatar::updateCollisionWithEnvironment(float deltaTime, float radius) {\n static CollisionList myCollisions(64);\n \n void MyAvatar::updateCollisionWithVoxels(float deltaTime, float radius) {\n-    const float MAX_VOXEL_COLLISION_SPEED = 100.0f;\n-    float speed = glm::length(_velocity);\n-    if (speed > MAX_VOXEL_COLLISION_SPEED) {\n-        // don't even bother to try to collide against voxles when moving very fast\n-        _trapDuration = 0.0f;\n-        return;\n-    }\n-    bool isTrapped = false;\n-    myCollisions.clear();\n-    const CapsuleShape& boundingShape = _skeletonModel.getBoundingShape();\n-    if (Application::getInstance()->getVoxelTree()->findShapeCollisions(&boundingShape, myCollisions, Octree::TryLock)) {\n-        const float VOXEL_ELASTICITY = 0.0f;\n-        const float VOXEL_DAMPING = 0.0f;\n-        float capsuleRadius = boundingShape.getRadius();\n-        float capsuleHalfHeight = boundingShape.getHalfHeight();\n-        const float MAX_STEP_HEIGHT = capsuleRadius + capsuleHalfHeight;\n-        const float MIN_STEP_HEIGHT = 0.0f;\n-        glm::vec3 footBase = boundingShape.getTranslation() - (capsuleRadius + capsuleHalfHeight) * _worldUpDirection;\n-        float highestStep = 0.0f;\n-        float lowestStep = MAX_STEP_HEIGHT;\n-        glm::vec3 floorPoint;\n-        glm::vec3 stepPenetration(0.0f);\n-        glm::vec3 totalPenetration(0.0f);\n-\n-        for (int i = 0; i < myCollisions.size(); ++i) {\n-            CollisionInfo* collision = myCollisions[i];\n-            glm::vec3 cubeCenter = collision->_vecData;\n-            float cubeSide = collision->_floatData;\n-            float verticalDepth = glm::dot(collision->_penetration, _worldUpDirection);\n-            float horizontalDepth = glm::length(collision->_penetration - verticalDepth * _worldUpDirection);\n-            const float MAX_TRAP_PERIOD = 0.125f;\n-            if (horizontalDepth > capsuleRadius || fabsf(verticalDepth) > MAX_STEP_HEIGHT) {\n-                isTrapped = true;\n-                if (_trapDuration > MAX_TRAP_PERIOD) {\n-                    float distance = glm::dot(boundingShape.getTranslation() - cubeCenter, _worldUpDirection);\n-                    if (distance < 0.0f) {\n-                        distance = fabsf(distance) + 0.5f * cubeSide;\n+    if (Menu::getInstance()->isOptionChecked(MenuOption::CollideAsRagdoll)) {\n+        // compute the bounding cube around avatar\n+        float radius = 2.0f * getBoundingRadius();","path":"interface/src/avatar/MyAvatar.cpp","position":67,"original_position":67,"commit_id":"e120697a9b7f8fd0594629c4d84df2003a811d89","original_commit_id":"e120697a9b7f8fd0594629c4d84df2003a811d89","user":{"login":"ZappoMan","id":2093195,"avatar_url":"https://avatars.githubusercontent.com/u/2093195?v=2","gravatar_id":"c42557c73e8386a04239c421c97732b3","url":"https://api.github.com/users/ZappoMan","html_url":"https://github.com/ZappoMan","followers_url":"https://api.github.com/users/ZappoMan/followers","following_url":"https://api.github.com/users/ZappoMan/following{/other_user}","gists_url":"https://api.github.com/users/ZappoMan/gists{/gist_id}","starred_url":"https://api.github.com/users/ZappoMan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ZappoMan/subscriptions","organizations_url":"https://api.github.com/users/ZappoMan/orgs","repos_url":"https://api.github.com/users/ZappoMan/repos","events_url":"https://api.github.com/users/ZappoMan/events{/privacy}","received_events_url":"https://api.github.com/users/ZappoMan/received_events","type":"User","site_admin":false},"body":"this variable name is confusing... why is radius 2 * radius?\r\n\r\nI think you the local variable radius is the \"search radius\"... maybe use that name?","created_at":"2014-09-04T16:56:22Z","updated_at":"2014-09-04T16:56:22Z","html_url":"https://github.com/highfidelity/hifi/pull/3343#discussion_r17126817","pull_request_url":"https://api.github.com/repos/highfidelity/hifi/pulls/3343","_links":{"self":{"href":"https://api.github.com/repos/highfidelity/hifi/pulls/comments/17126817"},"html":{"href":"https://github.com/highfidelity/hifi/pull/3343#discussion_r17126817"},"pull_request":{"href":"https://api.github.com/repos/highfidelity/hifi/pulls/3343"}}},"pull_request":{"url":"https://api.github.com/repos/highfidelity/hifi/pulls/3343","id":20713727,"html_url":"https://github.com/highfidelity/hifi/pull/3343","diff_url":"https://github.com/highfidelity/hifi/pull/3343.diff","patch_url":"https://github.com/highfidelity/hifi/pull/3343.patch","issue_url":"https://api.github.com/repos/highfidelity/hifi/issues/3343","number":3343,"state":"open","locked":false,"title":"Ragdoll Part 12: first pass avatar skeleton collides with voxels","user":{"login":"AndrewMeadows","id":2186405,"avatar_url":"https://avatars.githubusercontent.com/u/2186405?v=2","gravatar_id":"3108882716bb8dbfd1f990b6b04334b1","url":"https://api.github.com/users/AndrewMeadows","html_url":"https://github.com/AndrewMeadows","followers_url":"https://api.github.com/users/AndrewMeadows/followers","following_url":"https://api.github.com/users/AndrewMeadows/following{/other_user}","gists_url":"https://api.github.com/users/AndrewMeadows/gists{/gist_id}","starred_url":"https://api.github.com/users/AndrewMeadows/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/AndrewMeadows/subscriptions","organizations_url":"https://api.github.com/users/AndrewMeadows/orgs","repos_url":"https://api.github.com/users/AndrewMeadows/repos","events_url":"https://api.github.com/users/AndrewMeadows/events{/privacy}","received_events_url":"https://api.github.com/users/AndrewMeadows/received_events","type":"User","site_admin":false},"body":"The avatar skeleton now collides with voxels when ragdoll mode is enabled.  However, there are some known issues:\r\n\r\n(1) Skeleton shapes will collide with internal faces between adjacent voxels, so colliding against a row of small voxels will do strange interpenetrationary things.\r\n\r\n(2) The penetration resolution works independently for adjacent capsules in the skeleton, so what tends to happen is the tip of a finger might get pushed up out the top face of a voxel while lower joints are pushed out the front -- the result is somewhat chaotic when the constraints are enforced.  Typically the lower joint is heavier and more constrained (attached to the hand etc) and so the tip of the finger is pulled back to collide with the front of the voxel.  In short: when you drop a hand down on top of a voxel, near an edge, the hand will eventually be pushed out the side (with broken fingers) rather than resting on top.\r\n\r\n(3) The avatar's bounding capsule is not used for floor collisions when the ragdoll is colliding against voxels.  Intstead the avatar's feet and legs will collide against the floor.  It is kinda cool that this Just Works, but it causes two problems:\r\n\r\n(3a) Gravity is pulling the avatar skeleton down, collisions push the feet out, and constraints lift the rest of the body up.  The result is that there can be partial (and intermitent) collapse of the torso when there aren't enough iterations of the constraints to fully propagate up the chain of joints -- the torso will dip and weave a bit when the avatar just stands there.\r\n\r\n(3b) The avatar cannot walk up steps anymore.  There is no special handling of avatar foot collisions so the general penetration resolution between the feet and smaller voxels can be as much to the side as it is up -- the net result is that the legs will tend to be pushed akimbo rather than straight up when colliding against a platform made of small voxels.\r\n\r\nDunno yet if there are easy ways to solve all of these problems, but will think about them tonight.","created_at":"2014-09-03T23:34:11Z","updated_at":"2014-09-04T16:56:22Z","closed_at":null,"merged_at":null,"merge_commit_sha":"720be3bced2deb28a816cc46775b6234c1f0b727","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/highfidelity/hifi/pulls/3343/commits","review_comments_url":"https://api.github.com/repos/highfidelity/hifi/pulls/3343/comments","review_comment_url":"https://api.github.com/repos/highfidelity/hifi/pulls/comments/{number}","comments_url":"https://api.github.com/repos/highfidelity/hifi/issues/3343/comments","statuses_url":"https://api.github.com/repos/highfidelity/hifi/statuses/e120697a9b7f8fd0594629c4d84df2003a811d89","head":{"label":"AndrewMeadows:ragdoll","ref":"ragdoll","sha":"e120697a9b7f8fd0594629c4d84df2003a811d89","user":{"login":"AndrewMeadows","id":2186405,"avatar_url":"https://avatars.githubusercontent.com/u/2186405?v=2","gravatar_id":"3108882716bb8dbfd1f990b6b04334b1","url":"https://api.github.com/users/AndrewMeadows","html_url":"https://github.com/AndrewMeadows","followers_url":"https://api.github.com/users/AndrewMeadows/followers","following_url":"https://api.github.com/users/AndrewMeadows/following{/other_user}","gists_url":"https://api.github.com/users/AndrewMeadows/gists{/gist_id}","starred_url":"https://api.github.com/users/AndrewMeadows/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/AndrewMeadows/subscriptions","organizations_url":"https://api.github.com/users/AndrewMeadows/orgs","repos_url":"https://api.github.com/users/AndrewMeadows/repos","events_url":"https://api.github.com/users/AndrewMeadows/events{/privacy}","received_events_url":"https://api.github.com/users/AndrewMeadows/received_events","type":"User","site_admin":false},"repo":{"id":15350502,"name":"hifi","full_name":"AndrewMeadows/hifi","owner":{"login":"AndrewMeadows","id":2186405,"avatar_url":"https://avatars.githubusercontent.com/u/2186405?v=2","gravatar_id":"3108882716bb8dbfd1f990b6b04334b1","url":"https://api.github.com/users/AndrewMeadows","html_url":"https://github.com/AndrewMeadows","followers_url":"https://api.github.com/users/AndrewMeadows/followers","following_url":"https://api.github.com/users/AndrewMeadows/following{/other_user}","gists_url":"https://api.github.com/users/AndrewMeadows/gists{/gist_id}","starred_url":"https://api.github.com/users/AndrewMeadows/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/AndrewMeadows/subscriptions","organizations_url":"https://api.github.com/users/AndrewMeadows/orgs","repos_url":"https://api.github.com/users/AndrewMeadows/repos","events_url":"https://api.github.com/users/AndrewMeadows/events{/privacy}","received_events_url":"https://api.github.com/users/AndrewMeadows/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/AndrewMeadows/hifi","description":"Open, decentralized virtual world using sensors to control avatars and dynamically assigned devices as servers.  San Francisco based startup, we are hiring: http://highfidelity.io/jobs  You can also contribute by doing jobs listed at http://worklist.net/worklist.php","fork":true,"url":"https://api.github.com/repos/AndrewMeadows/hifi","forks_url":"https://api.github.com/repos/AndrewMeadows/hifi/forks","keys_url":"https://api.github.com/repos/AndrewMeadows/hifi/keys{/key_id}","collaborators_url":"https://api.github.com/repos/AndrewMeadows/hifi/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/AndrewMeadows/hifi/teams","hooks_url":"https://api.github.com/repos/AndrewMeadows/hifi/hooks","issue_events_url":"https://api.github.com/repos/AndrewMeadows/hifi/issues/events{/number}","events_url":"https://api.github.com/repos/AndrewMeadows/hifi/events","assignees_url":"https://api.github.com/repos/AndrewMeadows/hifi/assignees{/user}","branches_url":"https://api.github.com/repos/AndrewMeadows/hifi/branches{/branch}","tags_url":"https://api.github.com/repos/AndrewMeadows/hifi/tags","blobs_url":"https://api.github.com/repos/AndrewMeadows/hifi/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/AndrewMeadows/hifi/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/AndrewMeadows/hifi/git/refs{/sha}","trees_url":"https://api.github.com/repos/AndrewMeadows/hifi/git/trees{/sha}","statuses_url":"https://api.github.com/repos/AndrewMeadows/hifi/statuses/{sha}","languages_url":"https://api.github.com/repos/AndrewMeadows/hifi/languages","stargazers_url":"https://api.github.com/repos/AndrewMeadows/hifi/stargazers","contributors_url":"https://api.github.com/repos/AndrewMeadows/hifi/contributors","subscribers_url":"https://api.github.com/repos/AndrewMeadows/hifi/subscribers","subscription_url":"https://api.github.com/repos/AndrewMeadows/hifi/subscription","commits_url":"https://api.github.com/repos/AndrewMeadows/hifi/commits{/sha}","git_commits_url":"https://api.github.com/repos/AndrewMeadows/hifi/git/commits{/sha}","comments_url":"https://api.github.com/repos/AndrewMeadows/hifi/comments{/number}","issue_comment_url":"https://api.github.com/repos/AndrewMeadows/hifi/issues/comments/{number}","contents_url":"https://api.github.com/repos/AndrewMeadows/hifi/contents/{+path}","compare_url":"https://api.github.com/repos/AndrewMeadows/hifi/compare/{base}...{head}","merges_url":"https://api.github.com/repos/AndrewMeadows/hifi/merges","archive_url":"https://api.github.com/repos/AndrewMeadows/hifi/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/AndrewMeadows/hifi/downloads","issues_url":"https://api.github.com/repos/AndrewMeadows/hifi/issues{/number}","pulls_url":"https://api.github.com/repos/AndrewMeadows/hifi/pulls{/number}","milestones_url":"https://api.github.com/repos/AndrewMeadows/hifi/milestones{/number}","notifications_url":"https://api.github.com/repos/AndrewMeadows/hifi/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/AndrewMeadows/hifi/labels{/name}","releases_url":"https://api.github.com/repos/AndrewMeadows/hifi/releases{/id}","created_at":"2013-12-20T23:18:13Z","updated_at":"2014-08-25T00:12:09Z","pushed_at":"2014-09-04T16:53:42Z","git_url":"git://github.com/AndrewMeadows/hifi.git","ssh_url":"git@github.com:AndrewMeadows/hifi.git","clone_url":"https://github.com/AndrewMeadows/hifi.git","svn_url":"https://github.com/AndrewMeadows/hifi","homepage":"http://highfidelity.io","size":117798,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":false,"has_downloads":true,"has_wiki":true,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"highfidelity:master","ref":"master","sha":"bd55da4a8335d29683358f7abcbd252fa754faf8","user":{"login":"highfidelity","id":1565370,"avatar_url":"https://avatars.githubusercontent.com/u/1565370?v=2","gravatar_id":"46874b00c748488b5fa31e729cc27d94","url":"https://api.github.com/users/highfidelity","html_url":"https://github.com/highfidelity","followers_url":"https://api.github.com/users/highfidelity/followers","following_url":"https://api.github.com/users/highfidelity/following{/other_user}","gists_url":"https://api.github.com/users/highfidelity/gists{/gist_id}","starred_url":"https://api.github.com/users/highfidelity/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/highfidelity/subscriptions","organizations_url":"https://api.github.com/users/highfidelity/orgs","repos_url":"https://api.github.com/users/highfidelity/repos","events_url":"https://api.github.com/users/highfidelity/events{/privacy}","received_events_url":"https://api.github.com/users/highfidelity/received_events","type":"Organization","site_admin":false},"repo":{"id":5372872,"name":"hifi","full_name":"highfidelity/hifi","owner":{"login":"highfidelity","id":1565370,"avatar_url":"https://avatars.githubusercontent.com/u/1565370?v=2","gravatar_id":"46874b00c748488b5fa31e729cc27d94","url":"https://api.github.com/users/highfidelity","html_url":"https://github.com/highfidelity","followers_url":"https://api.github.com/users/highfidelity/followers","following_url":"https://api.github.com/users/highfidelity/following{/other_user}","gists_url":"https://api.github.com/users/highfidelity/gists{/gist_id}","starred_url":"https://api.github.com/users/highfidelity/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/highfidelity/subscriptions","organizations_url":"https://api.github.com/users/highfidelity/orgs","repos_url":"https://api.github.com/users/highfidelity/repos","events_url":"https://api.github.com/users/highfidelity/events{/privacy}","received_events_url":"https://api.github.com/users/highfidelity/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/highfidelity/hifi","description":"Open, decentralized virtual worlds using sensors to control avatars and dynamically assigned devices as servers.  San Francisco based startup, we are hiring: http://highfidelity.io/jobs  Contribute by reporting bugs & doing jobs on http://worklist.net","fork":false,"url":"https://api.github.com/repos/highfidelity/hifi","forks_url":"https://api.github.com/repos/highfidelity/hifi/forks","keys_url":"https://api.github.com/repos/highfidelity/hifi/keys{/key_id}","collaborators_url":"https://api.github.com/repos/highfidelity/hifi/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/highfidelity/hifi/teams","hooks_url":"https://api.github.com/repos/highfidelity/hifi/hooks","issue_events_url":"https://api.github.com/repos/highfidelity/hifi/issues/events{/number}","events_url":"https://api.github.com/repos/highfidelity/hifi/events","assignees_url":"https://api.github.com/repos/highfidelity/hifi/assignees{/user}","branches_url":"https://api.github.com/repos/highfidelity/hifi/branches{/branch}","tags_url":"https://api.github.com/repos/highfidelity/hifi/tags","blobs_url":"https://api.github.com/repos/highfidelity/hifi/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/highfidelity/hifi/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/highfidelity/hifi/git/refs{/sha}","trees_url":"https://api.github.com/repos/highfidelity/hifi/git/trees{/sha}","statuses_url":"https://api.github.com/repos/highfidelity/hifi/statuses/{sha}","languages_url":"https://api.github.com/repos/highfidelity/hifi/languages","stargazers_url":"https://api.github.com/repos/highfidelity/hifi/stargazers","contributors_url":"https://api.github.com/repos/highfidelity/hifi/contributors","subscribers_url":"https://api.github.com/repos/highfidelity/hifi/subscribers","subscription_url":"https://api.github.com/repos/highfidelity/hifi/subscription","commits_url":"https://api.github.com/repos/highfidelity/hifi/commits{/sha}","git_commits_url":"https://api.github.com/repos/highfidelity/hifi/git/commits{/sha}","comments_url":"https://api.github.com/repos/highfidelity/hifi/comments{/number}","issue_comment_url":"https://api.github.com/repos/highfidelity/hifi/issues/comments/{number}","contents_url":"https://api.github.com/repos/highfidelity/hifi/contents/{+path}","compare_url":"https://api.github.com/repos/highfidelity/hifi/compare/{base}...{head}","merges_url":"https://api.github.com/repos/highfidelity/hifi/merges","archive_url":"https://api.github.com/repos/highfidelity/hifi/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/highfidelity/hifi/downloads","issues_url":"https://api.github.com/repos/highfidelity/hifi/issues{/number}","pulls_url":"https://api.github.com/repos/highfidelity/hifi/pulls{/number}","milestones_url":"https://api.github.com/repos/highfidelity/hifi/milestones{/number}","notifications_url":"https://api.github.com/repos/highfidelity/hifi/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/highfidelity/hifi/labels{/name}","releases_url":"https://api.github.com/repos/highfidelity/hifi/releases{/id}","created_at":"2012-08-10T18:26:22Z","updated_at":"2014-09-04T00:37:43Z","pushed_at":"2014-09-04T16:30:30Z","git_url":"git://github.com/highfidelity/hifi.git","ssh_url":"git@github.com:highfidelity/hifi.git","clone_url":"https://github.com/highfidelity/hifi.git","svn_url":"https://github.com/highfidelity/hifi","homepage":"","size":173719,"stargazers_count":192,"watchers_count":192,"language":"C++","has_issues":false,"has_downloads":true,"has_wiki":true,"forks_count":117,"mirror_url":null,"open_issues_count":50,"forks":117,"open_issues":50,"watchers":192,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/highfidelity/hifi/pulls/3343"},"html":{"href":"https://github.com/highfidelity/hifi/pull/3343"},"issue":{"href":"https://api.github.com/repos/highfidelity/hifi/issues/3343"},"comments":{"href":"https://api.github.com/repos/highfidelity/hifi/issues/3343/comments"},"review_comments":{"href":"https://api.github.com/repos/highfidelity/hifi/pulls/3343/comments"},"review_comment":{"href":"https://api.github.com/repos/highfidelity/hifi/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/highfidelity/hifi/pulls/3343/commits"},"statuses":{"href":"https://api.github.com/repos/highfidelity/hifi/statuses/e120697a9b7f8fd0594629c4d84df2003a811d89"}}}}