Skip to content

Media changes including support for PRIM_MEDIA_FIRST_CLICK_INTERACT and HUD autoplay #3989

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

Open
wants to merge 7 commits into
base: release/2025.04
Choose a base branch
from

Conversation

DarlCat
Copy link

@DarlCat DarlCat commented Apr 25, 2025

This PR does three things which were relatively difficult for me to decouple:

  1. Adds conditional support for PRIM_MEDIA_FIRST_CLICK_INTERACT to the viewer
  2. Adds a new setting for media auto-play on HUDS
  3. Shuffles the sound & media tab just a bit to make space for these new goodies while cleaning up some forgotten XUI attributes on WebRTC settings. Creating room for both of these options made separate PRs less attractive, as I think everyone would agree less XUI merging the better.

Settings added are

  • MediaAutoPlayHuds (bool) Allows web media on HUDs to autoplay even if the user disables autoplay for inworld media
  • MediaFirstClickInteract (s32) Determines which (if any) objects are able to skip the focus click for their media

While adding coverage for media first click interact I discussed the feature with a few people, and it was determined that also passing hover events to media contexts without explicitly granting focus was also expected, the biggest reason was support for things like hover tips and hover controls on webpages.

I admit that XUI is nowhere to be found on my list of strengths, so I apologize in advance for what I've done. If there is any input on how to improve anything about this PR I warmly welcome feedback. I wanted to make sure that everything fit, and it didn't look too terribly thrown together. It may be time to think about breaking this preferences category up, or moving into tabs. With WebRTC voice changes and additional settings from that likely, a section for voice alone would make a lot of sense and de-clutter this area a lot.

Lastly, a screenshot of the modified sound & media preferences panel for first impressions

image

@github-actions github-actions bot added the c/cpp label Apr 25, 2025
Copy link

github-actions bot commented Apr 25, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@DarlCat
Copy link
Author

DarlCat commented Apr 25, 2025

I have read the CLA Document and I hereby sign the CLA

@DarlCat
Copy link
Author

DarlCat commented Apr 25, 2025

I forgot something here and will add that this evening, oops!

Copy link
Contributor

@marchcat marchcat left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!

The code looks good, but since you mentioned that you forgot something, let's wait for the complete patch.

@DarlCat
Copy link
Author

DarlCat commented Apr 26, 2025

I've gotten the changes I forgot pushed now, that's what I get for making this feature in another viewer and porting it. Can not wait for native Linux builds so I can actually work and test properly. Sorry!

@marchcat
Copy link
Contributor

Thank you very much!

Copy link
Contributor

@marchcat marchcat left a comment

Choose a reason for hiding this comment

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

@Geenz, please take a look. Can we accept this change in 2025.04?

@marchcat marchcat requested a review from Geenz April 26, 2025 02:01
@DarlCat
Copy link
Author

DarlCat commented Apr 26, 2025

I grabbed a copy of the Mac build that just finished, and the preference panel is all wonky. Hotfix coming shortly 🙃

@marchcat
Copy link
Contributor

Thanks, I'll start the new build.

@DarlCat
Copy link
Author

DarlCat commented Apr 26, 2025

I'm really sorry for the break fix break fix loop here, it's really hard for me to test this feature in another viewer and keep it ported since the only machine I have that can build in less than an hour runs Linux. I've broken autoplay and have it always on. I will have this fixed in the next hour or two, but if that won't do just let me know which branch to retarget this PR to and I'll gladly do that.

No, I just gaslit myself. It's good now. I performed clean logins with media auto-play set to off and always with HUD autoplay checked and unchecked and it behaved how it should.

@WolfGangS
Copy link

WolfGangS commented Apr 26, 2025

It might be overkill but if the "Any object" client setting for "Allow first click" was changed to -1, then the parameter could be later implemented as a bit field to allow for more granular permissions like "Allow group", "Allow Land owner", "Allow Friends", "Allow ..." down the road, with little additional work. and no real change to the current implementation.

Now if only xui had a nice multi select drop down...

@WolfGangS
Copy link

WolfGangS commented Apr 27, 2025

Also with regards to PRIM_MEDIA_FIRST_CLICK_INTERACT

I'd suggest ignoring it for HUD's, I cannot think of a scenario where any user or creator would want to not have first click interaction on a hud web ui.

