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

bitECS: Fix grabbable pinned media objects #6231

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

takahirox
Copy link
Contributor

@takahirox takahirox commented Aug 24, 2023

Fixes #6198

Problem

When you drop or paste a media file or url, a MediaLoader entity is created. This entity downloads the media file and creates a media (e.g., video) entity. The MediaLoader entity object then adds the media entity object as its child.

The MediaLoader entity does not have a visible object after the loading cube disappears, but the media entity does. Both entities are hover targets, but after the loading cube disappears, the MediaLoader entity can no longer be hovered because it does not have a visible object. Only the media entity can be hovered at this point.

The media entity is not a networked entity, but the MediaLoader entity is. This means that the MediaLoader entity can be pinned (the creator is "reticulum"), but the media entity cannot. If you check whether the media entity is pinned, it will always return false. Then media entity will always be grabbable even if it is pinned (more precisely its parent is pinned).

Solution

Check its parent is pinned to check media entity is pinned.

Changes

  • Add LoadedByMediaLoader component to detect a media entity is loaded via MediaLoader
  • Add a special path for media entity loaded via MediaLoader in hold-system that checks its parent is pinned to check whether it is pinned.

Additional context

This solution is a bit hacky. Let me know if you have simpler and more elegant solution idea.

@takahirox
Copy link
Contributor Author

takahirox commented Aug 24, 2023

Alternative solution might be, simply recognize an entity as pinned if its any ancestor is pinned (in hold-system) unless there is a case that descendant entity under pinned entity
wants to be grabbable.

Copy link
Contributor

@nikk15 nikk15 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 detailed explanation. I'm happy to approve this to fix the immediate problem. LGTM

** Problem **

When you drop or paste a media file or url, a MediaLoader entity is created.
This entity downloads the media file and creates a media (e.g., video)
entity. The MediaLoader entity object then adds the media entity object
as its child.

The MediaLoader entity does not have a visible object after the loading
cube disappears, but the media entity does. Both entities are hover targets,
but after the loading cube disappears, the MediaLoader entity can no longer
be hovered because it does not have a visible object. Only the media entity
can be hovered at this point.

The media entity is not a networked entity, but the MediaLoader entity is.
This means that the MediaLoader entity can be pinned (the creator is
"reticulum"), but the media entity cannot. If you check whether the media
entity is pinned, it will always return false. Then media entity will
always be grabbable even if it is pinned (more precisely its parent is
pinned).

**Solution**

Check its parent is pinned to check media entity is pinned.

**Changes**

- Add LoadedByMediaLoader component to detect a media entity
  is loaded via MediaLoader
- Add a special path for media entity loaded via MediaLoader in
  hold-system that checks its parent is pinned to check whether
  it is pinned.
@takahirox takahirox force-pushed the FixGrabbablePinnedMedia branch from 3a95f9b to d13b096 Compare September 1, 2023 16:58
@takahirox takahirox merged commit 9b216c8 into master Sep 1, 2023
9 of 11 checks passed
@takahirox takahirox deleted the FixGrabbablePinnedMedia branch September 1, 2023 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-loader P1 Address as quickly as possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bitECS: Pinned video, audio, gif, pdfs are movable
2 participants