Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--Enable saving of edited attributes configs to json #1416

Merged
merged 25 commits into from
Oct 7, 2021

Conversation

jturner65
Copy link
Contributor

@jturner65 jturner65 commented Jul 29, 2021

Motivation and Context

This PR enables the saving of user-modified JSON configuration files to disk. All the functionality is complete, and a test for reading and verifying physics manager attributes values passes. All tests that test json strings now also save configs based on those strings, and then reload them and verify they are unchanged in AttributesManagerTest.

How Has This Been Tested

Locally C++ and python tests

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jul 29, 2021
@jturner65 jturner65 force-pushed the MM_SaveConfigJSON branch 5 times, most recently from b5468dd to 6f95565 Compare August 11, 2021 19:30
@jturner65 jturner65 force-pushed the MM_SaveConfigJSON branch 5 times, most recently from 2e6eacc to bdc806e Compare August 17, 2021 22:47
@jturner65 jturner65 force-pushed the MM_SaveConfigJSON branch 5 times, most recently from 18aaf59 to 725eb31 Compare August 26, 2021 18:39
@jturner65 jturner65 force-pushed the MM_SaveConfigJSON branch 3 times, most recently from 8dd4633 to 4b23541 Compare August 31, 2021 13:11
@jturner65 jturner65 changed the title [WIP] Enable saving of edited attributes configs to json Enable saving of edited attributes configs to json Aug 31, 2021
@jturner65 jturner65 requested review from eundersander and aclegg3 and removed request for aclegg3 August 31, 2021 19:59
@jturner65 jturner65 marked this pull request as ready for review September 1, 2021 11:19
@jturner65 jturner65 force-pushed the MM_SaveConfigJSON branch 3 times, most recently from e00c5b5 to 4354aa5 Compare September 3, 2021 09:27
@jturner65 jturner65 changed the title Enable saving of edited attributes configs to json --Enable saving of edited attributes configs to json Sep 9, 2021
@jturner65 jturner65 changed the title [WIP]--Enable saving of edited attributes configs to json --Enable saving of edited attributes configs to json Oct 7, 2021
Copy link
Contributor

@aclegg3 aclegg3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@@ -156,6 +157,11 @@ class ConfigValue {

bool isValid() const { return _type != ConfigStoredType::Unknown; }

/**
* @brief Write this ConfigValue to an appropriately configure json object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo?

@jturner65 jturner65 merged commit 3b90479 into facebookresearch:main Oct 7, 2021
@jturner65 jturner65 deleted the MM_SaveConfigJSON branch October 7, 2021 21:46
JuanTheEngineer pushed a commit that referenced this pull request Oct 8, 2021
* --move AbstractFileBasedManagedObject to its own file
* --add function stubs to save a ManagedFileBasedObject to JSON
* --appropriately configure document for saving.
* --enable retrieval of File-based attributes extension from manager.
* --properly write json files to disk.
* --store spotlight values in subconfiguration of light instances
* --move Configuration json load/save code from io::json to Configuration
* --update test to ref actual files for stage & object render asset verify
* break up json config mapping to have separate value and config procs
This enables root-level attributes to have custom handling so they only write what is expected to json.

* --move mesh-type handling to attributesBase
* --fix orientation bug.  write value to json with different tag than key
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants