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

Release/v0.5.8 #101

Merged
merged 18 commits into from
Feb 25, 2022
Merged

Release/v0.5.8 #101

merged 18 commits into from
Feb 25, 2022

Conversation

parseccentric
Copy link
Contributor

@parseccentric parseccentric commented Feb 8, 2022

v0.5.8 Release: Interactable Drawings, Refactored DrawingInstanceManager

Interactable Drawings

Please see #100

Refactoring

Renamed Draw_Refresh to ReceiveDrawUpdate and refactored into the following functions:

  • ContinueLine
  • EndLine
  • DeleteLine
  • HideLine
  • ShowLine

Combined lineRenderersInQueue and allStrokeIDValidator into a single dictionary, lineRendererFromID, and made sure any uses of that dictionary went through new functions that talk directly to the ID. Did not change the overall functionality.

De-coupled DrawingInstanceManager from NetworkObjectsManager.

Added various warnings for edge cases, like trying to move a line that exists in your client but not in someone else's (because you drew it before they joined).

The motivation is that we should now be able to more easily debug the content. In particular, we can address the warning in the linked pull request and display the drawing line / stroke ID.

Type of change

  • 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 not work as expected)
  • Testing
  • Change that requires a documentation update

How Has This Been Tested?

Test basic single-client transforming of drawings

  • Connect with a client, then...
  • make drawings
  • try to transform your own drawings (translate / rotate / scale)

Test basic synchronized transforming of drawings

  • Connect with two clients, then...
  • make drawings
  • try to transform your own drawings (translate / rotate / scale)
    • expected result: transform synchronizes correctly
  • try to transform each others' drawings (translate / rotate / scale)
    • expected result: transform is successful
    • expected result: transform synchronizes correctly

Note that undo only applies to the draw action and erase action, and not to any transform actions.

  • Manual Test
  • Unit Test
  • Integration / End-to-End Test

[Duplicate all of the above for each distinct feature / bug fix / etc.]


Test Configuration

  • Firefox
  • Oculus Quest 1
  • Oculus Quest 2
  • PC VR (Link / AirLink)
  • Standalone

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas that are not self-documenting
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • My changes have no unnecessary logging
  • I have added tests that prove my fix is effective or that my feature works, for sufficiently complex features
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • Sensitive info like tokens, secrets, and passwords have been removed before submitting

Modified from this article:
Phillip Johnston, “A GitHub Pull Request Template for Your Projects - Embedded Artistry,” Embedded Artistry, Aug. 04, 2017. https://embeddedartistry.com/blog/2017/08/04/a-github-pull-request-template-for-your-projects/ (accessed Jul. 22, 2021).

{
UndoRedoManager.Instance.savedStrokeActions.Push
(
(System.Action)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Future) Do we need the System.Action wrapper?

@parseccentric
Copy link
Contributor Author

parseccentric commented Feb 9, 2022

  • Add manual and semi-automated tests for various drawing functions
  • Put new manual tests onto this branch
  • Test dev build
  • Test non-dev build
  • Upload build with any attached logs

@parseccentric
Copy link
Contributor Author

See https://uofi.box.com/s/4d4nbx1l98ipoe41ko7m23kcsa36fg9g for the capture log, the build test results, additional bug and workaround notes, etc.

@parseccentric
Copy link
Contributor Author

parseccentric commented Feb 10, 2022

VR Notes:

  • avatar only seems to disappear from own client when tab is closed -- not from other clients. Need to verify further.
  • after exiting VR mode and going to spectator mode, avatar goes to the ground
  • after pressing leave and rejoin, some avatar positions get copied to non-connected users's avatars (their head and hands are in the same place)

Spectator Notes:

  • CAUTION: IF YOU EVER ENTER VR AND EXIT VR, YOU WILL LOSE ACCESS TO THE INSTRUCTOR MENU BUTTON. Workaround: refresh the page.
  • Known issue: if you ever refresh the page, the capture will appear to not be running.
    • If you intend to stop the capture, press start capture (which will try to start the capture but be ignored), then press stop capture (which will successfully stop the capture)
    • If you intend to let the capture continue, it's best to not press anything.

WORKAROUND: it is paramount to check the status of the session connection in the people tab before pressing Start Capture. IT CANNOT say […] <session id>. If it does, then your capture will fail to start. If it does have the ellipses, you must refresh the page, Leave and Rejoin, or Close Connection and Rejoin until the ellipses goes away.

VR Bug: Drawing messages aren't always received after reconnection

Steps to reproduce:

  • J joined
  • B joined
  • J turned off wifi and turned back on
  • B refreshed (multiple times)
  • B entered VR and exited VR (multiple times)
  • B started capture
  • B could see J moving and drawing
  • J could see B moving
  • J could not see B drawing
  • B's people tab said [...] <session ID>
  • B left and rejoined
  • same results
  • B closed connection and rejoined
  • same results
  • J left and rejoined
  • same results
  • J closed connection and rejoined
  • J could see B drawing

@parseccentric
Copy link
Contributor Author

Please see commits in #99 to understand some bug fixes finished in release/v0.5.7.

@parseccentric
Copy link
Contributor Author

@Barasakar
Copy link
Contributor

@parseccentric
Copy link
Contributor Author

@parseccentric
Copy link
Contributor Author

parseccentric commented Feb 25, 2022

  • Finish writing build test results and upload them
  • Upload capture files and annotate them
  • Create non-development build
  • Upload non-dev build to base/v0.5.8 and change relay URLs
  • Upload non-dev build to base/stable and change relay URLs
  • Update test template
    Then merge!
  • Create tag and write release (to OpenUPM)
  • Upload public non-dev build to Box and to release (with localhost relay URLs)

@parseccentric
Copy link
Contributor Author

parseccentric commented Feb 25, 2022

@parseccentric
Copy link
Contributor Author

@Barasakar I'm just going to bypass our typical review process and merge this myself. Please let me know if you find anything wrong in the repo, and we can recuperate from there.

@parseccentric parseccentric merged commit 5ee54e5 into main Feb 25, 2022
@parseccentric
Copy link
Contributor Author

  • Finish writing build test results and upload them
  • Upload capture files and annotate them
  • Create non-development build
  • Upload non-dev build to base/v0.5.8 and change relay URLs
  • Upload non-dev build to base/stable and change relay URLs
  • Update test template
    Then merge!
  • Create tag and write release (to OpenUPM)
  • Upload public non-dev build to Box and to release (with localhost relay URLs)

@Barasakar This is everything I do after each testing session. It's kind of a lot TBH so if you find a way to simplify that then go right ahead!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants