{"action":"created","comment":{"url":"https://api.github.com/repos/opensim-org/opensim-core/pulls/comments/221781011","pull_request_review_id":160514778,"id":221781011,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIyMTc4MTAxMQ==","diff_hunk":"@@ -84,7 +84,7 @@ OpenSim_DECLARE_CONCRETE_OBJECT(ExternalLoads,\n     /* If point of applications for external forces must be re-expressed\n        then build new storages to be assigned to the individual ExternalForces\n        with the transformed point data. Hang-on to them so we can delete them. */\n-    ArrayPtrs<Storage> _storages;\n+    std::vector<std::shared_ptr<Storage>> _storages;","path":"OpenSim/Simulation/Model/ExternalLoads.h","position":5,"original_position":5,"commit_id":"60ed6894d1d20b79580d7cfeccf2a18c667c0b25","original_commit_id":"c84a8d0d7b0ed5c11acf9bdcd452536210017c0b","user":{"login":"aseth1","id":6494122,"node_id":"MDQ6VXNlcjY0OTQxMjI=","avatar_url":"https://avatars0.githubusercontent.com/u/6494122?v=4","gravatar_id":"","url":"https://api.github.com/users/aseth1","html_url":"https://github.com/aseth1","followers_url":"https://api.github.com/users/aseth1/followers","following_url":"https://api.github.com/users/aseth1/following{/other_user}","gists_url":"https://api.github.com/users/aseth1/gists{/gist_id}","starred_url":"https://api.github.com/users/aseth1/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aseth1/subscriptions","organizations_url":"https://api.github.com/users/aseth1/orgs","repos_url":"https://api.github.com/users/aseth1/repos","events_url":"https://api.github.com/users/aseth1/events{/privacy}","received_events_url":"https://api.github.com/users/aseth1/received_events","type":"User","site_admin":false},"body":"I used `shared_ptr` because I didn't want to re-specify the data source after copying or pay the price of copying potentially large data sets.  It this case it is the `ExternalLoads` itself that is creating the new data source based on transforming the point of force application. This should be an operation done outside of `ExternalLoads` and the data source reset. It could then be easily reassigned to a clone, but as it stands now, the clone is added to the `Model` and it would require more widespread changes that I thought would be going even further beyond the scope of this PR. This could be addressed as part of a more general `ExternalLoads` refactor. As it stands, I don't think there is an issue of leaking.","created_at":"2018-10-01T22:49:59Z","updated_at":"2018-10-01T22:49:59Z","html_url":"https://github.com/opensim-org/opensim-core/pull/2285#discussion_r221781011","pull_request_url":"https://api.github.com/repos/opensim-org/opensim-core/pulls/2285","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/opensim-org/opensim-core/pulls/comments/221781011"},"html":{"href":"https://github.com/opensim-org/opensim-core/pull/2285#discussion_r221781011"},"pull_request":{"href":"https://api.github.com/repos/opensim-org/opensim-core/pulls/2285"}},"in_reply_to_id":221699458},"pull_request":{"url":"https://api.github.com/repos/opensim-org/opensim-core/pulls/2285","id":214453597,"node_id":"MDExOlB1bGxSZXF1ZXN0MjE0NDUzNTk3","html_url":"https://github.com/opensim-org/opensim-core/pull/2285","diff_url":"https://github.com/opensim-org/opensim-core/pull/2285.diff","patch_url":"https://github.com/opensim-org/opensim-core/pull/2285.patch","issue_url":"https://api.github.com/repos/opensim-org/opensim-core/issues/2285","number":2285,"state":"open","locked":false,"title":"Make ModelComponentSet a ModelComponent","user":{"login":"aseth1","id":6494122,"node_id":"MDQ6VXNlcjY0OTQxMjI=","avatar_url":"https://avatars0.githubusercontent.com/u/6494122?v=4","gravatar_id":"","url":"https://api.github.com/users/aseth1","html_url":"https://github.com/aseth1","followers_url":"https://api.github.com/users/aseth1/followers","following_url":"https://api.github.com/users/aseth1/following{/other_user}","gists_url":"https://api.github.com/users/aseth1/gists{/gist_id}","starred_url":"https://api.github.com/users/aseth1/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aseth1/subscriptions","organizations_url":"https://api.github.com/users/aseth1/orgs","repos_url":"https://api.github.com/users/aseth1/repos","events_url":"https://api.github.com/users/aseth1/events{/privacy}","received_events_url":"https://api.github.com/users/aseth1/received_events","type":"User","site_admin":false},"body":"Addresses a significant cause of the issues in #2024, #2121 and https://github.com/opensim-org/opensim-gui/issues/483 that stem from the strictly enforced requirement that all Components have unique pathnames. This fix allows us to use old model files with a `Body`, `Joint` and even a `Marker` sharing the same name as long as they live in different `ModelComponentSet`s. For example, what used to require renaming for having a *toes* ` Joint` and *toes* `Body` (which also broke the `Socket` connection to the *toes* Body because it was renamed) are now both unique as */model/jointset/toes* and */model/bodyset/toes*. As a consequence many older model will have fewer renaming and connection issues.\r\n\r\nAs part of the serialization fix, it makes Ground's name unchangable, which also addresses  #1876 (3.).\r\n\r\n### Brief summary of changes\r\n1. Made a `Set<ObjectTyp, DerivedType>` mixin so that the Set interface would not have to be duplicated for Sets of objects and Sets of Components.\r\n\r\n2. `Object` Sets default to` Set<T, Object>` which is exactly what it was before. Thus, FunctionSet, ScaleSet, etc... are unchanged.\r\n\r\n3. `Component` Sets and `ModelComponent` Sets now derive from ` Set<T, Component>` and ` Set<T, ModelComponent>` respectively.\r\n\r\n4. Gutted `ModelComponentSet` and  derived implementations since they no longer need custom setup and model connection implementations since that machinery is provided by `Component` and `ModelComponent` interfaces.\r\n\r\n5. Removed a KLUDGE in `Property` writing that would change the name of the `Component` (`Object`) during printing (which is supposed to be const!) that caused `Set` names to be overwritten and resulted in-memory and serialized->deserialized models to differ. Now `Object` XML writing (node updating) is responsive to the Property being *isUnnamedProperty* and logic  was added to decide when it is appropriate to use the Property name in place of the `Object` name. \r\n\r\n6. `ExternalLoads` and its convoluted handling by the Tools was reworked so that is just added to the `Model` like any `ModelComponent` and it is configured by overriding the extension of the Component interface. For example, Tools no longer have to manually add the individual `ExternalForces` to the `Model`. Much more could be done to separate `ExternalLoads` from Tools, but as it stands these changes do not effect Tool interface or existing setup files.\r\n\r\n7.  The bulk of the remaining changes are updates to component access to include the intermediate `ModelComponentSet`s in their pathnames and similarly for finding accessing state variable values by name. In many instance, asking the Component for its stateVariableNames() was a lot simpler.\r\n\r\n### Testing I've completed\r\n~~More than 80% of the tests pass from `Object` serialization, `Component` interface up to most of the Tools, but the following fail:~~\r\n\r\n~~Java bindings fail to build due to:~~\r\n\r\nUPDATE 9/20/2018 Merged master into branch. All tests pass locally.\r\n\r\n### Looking for feedback on...\r\n\r\n1. Changes to `Set` and its impact on Java and Python bindings. @chrisdembia @aymanhab \r\n2. Any other potential fallout from removing the KLUDGE to Object/Property serialization? @aymanhab\r\n\r\n### CHANGELOG.md (choose one)\r\n\r\n- no need to update because...\r\n- updated...\r\n\r\nThe Doxygen for this PR can be viewed at http://myosin.sourceforge.net/?C=N;O=D; click the folder whose name is this PR's number.\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/opensim-org/opensim-core/2285)\n<!-- Reviewable:end -->\n","created_at":"2018-09-10T21:46:09Z","updated_at":"2018-10-01T22:49:59Z","closed_at":null,"merged_at":null,"merge_commit_sha":"18981d9bbe9a05116dc59d419147f9698b0ef5e9","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/opensim-org/opensim-core/pulls/2285/commits","review_comments_url":"https://api.github.com/repos/opensim-org/opensim-core/pulls/2285/comments","review_comment_url":"https://api.github.com/repos/opensim-org/opensim-core/pulls/comments{/number}","comments_url":"https://api.github.com/repos/opensim-org/opensim-core/issues/2285/comments","statuses_url":"https://api.github.com/repos/opensim-org/opensim-core/statuses/60ed6894d1d20b79580d7cfeccf2a18c667c0b25","head":{"label":"opensim-org:make_component_sets_components","ref":"make_component_sets_components","sha":"60ed6894d1d20b79580d7cfeccf2a18c667c0b25","user":{"login":"opensim-org","id":4370782,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQzNzA3ODI=","avatar_url":"https://avatars0.githubusercontent.com/u/4370782?v=4","gravatar_id":"","url":"https://api.github.com/users/opensim-org","html_url":"https://github.com/opensim-org","followers_url":"https://api.github.com/users/opensim-org/followers","following_url":"https://api.github.com/users/opensim-org/following{/other_user}","gists_url":"https://api.github.com/users/opensim-org/gists{/gist_id}","starred_url":"https://api.github.com/users/opensim-org/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/opensim-org/subscriptions","organizations_url":"https://api.github.com/users/opensim-org/orgs","repos_url":"https://api.github.com/users/opensim-org/repos","events_url":"https://api.github.com/users/opensim-org/events{/privacy}","received_events_url":"https://api.github.com/users/opensim-org/received_events","type":"Organization","site_admin":false},"repo":{"id":20775600,"node_id":"MDEwOlJlcG9zaXRvcnkyMDc3NTYwMA==","name":"opensim-core","full_name":"opensim-org/opensim-core","private":false,"owner":{"login":"opensim-org","id":4370782,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQzNzA3ODI=","avatar_url":"https://avatars0.githubusercontent.com/u/4370782?v=4","gravatar_id":"","url":"https://api.github.com/users/opensim-org","html_url":"https://github.com/opensim-org","followers_url":"https://api.github.com/users/opensim-org/followers","following_url":"https://api.github.com/users/opensim-org/following{/other_user}","gists_url":"https://api.github.com/users/opensim-org/gists{/gist_id}","starred_url":"https://api.github.com/users/opensim-org/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/opensim-org/subscriptions","organizations_url":"https://api.github.com/users/opensim-org/orgs","repos_url":"https://api.github.com/users/opensim-org/repos","events_url":"https://api.github.com/users/opensim-org/events{/privacy}","received_events_url":"https://api.github.com/users/opensim-org/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/opensim-org/opensim-core","description":"SimTK OpenSim 4.0 C++ libraries and command-line applications, and Java/Python wrapping.","fork":false,"url":"https://api.github.com/repos/opensim-org/opensim-core","forks_url":"https://api.github.com/repos/opensim-org/opensim-core/forks","keys_url":"https://api.github.com/repos/opensim-org/opensim-core/keys{/key_id}","collaborators_url":"https://api.github.com/repos/opensim-org/opensim-core/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/opensim-org/opensim-core/teams","hooks_url":"https://api.github.com/repos/opensim-org/opensim-core/hooks","issue_events_url":"https://api.github.com/repos/opensim-org/opensim-core/issues/events{/number}","events_url":"https://api.github.com/repos/opensim-org/opensim-core/events","assignees_url":"https://api.github.com/repos/opensim-org/opensim-core/assignees{/user}","branches_url":"https://api.github.com/repos/opensim-org/opensim-core/branches{/branch}","tags_url":"https://api.github.com/repos/opensim-org/opensim-core/tags","blobs_url":"https://api.github.com/repos/opensim-org/opensim-core/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/opensim-org/opensim-core/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/opensim-org/opensim-core/git/refs{/sha}","trees_url":"https://api.github.com/repos/opensim-org/opensim-core/git/trees{/sha}","statuses_url":"https://api.github.com/repos/opensim-org/opensim-core/statuses/{sha}","languages_url":"https://api.github.com/repos/opensim-org/opensim-core/languages","stargazers_url":"https://api.github.com/repos/opensim-org/opensim-core/stargazers","contributors_url":"https://api.github.com/repos/opensim-org/opensim-core/contributors","subscribers_url":"https://api.github.com/repos/opensim-org/opensim-core/subscribers","subscription_url":"https://api.github.com/repos/opensim-org/opensim-core/subscription","commits_url":"https://api.github.com/repos/opensim-org/opensim-core/commits{/sha}","git_commits_url":"https://api.github.com/repos/opensim-org/opensim-core/git/commits{/sha}","comments_url":"https://api.github.com/repos/opensim-org/opensim-core/comments{/number}","issue_comment_url":"https://api.github.com/repos/opensim-org/opensim-core/issues/comments{/number}","contents_url":"https://api.github.com/repos/opensim-org/opensim-core/contents/{+path}","compare_url":"https://api.github.com/repos/opensim-org/opensim-core/compare/{base}...{head}","merges_url":"https://api.github.com/repos/opensim-org/opensim-core/merges","archive_url":"https://api.github.com/repos/opensim-org/opensim-core/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/opensim-org/opensim-core/downloads","issues_url":"https://api.github.com/repos/opensim-org/opensim-core/issues{/number}","pulls_url":"https://api.github.com/repos/opensim-org/opensim-core/pulls{/number}","milestones_url":"https://api.github.com/repos/opensim-org/opensim-core/milestones{/number}","notifications_url":"https://api.github.com/repos/opensim-org/opensim-core/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/opensim-org/opensim-core/labels{/name}","releases_url":"https://api.github.com/repos/opensim-org/opensim-core/releases{/id}","deployments_url":"https://api.github.com/repos/opensim-org/opensim-core/deployments","created_at":"2014-06-12T16:57:56Z","updated_at":"2018-09-27T19:27:10Z","pushed_at":"2018-10-01T20:10:30Z","git_url":"git://github.com/opensim-org/opensim-core.git","ssh_url":"git@github.com:opensim-org/opensim-core.git","clone_url":"https://github.com/opensim-org/opensim-core.git","svn_url":"https://github.com/opensim-org/opensim-core","homepage":"https://opensim.stanford.edu","size":146638,"stargazers_count":169,"watchers_count":169,"language":"C++","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":113,"mirror_url":null,"archived":false,"open_issues_count":521,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0","node_id":"MDc6TGljZW5zZTI="},"forks":113,"open_issues":521,"watchers":169,"default_branch":"master"}},"base":{"label":"opensim-org:master","ref":"master","sha":"9d250363aa15e618657f85d391b0a40540863ee9","user":{"login":"opensim-org","id":4370782,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQzNzA3ODI=","avatar_url":"https://avatars0.githubusercontent.com/u/4370782?v=4","gravatar_id":"","url":"https://api.github.com/users/opensim-org","html_url":"https://github.com/opensim-org","followers_url":"https://api.github.com/users/opensim-org/followers","following_url":"https://api.github.com/users/opensim-org/following{/other_user}","gists_url":"https://api.github.com/users/opensim-org/gists{/gist_id}","starred_url":"https://api.github.com/users/opensim-org/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/opensim-org/subscriptions","organizations_url":"https://api.github.com/users/opensim-org/orgs","repos_url":"https://api.github.com/users/opensim-org/repos","events_url":"https://api.github.com/users/opensim-org/events{/privacy}","received_events_url":"https://api.github.com/users/opensim-org/received_events","type":"Organization","site_admin":false},"repo":{"id":20775600,"node_id":"MDEwOlJlcG9zaXRvcnkyMDc3NTYwMA==","name":"opensim-core","full_name":"opensim-org/opensim-core","private":false,"owner":{"login":"opensim-org","id":4370782,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQzNzA3ODI=","avatar_url":"https://avatars0.githubusercontent.com/u/4370782?v=4","gravatar_id":"","url":"https://api.github.com/users/opensim-org","html_url":"https://github.com/opensim-org","followers_url":"https://api.github.com/users/opensim-org/followers","following_url":"https://api.github.com/users/opensim-org/following{/other_user}","gists_url":"https://api.github.com/users/opensim-org/gists{/gist_id}","starred_url":"https://api.github.com/users/opensim-org/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/opensim-org/subscriptions","organizations_url":"https://api.github.com/users/opensim-org/orgs","repos_url":"https://api.github.com/users/opensim-org/repos","events_url":"https://api.github.com/users/opensim-org/events{/privacy}","received_events_url":"https://api.github.com/users/opensim-org/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/opensim-org/opensim-core","description":"SimTK OpenSim 4.0 C++ libraries and command-line applications, and Java/Python wrapping.","fork":false,"url":"https://api.github.com/repos/opensim-org/opensim-core","forks_url":"https://api.github.com/repos/opensim-org/opensim-core/forks","keys_url":"https://api.github.com/repos/opensim-org/opensim-core/keys{/key_id}","collaborators_url":"https://api.github.com/repos/opensim-org/opensim-core/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/opensim-org/opensim-core/teams","hooks_url":"https://api.github.com/repos/opensim-org/opensim-core/hooks","issue_events_url":"https://api.github.com/repos/opensim-org/opensim-core/issues/events{/number}","events_url":"https://api.github.com/repos/opensim-org/opensim-core/events","assignees_url":"https://api.github.com/repos/opensim-org/opensim-core/assignees{/user}","branches_url":"https://api.github.com/repos/opensim-org/opensim-core/branches{/branch}","tags_url":"https://api.github.com/repos/opensim-org/opensim-core/tags","blobs_url":"https://api.github.com/repos/opensim-org/opensim-core/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/opensim-org/opensim-core/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/opensim-org/opensim-core/git/refs{/sha}","trees_url":"https://api.github.com/repos/opensim-org/opensim-core/git/trees{/sha}","statuses_url":"https://api.github.com/repos/opensim-org/opensim-core/statuses/{sha}","languages_url":"https://api.github.com/repos/opensim-org/opensim-core/languages","stargazers_url":"https://api.github.com/repos/opensim-org/opensim-core/stargazers","contributors_url":"https://api.github.com/repos/opensim-org/opensim-core/contributors","subscribers_url":"https://api.github.com/repos/opensim-org/opensim-core/subscribers","subscription_url":"https://api.github.com/repos/opensim-org/opensim-core/subscription","commits_url":"https://api.github.com/repos/opensim-org/opensim-core/commits{/sha}","git_commits_url":"https://api.github.com/repos/opensim-org/opensim-core/git/commits{/sha}","comments_url":"https://api.github.com/repos/opensim-org/opensim-core/comments{/number}","issue_comment_url":"https://api.github.com/repos/opensim-org/opensim-core/issues/comments{/number}","contents_url":"https://api.github.com/repos/opensim-org/opensim-core/contents/{+path}","compare_url":"https://api.github.com/repos/opensim-org/opensim-core/compare/{base}...{head}","merges_url":"https://api.github.com/repos/opensim-org/opensim-core/merges","archive_url":"https://api.github.com/repos/opensim-org/opensim-core/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/opensim-org/opensim-core/downloads","issues_url":"https://api.github.com/repos/opensim-org/opensim-core/issues{/number}","pulls_url":"https://api.github.com/repos/opensim-org/opensim-core/pulls{/number}","milestones_url":"https://api.github.com/repos/opensim-org/opensim-core/milestones{/number}","notifications_url":"https://api.github.com/repos/opensim-org/opensim-core/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/opensim-org/opensim-core/labels{/name}","releases_url":"https://api.github.com/repos/opensim-org/opensim-core/releases{/id}","deployments_url":"https://api.github.com/repos/opensim-org/opensim-core/deployments","created_at":"2014-06-12T16:57:56Z","updated_at":"2018-09-27T19:27:10Z","pushed_at":"2018-10-01T20:10:30Z","git_url":"git://github.com/opensim-org/opensim-core.git","ssh_url":"git@github.com:opensim-org/opensim-core.git","clone_url":"https://github.com/opensim-org/opensim-core.git","svn_url":"https://github.com/opensim-org/opensim-core","homepage":"https://opensim.stanford.edu","size":146638,"stargazers_count":169,"watchers_count":169,"language":"C++","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":113,"mirror_url":null,"archived":false,"open_issues_count":521,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0","node_id":"MDc6TGljZW5zZTI="},"forks":113,"open_issues":521,"watchers":169,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/opensim-org/opensim-core/pulls/2285"},"html":{"href":"https://github.com/opensim-org/opensim-core/pull/2285"},"issue":{"href":"https://api.github.com/repos/opensim-org/opensim-core/issues/2285"},"comments":{"href":"https://api.github.com/repos/opensim-org/opensim-core/issues/2285/comments"},"review_comments":{"href":"https://api.github.com/repos/opensim-org/opensim-core/pulls/2285/comments"},"review_comment":{"href":"https://api.github.com/repos/opensim-org/opensim-core/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/opensim-org/opensim-core/pulls/2285/commits"},"statuses":{"href":"https://api.github.com/repos/opensim-org/opensim-core/statuses/60ed6894d1d20b79580d7cfeccf2a18c667c0b25"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"aseth1"}}