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

feat: sdk pointer event toggle highlight #2651

Merged
merged 8 commits into from
Nov 5, 2024

Conversation

pravusjif
Copy link
Member

@pravusjif pravusjif commented Oct 30, 2024

WHY

Currently scenes have no control over the automatic shader highlighting that happens on interractable objects (pointer event entities).

These are the possible pointer events feedback combinations that the scene can set today:

  • Highlight ON + Hover Text ON
  • Highlight ON + Hover Text OFF

These other states are not possible today:

  • Highlight OFF + Hover Text OFF
  • Highlight OFF + Hover Text ON

WHAT

  • Implemented support for new showHighlight property in the PBPointerEvents component, binding the highlight setup of entities to that property.
  • Minor change on showFeedback usage to also avoid enabling the highlight if it's false, as the highlight is also the feedback
  • Minor check to avoid enabling the hoverText if the component comes with that property with an empty string. Didn't touch the existent behaviour of not coming with that optional property that ends up using the default "Interact" hover text.

Tackles: decentraland/creator-hub#160

Related PRs:

QA TEST INSTRUCTIONS

  1. Download test scene and uncompress it somewhere
  2. Enter the scene root folder and run npm i and then npm run start -- --explorer-alpha
  3. Close the Explorer that auto-opened. Leave the scene running in the console/terminal.
  4. Download the build from this PR and connect it to the running scene using the console command according to your OS
  5. Once the scene loads, run the /reload chat command to reload the scene, to avoid THIS OTHER BUG.
  6. Put your cursor on each one of the "pointer event cubes" and confirm that they have either the HIGHLIGHT/GLOW and the HOVER TEXT enabled/disabled according to the text shape on top of each cube. (The actual hover text value of each cube can be ignored, it's only important to see if the text appears or not)
Screenshot 2024-10-30 at 5 20 53 PM

@pravusjif pravusjif added feature a new feature sdk labels Oct 30, 2024
@pravusjif pravusjif self-assigned this Oct 30, 2024
@pravusjif
Copy link
Member Author

pravusjif commented Oct 30, 2024

I'll update the protocol package to point to the latest published one after the protocol PR is merged.

@pravusjif pravusjif marked this pull request as ready for review October 30, 2024 18:45
@pravusjif pravusjif requested review from lorux0 and removed request for dalkia October 30, 2024 18:53
@Ludmilafantaniella
Copy link

🔴 The HIGHLIGHT/GLOW highlight isn't working on both Windows and Mac.
🟢 HOVER TEXT is working correctly on both platts

2651.Fail.mp4

@pravusjif

@pravusjif pravusjif removed the request for review from lorux0 November 1, 2024 12:20
Copy link

@Ludmilafantaniella Ludmilafantaniella left a comment

Choose a reason for hiding this comment

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

🟢 Feat verified on Mac and Windows 🎉
Each pointer event cube displayed the correct hover/highlight states as labeled. QA Approved.

2651.approve.mp4

Smoke test was performed covering:

  • Social interaction (chat, passports, sync)
  • Backpack
  • Emoting
  • Changing location using chat command and map
  • changing wearables
  • Minigames
  • Hovering over GP buttons and other interactables elements
  • Scenes: Doll House, Casa Roustan, Metadynelabs

@pravusjif pravusjif merged commit d9c1978 into dev Nov 5, 2024
5 checks passed
@pravusjif pravusjif deleted the feat/sdk-pointer-event-toggle-highlight branch November 5, 2024 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a new feature sdk
Projects
Archived in project
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants