chore: sdk camera control minor iteration

[pravusjif]

Updated protocol and adapted to it.

Related PRs:

• chore: virtual cam components iteration protocol#219
• chore: virtual camera components iteration js-sdk-toolchain#998
• chore: virtual camera test scene updates 2 sdk7-test-scenes#17

Summary by CodeRabbit

• New Features

  • Enhanced camera transition handling with structured management for different modes (Time, Speed).
  • Improved clarity on virtual camera entity presence checks.

• Bug Fixes

  • Streamlined CameraTransition class by removing unused fields, focusing on essential properties.

• Documentation

  • Updated comments to clarify behavior and usage of camera transition and virtual camera entity features.

• Chores

  • Changed dependency versioning method for @dcl/protocol to a fixed reference URL.

---

QA TEST INSTRUCTIONS

• Download this PR's Explorer build
• Access the pravus.dcl.eth world for running the test scene (/goto pravus)
• Once the scene loads you are ready to test
  • Pressing 'E' will cycle between the scene virtual cameras
  • When you are with the Character default camera, if you look up you will be able to see the 4 entities that have virtual cameras (represented as cubes). The numbers represent the order in which they will be cycled.

Things to test:

Using the Character default camera

1. Enter the Avatar Modifier Area and exit it (being in third person cam), confirm that the Avatar is hidden only when inside the area.
2. Being in Third Person Cam enter the Camera Mode Area and confirm that the camera is forcefully changed to 1st perdon cam. Check that you can't change the camera mode inside that area but that it resets when you get out of it.

Using the SDK Camera 1

1. Confirm that you can move normally as you would if you were in third person cam.
2. Enter the Avatar Modifier Area and exit it, confirm that the Avatar is hidden only when inside the area.
3. Enter the Camera Mode Area and confirm that it doesn't change the camera at all.

Using the SDK Camera 2

1. Confirm that you can move normally as you would if you were in third person cam.
2. Enter the Avatar Modifier Area and exit it, confirm that the Avatar is hidden only when inside the area.
3. Enter the Camera Mode Area and confirm that it doesn't change the camera at all.
4. Exit the scene, and confirm that the camera goes back to the Character default camera and you can still control it. Then re-enter the scene and check that the last SDK Camera that was active is now active again.

Using the SDK Camera 3

1. Confirm that this camera constantly looks at the player, even when you move the player.
2. Confirm that you can move normally as you would if you were in third person cam.
3. Enter the Avatar Modifier Area and exit it, confirm that the Avatar is hidden only when inside the area.
4. Enter the Camera Mode Area and confirm that it doesn't change the camera at all.
5. Exit the scene, and confirm that the camera goes back to the Character default camera and you can still control it. Then re-enter the scene and check that the last SDK Camera that was active is now active again.

Using the SDK Camera 4

1. Confirm that you can move normally as you would if you were in third person cam.
2. Enter the Avatar Modifier Area and exit it, confirm that the Avatar is hidden only when inside the area.
3. Enter the Camera Mode Area and confirm that it doesn't change the camera at all.
4. Exit the scene, and confirm that the camera goes back to the Character default camera and you can still control it. Then re-enter the scene and check that the last SDK Camera that was active is now active again.

Camera Mode Area

