microsoft / MixedRealityToolkit-Unity

This repository is for the legacy Mixed Reality Toolkit (MRTK) v2. For the latest version of the MRTK please visit https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity
https://aka.ms/mrtkdocs
MIT License
6.01k stars 2.12k forks source link

Some rotation handles do not work in HandInteractionExamples #8555

Closed MaxWang-MS closed 4 years ago

MaxWang-MS commented 4 years ago

Describe the bug

Some rotation handles on certain GameObjects (i.e. the HL2 Platonic and coffee cup in the bounding box section) in the HandInteractionExamples scene are not working. When trying to use the horizontal rotation handle to rotate the GameObject around the x axis, the GameObject refuses to rotate around that axis. Further inspection showed that the rotation axis constraint script is causing this behavior.

Also related: different behaviors are observed when constraint on rotation is set to Nothing and None. Behavior of the horizontal rotation handle is correct when Nothing is used but wrong when None is used. Unity allows setting both None and x axis at the same time for the constraint, which could be a problem as well.

To reproduce

  1. Open HandInteractionExamples.unity
  2. Press play
  3. Try rotating the coffee cup in the bounding box section around x axis by moving the horizontal rotation handle
  4. See error

Expected behavior

The GameObject should be able to rotate around the x axis or the corresponding handles should not be shown.

Your setup

wiwei commented 4 years ago

Added to 2.5 release, this seems like a fairly clear thing that should work (i.e. especially in our examples)

MaxWang-MS commented 4 years ago

I think this is related to this #8190. It seems that the newly added None confuses Unity as Unity already provides a "Nothing". From the debugger I noticed a "one bit shift" of flag's value (i.e. set the flag to x axis which should be 1 but the debugger showed y axis which is 2) likely due to the "None".

thalbern commented 4 years ago

hmm those constraints probably were meant to be for object manipulator - might have to change constraint manager a bit so we can selectively choose which component gets which constraint when there's two supporting the constraint manager

thalbern commented 4 years ago

regarding that flag change - that is actually unrelated to that constraint problem - there's no option unlike in unreal where you could hide a certain enum value from the editor - we have the same situation in other spots as well - however this should be only a cosmetic issue / not interfer with the functionality - in case none and any other flag is selected it should just ignore the none value as it's mapped to zero. We can either change our code that we're returning ints instead of that enum to make the android build shut up and remove the none again or we can write custom editors for the enum types that include a none option and are defined as flags (or third option, we just leave it as is - which is what i would do :D)

thalbern commented 4 years ago

just realized I was wrong with above comment - max is right and that added None value is shifting the indices in the editor and as a result breaking any assets that were already saved before this None was added. I'm looking into fixing this, but have to understand first why this would break the android build. - However this is not related to the other constraint manager problem (this issue reports 2 separate bugs)

thalbern commented 4 years ago

Both issues should be fixed now - however I had to reopen the linked ticket for the flags issue on android as I had to revert the applied patch.