MixedRealityToolkit / MixedRealityToolkit-Unity

This repository holds the third generation of the Mixed Reality Toolkit for Unity. The latest version of the MRTK can be found here.
BSD 3-Clause "New" or "Revised" License
358 stars 91 forks source link

[IMPROVEMENT] Useless `[ExecuteAlways]` and `Update()` calls in CanvasSliderVisuals, editor slider value not sync in playmode #521

Open anonymous2585 opened 10 months ago

anonymous2585 commented 10 months ago

Describe the problem

The script CanvasSliderVisuals uses an Update() function to do things in the editor. https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity/blob/e30b350c131501084a4f9003cba1361248717f66/org.mixedrealitytoolkit.uxcore/Slider/Visuals/CanvasSliderVisuals.cs#L166

I fell it is not necessary in this case, a OnValidate() function should do the job. The [ExecuteAlways] could then be removed, improving in editor performances.

Moreother, the handle position is not updated when modifying the slider value from the inspector in playmode. This is different from the classic Unity UI. I d'like the behaviour in editor to be the same as the Unity UI slider.

Describe the solution you'd like

AMollis commented 9 months ago

Low priory issue, worth investigating.

AMollis commented 7 months ago

Hi @anonymous2585 ,

Thank you for reporting this issue. We appreciate your feedback and interest in MRTK3.

We have reviewed your issue. We have added it to our backlog and will prioritize it accordingly.

However, if you would like to speed up the process and contribute to the MRTK3 project, you are welcome to submit a Pull Request with your proposed fix or implementation. We would be happy to review your code and merge it into the main branch if it meets our quality standards.

Please let us know if you have any questions or need any assistance. Thank you for your support and collaboration.

Best regards, The MRTK3 team