search

GameLabGraz / Maroon

An interactive and immersive laboratory for Web, PC and VR.
https://maroon.tugraz.at
31 stars 16 forks source link

Feature/maroon 404 falling ball viscosimeter #511

Closed danielliegl closed 1 month ago

danielliegl commented 2 months ago

Close #404

FlorianGlawogger commented 1 month ago

Hi, just to be sure we are on the same page @danielliegl , you requested another review, and I saw you made tons of requested changes! Though I looked at a few of the previous review's comments, and it seems like some of the comments are not resolved yet, e.g. Debug.Log("dynamic visc " + dynamic_viscosity); is still in the Ball.cs file, or not sure if you wrote somewhere regarding the Decimal Quantity? Please tell me in case I am missing something, but otherwise please make sure to resolve all suggestions before requesting another review to save us both some time, (or please write a comment in case a suggestion doesn't make sense).

danielliegl commented 1 month ago

Hi, just to be sure we are on the same page @danielliegl , you requested another review, and I saw you made tons of requested changes! Though I looked at a few of the previous review's comments, and it seems like some of the comments are not resolved yet, e.g. Debug.Log("dynamic visc " + dynamic_viscosity); is still in the Ball.cs file, or not sure if you wrote somewhere regarding the Decimal Quantity? Please tell me in case I am missing something, but otherwise please make sure to resolve all suggestions before requesting another review to save us both some time, (or please write a comment in case a suggestion doesn't make sense).

Hi @FlorianGlawogger, I guess I got sloppy after going through these 200 conversations, sorry. I went through all of them again. Now all issues that I have fixed are resolved. There is only the question about the Localization of the Tooltips.

danielliegl commented 1 month ago

Fixed the last few things. Also fixed another bug, where the "Choose your object to measure" textbox stayed visible, after stopping measurement and added translations for the explanations of the measurement mode.

I hope this should be it :)

FlorianGlawogger commented 1 month ago

fyi: just waiting for Github Actions to finish, then we can merge it