As this flag hasn't worked for over a decade, many existing huds will not have it set

So obeying the flag on the hud would result in (I would guess pretty much most) existing huds suddenly being "broken" as far as their users are concerned, because some work on first click and some don't.

This could easily be documented on the wiki as the expected behavior, something like.

Sets whether the first click interaction is enabled, for media not worn on HUD.
HUD media always allows first click interact if the viewer allows it.

As the flag has never worked before I don't think a tweak to its "purpose" would be a problem.

…s and make the setting a bitfield for advanced users to customize.
@DarlCat
Copy link
Author

DarlCat commented Apr 27, 2025

@WolfGangS I've accommodated your feedback, and made the setting a bitfield with added cases. I tried to give the function as many early bail-outs as reasonable while keeping the code consistent and simple to follow. I'm satisfied with the behavior of the feature for now, and welcome additional feedback or input.

The cases are as follows from the enum in lltoolpie.h

MEDIA_FIRST_CLICK_NONE       = -1,        // Special case: Feature is disabled
MEDIA_FIRST_CLICK_HUD        = 1 << 0,    // 0b00000001 (1)
MEDIA_FIRST_CLICK_OWN        = 1 << 1,    // 0b00000010 (2)
MEDIA_FIRST_CLICK_GROUP      = 1 << 2,    // 0b00000100 (4)
MEDIA_FIRST_CLICK_FRIEND     = 1 << 3,    // 0b00001000 (8)
MEDIA_FIRST_CLICK_LAND       = 1 << 4,    // 0b00010000 (16)

// Covers any object with PRIM_MEDIA_FIRST_CLICK_INTERACT (combines all other flags)
MEDIA_FIRST_CLICK_ANY        = MEDIA_FIRST_CLICK_HUD |
                               MEDIA_FIRST_CLICK_OWN |
                               MEDIA_FIRST_CLICK_GROUP |
                               MEDIA_FIRST_CLICK_FRIEND |
                               MEDIA_FIRST_CLICK_LAND,

// Covers all media regardless of other rules or PRIM_MEDIA_FIRST_CLICK_INTERACT
MEDIA_FIRST_CLICK_ALL        = 1 << 30    // 0b01000000000000000000000000000000 (1073741824)

While this does leave a lot of room for advanced users to customize their experience to their exact taste, I see them as more of a hierarchical structure so that's how I implemented them in the preferences, where each one inherits the ones that come before it.

I've added MEDIA_FIRST_CLICK_ALL with an absurdly large value to keep it well out of the way for possible future expansion on this system, but it's not reachable by preferences because I disabled that entry. If set manually, it ignores PRIM_MEDIA_FIRST_CLICK_INTERACT entirely and enables the functionality for every MOAP instance the viewer encounters. I don't exactly want to ship that in the UI, but am open to feedback so I put it in there tentatively for now.

Lastly I don't think I completely agree with the idea of ignoring it for HUDs, but I do believe that HUDs should get a free pass out of the gate which is why the default I've provided for the new setting is to allow the feature for HUDs only. It's the most present case for this feature that I'm aware of, and what I personally wanted out of this when I set out to get this working.

…son specifically should be exact, not bitwise.
@WolfGangS
Copy link

WolfGangS commented Apr 28, 2025

As its now designed as a bitfield, -1 should not be the disabled option. But the ALL option

MEDIA_FIRST_CLICK_NONE       = 0,   // All bits 0, Feature is disabled
MEDIA_FIRST_CLICK_ALL        = -1   // All bits 1, Everything is enabled

@DarlCat
Copy link
Author

DarlCat commented Apr 28, 2025

You're 100% right, but that specific flag is not handled as a bitfield and has an equality compare rather than a bitwise compare. It should be updated to 0 instead of -1 but I want to give LL a moment to review this before that's done since this PR has already seen a ton of build workflow runs that cost LL money, and I'd rather get meaningful changes in with my next push rather than one that's just a bit pedantic. :p

I'm comitting this change now for my next push and will await formal review before that push happens since it won't constitute any functional change.

@marchcat
Copy link
Contributor

Please feel free to push anytime!

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

Successfully merging this pull request may close these issues.

Enable PRIM_MEDIA_FIRST_CLICK_INTERACT conditionally in the viewer
3 participants