1. Being in character's default camera
   -> move inside the CameraModeArea that will force 1st person cam
   -> press 'E' to activate any SDK camera (doesn't matter which one)
   -> cycle through the cameras until you get back to the character camera
   -> confirm that you are again under the effect of the Camera Mode Area
   -> get out of the Camera Mode Area and confirm you can control the camera again
2. Being in character's default camera
   -> press 'E' to activate any SDK camera (doesn't matter which one)
   -> move the character inside the CameraModeArea
   -> cycle through the cameras until you get back to the character camera
   -> confirm that you are now under the effect of the Camera Mode Area
   -> get out of the Camera Mode Area and confirm you can control the camera again
3. Being in character's default camera
   -> move inside the CameraModeArea that will force 1st person cam
   -> press 'E' to activate any SDK camera (doesn't matter which one)
   -> move the character outside the Camera Mode Area
   -> cycle through the cameras until you get back to the character camera
   -> confirm that you free from the effect of the Camera Mode Area and you can control the camera again
4. Being in character's default camera
   -> move inside the CameraModeArea that will force 1st person cam
   -> press 'E' to activate any SDK camera (doesn't matter which one)
   -> move the character outside the Camera Mode Area
   -> move the character inside the Camera Mode Area again
   -> cycle through the cameras until you get back to the character camera
   -> confirm that you are still under the effect of the Camera Mode Area
   -> get out of the Camera Mode Area and confirm you can control the camera again
5. Being in character's default camera
   -> press 'E' to activate any SDK camera (doesn't matter which one)
   -> move the character inside the Camera Mode Area
   -> move the character outside the Camera Mode Area again
   -> cycle through the cameras until you get back to the character camera
   -> confirm that you are still free from the effect of the Camera Mode Area

Video reference for testing

SDKCameraControlDemo-NO-CONTROLLABLE-CAM.mp4

[coderabbitai]

Walkthrough

The pull request introduces several changes across multiple files, primarily focusing on enhancing the handling of camera transitions and the management of virtual camera entities. Key modifications include the restructuring of transition logic using switch statements, the removal of unnecessary fields from the CameraTransition class, and improvements in the PBMainCamera class for better field management. Additionally, the dependency versioning for the @dcl/protocol package has been updated to reference a specific tarball URL.

Changes

File Path	Change Summary
Explorer/Assets/DCL/SDKComponents/CameraControl/MainCamera/...	Updated logic in MainCameraSystem.cs to use HasVirtualCameraEntity for clarity. Enhanced camera transition handling in VirtualCameraUtils.cs with a switch statement for transition modes.
Explorer/Assets/Protocol/DecentralandProtocol/CameraTransition.gen.cs	Removed FromEntity and ToEntity fields from CameraTransition, simplifying the class structure. Updated serialization logic accordingly.
Explorer/Assets/Protocol/DecentralandProtocol/MainCamera.gen.cs	Introduced a bitfield _hasBits0 in PBMainCamera to track the presence of VirtualCameraEntity. Added methods for checking and clearing this field.
Explorer/Assets/Protocol/DecentralandProtocol/VirtualCamera.gen.cs	Updated the static constructor of VirtualCameraReflection to clarify default transition behavior and modified property representation in the descriptor data.
scripts/package.json	Changed the @dcl/protocol dependency version from a semantic versioning string to a direct URL to a tarball, impacting how dependencies are resolved.

Possibly related PRs

• feat: sdk camera control components #1744: Changes in MainCameraSystem.cs regarding virtualCameraCRDTEntity relate to enhancements in MainCamera.gen.cs for VirtualCameraEntity.
• feat: block player movement #1777: Modifications in input systems may relate to overall camera control and visibility logic impacted by the main PR.
• Refactor: initial state sync #1936: Changes in scene data propagation connect with enhancements in VirtualCameraEntity handling.
• Optimization: avatar area system #1998: Updates in CharacterTriggerAreaComponent relate to camera control logic and spatial management.
• Enhacement: Show scene limits through debug panel (#1873) #2023: Enhancements in scene information display may connect with improved camera handling and visibility.

Suggested labels

feature

Poem

🐇 In the land where cameras play,
A new path has come our way.
With transitions smooth and bright,
Virtual views take joyful flight.
A hop, a skip, a leap we cheer,
For clearer sights are finally here! 🌟

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  -- @coderabbitai generate unit testing code for this file.
  -- @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  -- @coderabbitai generate interesting stats about this repository and render them as a table.
  -- @coderabbitai read src/utils.ts and generate unit testing code.
  -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  -- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[github-actions]

Windows and Mac build successfull in Unity Cloud! You can find a link to the downloadable artifact below.

Name	Link
Commit	870bcf8
Logs	https://github.com/decentraland/unity-explorer/actions/runs/10957655587
Download Windows	https://github.com/decentraland/unity-explorer/suites/28636390122/artifacts/1957772819
Download Mac	https://github.com/decentraland/unity-explorer/suites/28636390122/artifacts/1957766119
Built on	2024-09-20T11:08:57Z

[github-actions]

New build in progress, come back later!

[pravusjif]

TODO: after protocol PR gets merged, update the package used by this branch

[coderabbitai]

Actionable comments posted: 0

Outside diff range and nitpick comments (2)

scripts/package.json (1)

19-19: Consider the implications of switching to a tarball URL for the @dcl/protocol dependency.

Changing the dependency from a semantic version string to a fixed tarball URL has a few implications:

1. It ensures that the exact same code is used every time, regardless of any updates to the package on the registry. This can help maintain consistency but also means manually updating the URL for any desired changes.

2. Fetching the dependency from a CDN URL could impact build times and reliability, as it introduces a dependency on an external service.

3. The tarball URL doesn't specify a hash, which means there's no guarantee of content integrity. If the content at the URL changes, it could lead to unexpected behavior.

Recommendations:

• Consider adding a hash to the URL for content integrity verification.
• Document the rationale behind this change, including any benefits it provides and any additional considerations for maintainers.

Explorer/Assets/Protocol/DecentralandProtocol/CameraTransition.gen.cs (1)

42-45: Informative summary comment.

The summary comment provides useful context about the potential future updates to the CameraTransition class, such as adding support for easing functions and entity specifications. This helps developers understand the broader context and anticipate future changes.

Consider creating separate GitHub issues or tracking tasks for the mentioned future updates to ensure they are properly planned and implemented when the time comes.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e167fb8 and 1734cdd.

Files ignored due to path filters (1)

• scripts/package-lock.json is excluded by !**/package-lock.json

Files selected for processing (6)

• Explorer/Assets/DCL/SDKComponents/CameraControl/MainCamera/Systems/MainCameraSystem.cs (1 hunks)
• Explorer/Assets/DCL/SDKComponents/CameraControl/MainCamera/VirtualCameraUtils.cs (1 hunks)
• Explorer/Assets/Protocol/DecentralandProtocol/CameraTransition.gen.cs (8 hunks)
• Explorer/Assets/Protocol/DecentralandProtocol/MainCamera.gen.cs (9 hunks)
• Explorer/Assets/Protocol/DecentralandProtocol/VirtualCamera.gen.cs (2 hunks)
• scripts/package.json (1 hunks)

Additional context used

Path-based instructions (5)

Explorer/Assets/DCL/SDKComponents/CameraControl/MainCamera/VirtualCameraUtils.cs (2)

Pattern **/*.cs: Review the code for heap allocations and suggest potential improvements to avoid runtime allocations.

---

Pattern **/*.cs: Review the code for specific unity engine optimizations and suggest potential improvements.

Explorer/Assets/DCL/SDKComponents/CameraControl/MainCamera/Systems/MainCameraSystem.cs (2)

Pattern **/*.cs: Review the code for heap allocations and suggest potential improvements to avoid runtime allocations.

---

Pattern **/*.cs: Review the code for specific unity engine optimizations and suggest potential improvements.

Explorer/Assets/Protocol/DecentralandProtocol/MainCamera.gen.cs (2)

Pattern **/*.cs: Review the code for heap allocations and suggest potential improvements to avoid runtime allocations.

---

Pattern **/*.cs: Review the code for specific unity engine optimizations and suggest potential improvements.

Explorer/Assets/Protocol/DecentralandProtocol/CameraTransition.gen.cs (2)

Pattern **/*.cs: Review the code for heap allocations and suggest potential improvements to avoid runtime allocations.

---

Pattern **/*.cs: Review the code for specific unity engine optimizations and suggest potential improvements.

Explorer/Assets/Protocol/DecentralandProtocol/VirtualCamera.gen.cs (2)

Pattern **/*.cs: Review the code for heap allocations and suggest potential improvements to avoid runtime allocations.

---

Pattern **/*.cs: Review the code for specific unity engine optimizations and suggest potential improvements.

Additional comments not posted (11)

Explorer/Assets/DCL/SDKComponents/CameraControl/MainCamera/VirtualCameraUtils.cs (1)

45-78: LGTM!

The code changes in the ConfigureVirtualCameraTransition method are well-structured and follow a clear logic for handling different transition modes. The code correctly sets the blend style and time based on the transition mode and handles the case when the default transition is null. The code is readable and maintainable.

Explorer/Assets/DCL/SDKComponents/CameraControl/MainCamera/Systems/MainCameraSystem.cs (1)

66-66: LGTM!

The change improves the clarity of the condition by explicitly indicating whether a virtual camera entity exists, rather than relying on a numeric comparison. The logic remains correct.

Explorer/Assets/Protocol/DecentralandProtocol/MainCamera.gen.cs (4)

53-53: Good use of bitfield for tracking optional fields.

Using a bitfield _hasBits0 to track whether optional fields like VirtualCameraEntity have been set is a good optimization technique to minimize memory usage. Since int is a value type, it will not cause any heap allocations.

---

101-117: Proper handling of optional fields using the bitfield.

The updates to the VirtualCameraEntity property getter to return 0 when the _hasBits0 bitfield indicates the field has not been set is a good practice to avoid returning uninitialized data.

The new HasVirtualCameraEntity() and ClearVirtualCameraEntity() methods provide a clean way to check the presence of the field and reset it by leveraging the bitfield.

These changes efficiently manage the optional field without introducing any new heap allocations or negatively impacting performance.

---

143-143: Proper handling of optional fields in hash computation.

Updating the GetHashCode() method to include the VirtualCameraEntity field in the hash computation only when the _hasBits0 bitfield indicates it has been set is a good practice.

This ensures that two instances of PBMainCamera with unset VirtualCameraEntity fields will have the same hash code, maintaining consistency.

The change does not introduce any performance issues or allocations.

---

Line range hint 162-166: Efficient serialization and merging of optional fields.

The updates to the WriteTo(), InternalWriteTo(), CalculateSize(), and MergeFrom() methods to check the _hasBits0 bitfield before serializing, deserializing, or merging the VirtualCameraEntity field are an excellent optimization.

By avoiding unnecessary reads and writes when the field is not set, these changes improve performance without introducing any new heap allocations.

Moreover, checking the bitfield ensures that the serialized data accurately represents the state of the object by omitting optional fields that are not set.

Also applies to: 176-180, 190-192, 205-207

Explorer/Assets/Protocol/DecentralandProtocol/CameraTransition.gen.cs (4)

29-31: LGTM!

The updated field numbers for Time and Speed are consistent with the removal of the FromEntity and ToEntity fields. This ensures proper serialization of the remaining fields.

---

35-35: LGTM!

The updated CameraTransition class definition, which now includes only the Time and Speed fields, simplifies the class structure and is consistent with the removal of the FromEntity and ToEntity fields.

---

Line range hint 99-130: LGTM!

The updates to the TimeFieldNumber and SpeedFieldNumber constants, as well as the TransitionModeOneofCase enum, are consistent with the changes in the protocol buffer definition. These changes ensure proper serialization and deserialization of the Time and Speed fields.

---

193-199: LGTM!

The updates to the serialization and deserialization logic for the Time and Speed fields are consistent with the changes in the protocol buffer definition. The new field numbers, 13 and 21, adhere to the protocol buffer wire format and ensure correct serialization and deserialization of these fields.

Also applies to: 211-217, 270-278, 293-301

Explorer/Assets/Protocol/DecentralandProtocol/VirtualCamera.gen.cs (1)

30-34: LGTM!

The changes in the descriptor data and the GeneratedClrTypeInfo constructor are auto-generated by the protocol buffer compiler to reflect updates in the protocol definition. They improve clarity regarding default camera transition behavior and serialization of the DefaultTransition and LookAtEntity properties.

As indicated by the comment at the top of the file, these auto-generated changes should not be manually edited.

Also applies to: 38-38

[AlejandroAlvarezMelucciDCL]

LGTM, I like that you took good care of the case with no mode set on the SDK, something we learned recently with the Input Modifier 😅 Good job 💪

[popuz]

LGTM!

[anicalbano]

🟢 Reviewed by QA and working as expected, as per the instructions on the PR

20.09.2024_08.28.40_REC.mp4

20.09.2024_08.37.16_REC.mp4

Uploading 20.09.2024_08.34.26_REC.mp4…