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

Fixes dynamically created a-scene attributes #5096

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rockwalrus
Copy link

Description:
document.createElement("a-scene").setAttribute doesn't work
correctly when there is already a registered system and component
with the same name.

Changes proposed:

  • Call the ANode implementation if there are no systemNames yet.

document.createElement("a-scene").setAttribute doesn't work
correctly when there is already a registered system and component
with the same name.
@dmarcos
Copy link
Member

dmarcos commented Aug 22, 2022

Thanks. Can you provide a concrete example of expected vs. what you get behavior?

@rockwalrus
Copy link
Author

Well, the test case shows the issue pretty well. What happens if you create your a-scene after A-Frame initializes is that in certain cases some of the attributes on the element never get set, leading to misconfigured systems. I noticed this with gltf-model but it's true at least for all attributes that correspond to systems with the same name as their associated component.

My expectation is that A-Frame should handle system attributes correctly regardless of whether a-scene is created before or after A-Frame initializes. Note that I'm only ever creating one a-scene and this is the only apparent problem with dynamically creating it.

@dmarcos
Copy link
Member

dmarcos commented Aug 25, 2022

Can you provide some code that reproduces the issue? https://glitch.com/~aframe highly recommended

@rockwalrus
Copy link
Author

It's at https://aframe-dynamic-system-bug.glitch.me . Check the console logs to see the problem.

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

Successfully merging this pull request may close these issues.

2 participants