Skip to content
This repository has been archived by the owner on Aug 11, 2024. It is now read-only.

Refactor of boundary system #624

Merged
merged 23 commits into from
Jun 12, 2020

Conversation

StephenHodgson
Copy link
Contributor

@StephenHodgson StephenHodgson commented Jun 2, 2020

XRTK - Mixed Reality Toolkit Pull Request

Overview

This PR refactors the Boundary System as defined in #623

Breaking Changes

  • Removed IMixedRealityBoundaryHandler
  • IMixedRealityBoundarySystem no longer implements IMixedRealityEventSource
  • Removed IMixedRealityBoundarySystem.FloorScale
  • Removed IMixedRealityBoundarySystem.FloorPhysicsLayer
  • Removed IMixedRealityBoundarySystem.FloorMaterial
  • Removed IMixedRealityBoundarySystem.ShowPlayArea
  • Removed IMixedRealityBoundarySystem.PlayAreaMaterial
  • Removed IMixedRealityBoundarySystem.PlayAreaPhysicsLayer
  • Removed IMixedRealityBoundarySystem.ShowTrackedArea
  • Removed IMixedRealityBoundarySystem.TrackedAreaMaterial
  • Removed IMixedRealityBoundarySystem.TrackedAreaPhysicsLayer
  • Removed IMixedRealityBoundarySystem.BoundaryWallMaterial
  • Removed IMixedRealityBoundarySystem.BoundaryWallsPhysicsLayer
  • Removed IMixedRealityBoundarySystem.BoundaryCeilingMaterial
  • Removed IMixedRealityBoundarySystem.CeilingPhysicsLayer
  • Removed IMixedRealityBoundarySystem.GetFloorVisualization
  • Removed IMixedRealityBoundarySystem.GetPlayAreaVisualization
  • Removed IMixedRealityBoundarySystem.GetTrackedAreaVisualization
  • Removed IMixedRealityBoundarySystem.GetBoundaryWallVisualization
  • Added IMixedRealityBoundarySystem.BoundaryProximityAlert
  • Added IMixedRealityBoundarySystem.TrackedObjects
  • Added IMixedRealityBoundarySystem.BoundaryDataProvider
  • Added IMixedRealityBoundarySystem.IsVisible
  • Added IMixedRealityBoundarySystem.IsConfigured
  • Added IMixedRealityBoundarySystem.ShowBoundary
  • Added IMixedRealityBoundarySystem.RegisterTrackedObject
  • Added IMixedRealityBoundarySystem.UnregisterTrackedObject
  • Renamed IMixedRealityBoundarySystem.Bounds -> IMixedRealityBoundarySystem.BoundaryBounds
  • Renamed IMixedRealityBoundarySystem.Contains -> IMixedRealityBoundarySystem.IsInsideBoundary
  • Renamed IMixedRealityBoundarySystem.ShowBoundaryWalls -> IMixedRealityBoundarySystem.ShowWalls
  • Renamed IMixedRealityBoundarySystem.ShowBoundaryCeiling -> IMixedRealityBoundarySystem.ShowCeiling

Submodule Changes

@StephenHodgson StephenHodgson added Breaking Change In Progress PR currently still being developed labels Jun 2, 2020
@StephenHodgson StephenHodgson self-assigned this Jun 2, 2020
@StephenHodgson StephenHodgson marked this pull request as ready for review June 2, 2020 19:35
@StephenHodgson StephenHodgson added Ready for review PR finished primary development, open for review and removed In Progress PR currently still being developed labels Jun 2, 2020
@FejZa
Copy link
Contributor

FejZa commented Jun 3, 2020

I can't see where the new ProximityAlert event is being raised.

@StephenHodgson
Copy link
Contributor Author

Ah seems like I only raise it once place so far. I think I stopped cause I didn't have any platform data providers implemented yet.

Copy link
Contributor

@SimonDarksideJ SimonDarksideJ left a comment

Choose a reason for hiding this comment

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

Need to ensure unimplemented elements are either documented or removed.

@FejZa
Copy link
Contributor

FejZa commented Jun 3, 2020

Ah sorry it was my mistake. The diffs in the file were too many so I had to explicitly click "load diffs". During my first pass I skipped that file by mistake.

Good work so far. Let's wait for the submodule PRs / data providers to see the whole picture.

@StephenHodgson
Copy link
Contributor Author

Good work so far. Let's wait for the submodule PRs / data providers to see the whole picture.

WMR data provider is ready, but I didn't include it in the PR bc I wanna wait until our native API push to implement

@StephenHodgson StephenHodgson added In Progress PR currently still being developed and removed Ready for review PR finished primary development, open for review labels Jun 4, 2020
@StephenHodgson StephenHodgson added Ready for review PR finished primary development, open for review and removed In Progress PR currently still being developed labels Jun 11, 2020
@StephenHodgson
Copy link
Contributor Author

I updated this PR. Should be ready to review again.

FejZa
FejZa previously approved these changes Jun 11, 2020
@StephenHodgson
Copy link
Contributor Author

BTW the re-writer error is harmless. BUT our Windows64 build went from 7 min to 42 min 👀

You guys see anything that would make that happen?

@StephenHodgson StephenHodgson requested a review from FejZa June 12, 2020 07:10
@SimonDarksideJ
Copy link
Contributor

Is this still not ready as the dependant PR's are still in draft?

@StephenHodgson
Copy link
Contributor Author

It's a breaking change. The wmr submodule or cannot be reviewed until this PR is merged and a preview sent out.

@StephenHodgson StephenHodgson merged commit 632b84d into development Jun 12, 2020
@StephenHodgson StephenHodgson deleted the feature/boundary-system-data-providers branch June 12, 2020 14:19
XRTK-Build-Bot pushed a commit that referenced this pull request Dec 25, 2020
* initial refactor of boundary system

* sorted some properties

* updated examples checkout

* changed wmr checkout back to development

* updated examples

* fixed examples checkout

* test wmr build

* revert

* Added warning when geometry fails to be generated

* try setting the timeout to 2 hours

* updated wmr checkout with functional boundary data provider

* updated boundary tracked object logic

Added BoundsCache
updated Bounds Extension methods

* fix wait for log to be created

* updated examples checkout
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Breaking Change Ready for review PR finished primary development, open for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants