Make sure that hasLoaded flag is only true when the loaded event has fired. Alternative implementation of commit 924dc00 (fix #5425)

[dmarcos]

No description provided.

[dmarcos]

@mrxz this should cover all the issues of #5425

I introduced an isLoading flag different that hasLoaded (only set before 'loaded' fires) and also a loaded-private (don't love the name) event analog to loaded but "private" so A-Frame can handle internal stuff before the public event fires.

I tested on your glitches and added test to cover those scenarios. Can you also validate on your side? Thanks

[dmarcos]

Glitch with build for convenience:

https://glitch.com/edit/#!/lemon-magnificent-badge?path=index.html%3A1%3A0

[mrxz]

Perhaps update this comment to something like "... until the entity starts loading"

[mrxz]

Thanks, this does indeed seem to cover all the issues. Did some more testing and didn't find any other regressions, so seems good to go.

The naming of the loaded-private event could use some work, but it's tricky to come up with a suitable one.

[dmarcos]

@mrxz Thanks for testing it. Just one request. I copied your glitch test case from #5425 (comment) verbatim to the tests include in this PR. They are a tad complicated / convoluted and will be hard to understand and maintain. Do you think you can simplify them a bit? Thanks

[dmarcos]

@mrxz just ping in case you have some bandwidth to simplify your test cases so I can incorporate them to the unit tests. Thanks so much

[mrxz]

@dmarcos Gave it a shot. The test cases included visual output, but as unit tests that's not relevant. However they do all rely on specific order of execution (things taking place during isLoading), so I'm afraid they still remain somewhat complex.

See my attempt at simplifying them: mrxz@2ff8fac

In summary:

• The double update test now uses the (built-in) visible component, as the erroneous behaviour occurs with any component, so no need to create a test-specific one
• Remove geometry from the mixin test and changed test name to align with surrounding tests
• Simplified primitive and updated naming to better describe what the component does.

[dmarcos]

Thanks. I cherry picked your commit