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

v0.5.0: Mandatory Height Calibration Prompt, Compatibility with Relay Server v1.1.0, Enhanced People Panel, Various Refactors #80

Closed
wants to merge 64 commits into from

Conversation

Barasakar
Copy link
Contributor

@Barasakar Barasakar commented Oct 14, 2021

Mandatory Height Calibration Prompt

Created a prototype of the height calibration prompt that asks users to calibrate their height.

Having the height calibration done early might help addressing one of the issues (i.e., teleporting into the ground).

Work around for #61 .

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?

Komodo runs -> prompt shows up -> calibrate height -> prompt disappears
This is not a multi-player feature.

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

Test Configuration:

  • REPLACE ME: Browser vendor and version: e.g., Chrome Version 91.0.4472.164 (Official Build) (64-bit)
  • REPLACE ME: VR Device and OS version: e.g., Oculus Quest v29
  • 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).

@@ -145,6 +145,8 @@ public void EndCalibration ()

onFinishedCalibration.Invoke(heightToBumpPlayer);

KomodoEventManager.TriggerEvent("FinishedHeightCalibration");
Copy link
Contributor

Choose a reason for hiding this comment

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

(future) store event names as constant strings in a central files

private static KomodoEventManager eventManager;

public static KomodoEventManager Instance
{
//using the getter to find the instance
Copy link
Contributor

Choose a reason for hiding this comment

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

(future) consolidate comments

@parseccentric parseccentric changed the title Mandatory Height Calibration Prompt v0.5.0: Mandatory Height Calibration Prompt, Compatibility with Relay Server v1.1.0, Enhanced People Panel, Various Refactors Oct 21, 2021
@parseccentric
Copy link
Contributor

parseccentric commented Nov 2, 2021

Compatibility with Relay Server v1.1.0, Enhanced People Panel, Various Refactors

Connect to sync namespace (breaking change)

The client is now compatible with komodo-relay (server) v1.1.0, which requires a connection to the sync namespace.

How to test: open the Unity client in a browser and connect to a v1.1.0 server -> the people tab should report a successful connection.

Support for v1.1.0 should theoretically fix #59

Fix state catch-up to use messaging system (bug fix, new feature)

The state catch-up had a bug where it would always ask the ClientSpawnManager to apply state, so networked objects (synchronized models) would not get transform, visibility, or lock states. This is now fixed.

How to test: go to Unity window > Hierarchy panel > Visible managers > NetworkManager. See Inspector panel > SocketIOEditorSimulator component, right click it, and choose ReceiveExampleStateCatchup. ->The models should all be lined up in a diagonal line, be spaced evenly, and up to four of them should be visible and locked. (Fewer will be visible and locked if some are marked as "isWholeObject = false" in ModelData. This is a limitation of the test.)

Lock and visibility toggle infinite loop (bug fix)

It should no longer be possible to trigger an infinite loop by rapidly pressing the lock and visibility toggle.

How to test: open two clients and rapidly press the visibility or lock button, then let go -> the button should quickly stop flashing on and off.

Max client count (new feature)

The maximum number of clients has arbitrarily been increased to 15.

Improved People tab

  • The server name is displayed.
  • The session number is displayed.
  • The connection status is displayed.
  • More connection or SocketIO room errors are displayed.
  • There's now a proper grid view for client names in menu.
  • The runtime app and build is now shown, rather than the configuration-time app and build. ("App" might be called "scope" or "folder" in other documentation.) Fixes render build string in actual use, not the config one #52

Fixes #60

Major networking refactors

  • SocketIOJSLib
  • SocketIOAdapter
  • SocketIOEditorSimulator

Refactors

SessionStateManager has been introduced to apply multiplayer server state catch-ups to the avatars (clients) and networked objects (synchronized models).

A new class, NetworkedObjectsManager, has been introduced to manage synchronized models.

Adding and removing clients was refactored into smaller functions in an attempt to repair the out-of-body experience.

The client teardown code was moved from NetworkUpdateHandler to SocketIOAdapter for the same reason.

Minor renamings.

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?

Please see above for the manual test instructions.

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

Test Configuration:

  • Firefox v??
  • Oculus Quest v??
  • PC VR (Link)
  • 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).

@parseccentric
Copy link
Contributor

@Barasakar Could you review this when you have a chance?

@Barasakar
Copy link
Contributor Author

Barasakar commented Nov 3, 2021

@parseccentric Yeah, I have reviewed this whole thread. Let me know if there is anything else I need to do.

@parseccentric
Copy link
Contributor

@Barasakar I made another mistake. As you can see above, I mistakenly committed 508b74d and 18651ff onto this branch before merging. I apologize for this error. I will close this pull request and open a new one with a temp branch that reflects the state of the pull request.

@parseccentric parseccentric mentioned this pull request Nov 10, 2021
@parseccentric parseccentric deleted the develop branch November 12, 2021 19:51
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