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

NavMeshSettings JSON Serialization #1883

Merged
merged 11 commits into from
Oct 7, 2022
Merged

NavMeshSettings JSON Serialization #1883

merged 11 commits into from
Oct 7, 2022

Conversation

aclegg3
Copy link
Contributor

@aclegg3 aclegg3 commented Sep 30, 2022

Motivation and Context

With expanding NavMesh use-cases it becomes useful to cache non-default settings for later reload.

Example use cases include configuring the NavMesh to generate receptacle surfaces for placing objects of a particular size or caching NavMesh parameters for a particular robot/agent embodiment. Once cached, these settings could be easily re-used in any scene/stage.

How Has This Been Tested

Added a C++ unit test.

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 Sep 30, 2022
Copy link
Contributor

@Skylion007 Skylion007 left a comment

Choose a reason for hiding this comment

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

Other than a minor typo looks good. Not sure JSON is the best configuration serialization for this longterm, but we are already abusing it so might as well keep doing so.

aclegg3 and others added 7 commits October 5, 2022 11:19
Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>
… crosslinking (#1889)

* docs: update Corrade and Magnum tagfiles so docs can link to new APIs.

* docs: add convenience Doxygen aliases for inline code highlighting.

* docs: update Magnum Bindings inventory file for Python crosslinking.
* Remove unused and wrong Magnum typedefs.

Those have a hardcoded assumption that any mesh is a GL mesh, and that
any material is a Phong material. Which is kinda true now, but it won't
be going forward.

Moreover, they weren't really used anywhere anyway, so there's no point
in having them at all.

* Don't include all of Magnum everywhere for no reason.

The magnum.h header now contains SceneGraph forward declarations
typedefs and only that. It doesn't and shouldn't pull in also GL or
Trade stuff.

* Contain Eigen into a separate header, not the main main header.

It's still pulled into far too many places, but this way it can at least
start getting gradually removed.

* Use Magnum's forward declaration headers, not manual declarations.

* Don't include MetadataMediator in Simulator.h and ResourceManager.h.

Not exactly sure what is it doing, but it's quite heavy. Unfortunately
most of the tests need it to set things up anyway, so it only helps
partially.

Also, the ::ptr typedefs are nice and all, but they mean one has to
include the whole class definition. Which is not necessary in 99% of
cases, so I'm converting T::ptr to the verbose std::shared_ptr<T> where
a forward declaration is enough.

* Remove most includes from ResourceManager.h.

Wow, imagine changing anything in any of those headers. Boom, everything
gets recompiled.

* Stop including ResourceManager from all headers.

* This is not the right way to create a string constant.

The static will *copy* the string into each and every object file that
directly or indirectly includes Asset.h.

* Disable a useless Clang Tidy check.

* Fix definition of the global logging context variable.

It only worked so far because <Magnum/configure.h> was not included in
Logging.cpp and so MAGNUM_BUILD_STATIC_UNIQUE_GLOBALS wasn't defined.
Now it is, and the anonymous namespace is in direct conflict with the
export macro.
* Habitat-lab requirements.txt has been moved to habitat-lab/habitat-lab/ directory

* update requirements path in colab installer

* change install from setup.py to pip

* install mock manually in CI
The warning note was good enough already, but apparently those two were
never used so far so they still had the dark theme colors.

Also updated m.css to latest, and some other Doxyfile additions.
@aclegg3 aclegg3 merged commit e7e8b11 into main Oct 7, 2022
@aclegg3 aclegg3 deleted the json-navmeshsettings branch October 7, 2022 14:44
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.

None yet

5 participants