Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

Media system / components cleanup #6861

Merged
merged 49 commits into from
Sep 10, 2022
Merged

Media system / components cleanup #6861

merged 49 commits into from
Sep 10, 2022

Conversation

speigg
Copy link
Member

@speigg speigg commented Sep 1, 2022

Summary

  • Decouples all media components from one-another ( Image / Video / Volumetric )
  • Unifies all playlist management and media element data binding in MediaComponent.ts
  • Clean up ComponentFunctions.ts
    • deprecates createMappedComponent
    • adds defineComponent
  • Improved Editor React typings
  • Update hookstate
  • Improved Editor support for hookstate-driven component data

References

closes #insert number here

Checklist

  • If this PR is still a WIP, convert to a draft
  • ensure all checks pass
  • When this PR is ready, mark it as "Ready for review"
  • Changes have been manually QA'd
  • Changes reviewed by at least 2 approved reviewer

QA Steps

List any additional steps required to QA the changes of this PR, as well as any supplemental images or videos.

@speigg speigg changed the title Image component fixes Media system / components cleanup Sep 6, 2022
@speigg speigg linked an issue Sep 9, 2022 that may be closed by this pull request
@@ -15,7 +15,7 @@ export const removeEntity = (entity: Entity, immediately = false, world = Engine
bitECS.removeEntity(world, entity)
} else {
removeAllComponents(entity, world)
addComponent(entity, EntityRemovedComponent, {})
addComponent(entity, EntityRemovedComponent, null)
Copy link
Member

Choose a reason for hiding this comment

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

what's the reasoning for this? it will cause hasComponent checks to always return false

Copy link
Member Author

Choose a reason for hiding this comment

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

it won't

Copy link
Member

Choose a reason for hiding this comment

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

whats the reason for doing this instead of true

Copy link
Member Author

@speigg speigg Sep 9, 2022

Choose a reason for hiding this comment

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

true takes up space, and isn't necessary; it's annoying that we even have to pass null, frankly, but it doesn't seem possible to have optional generic parameters that are also sometimes required

@speigg speigg marked this pull request as ready for review September 10, 2022 22:08
@speigg speigg merged commit 2de6295 into dev Sep 10, 2022
@speigg speigg deleted the image-component-fixes branch September 10, 2022 22:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Positional Audio settings don't work
2 participants