Add readyPromise to ImageryProviders #3175
Conversation
|
@klim705 this looks good to me! Are you going to add a |
|
@hpinkos It's not necessary at the moment, so I can add it to the terrain providers once we get there, if needed. Thank you! |
|
Actually, @hpinkos makes a good point, for completeness we should have readyPromises for terrain as well, I think this will take care of everywhere in Cesium we currently have a |
|
Okay, I'll just add it to terrain as well and add to this PR then. |
|
Great! Let me know when it's ready and I'll take a look. |
|
Added to terrain providers. I found the |
|
Lots of people have asked for a |
|
Billboard is a different beast, so I wouldn't worry about it. The other ones are private, so no need to worry about them either. |
| */ | ||
| readyPromise : { | ||
| get : function() { | ||
| return when.resolve(true); |
There was a problem hiding this comment.
You should define this._readyPromise = when.resolve(true); in the constructor and then simply return it here. The code as it is now returns a different promise every time.
|
One last thing, in providers that aren't always ready, I'm pretty sure there is an error handler somewhere that spits out a console message on failure. We should |
|
Other than that, this looks great. Thanks @klim705! and congrats on your first Cesium PR. |
|
@klim705 can you add some tests to make sure the promise gets rejected for providers where that can happen? Other than that this looks great and I'll merge as soon as those changes are in. |
|
@mramato Added rejection tests. |
|
Thanks again @klim705. |
Add readyPromise to ImageryProviders
|
I updated CHANGES in master. |
Added promise property (similar to
ModelandPrimitive'sreadyPromise) to each imagery provider that resolves to true when the provider is ready for use.