Creating tileset assumption fix #3435
Conversation
| @@ -7,6 +7,9 @@ define([ | |||
| '../Core/destroyObject', | |||
| '../Core/DeveloperError', | |||
| '../Core/Event', | |||
| '../Core/getBaseUri', | |||
| '../Core/GetExtensionFromUri', | |||
|
The code looks mostly good. There may be an issue if the url is given as a directory without a trailing slash, but you can check for that in the tests. |
|
Do you want to also address CesiumGS/3d-tiles#53 (comment) in this PR? |
|
Updated. |
|
|
||
| this._url = url; | ||
| this._tilesJson = url + 'tiles.json'; | ||
| this._tilesJson = url + fileName; |
There was a problem hiding this comment.
I think this code can be cleaner. A structure like this might be better:
var url;
var tilesJson;
if (getExtensionFromUri(url) === 'json') {
// Set url and tilesJson
} else {
// Set url and tilesJson
}
this._url = url;
this._tilesJson = tilesJson;
|
Updated. |
| } else { | ||
| if (url.lastIndexOf('/') !== (url.length - 1)) { | ||
| url = appendForwardSlash(url); | ||
| } |
There was a problem hiding this comment.
You don't need to check for the trailing slash here because appendForwardSlash should handle that.
|
If I don't check for the training slash and there already is one then I would have two slashes, wouldn't I? |
|
|
|
Oh, gotcha. That's cool. |
|
Updated. |
|
I tweaked the test descriptions a little bit, but this is good to go. Merged. |
Creating tileset assumption fix
|
Thanks Sean |
Fixes #3404
I believe this fixes the issue, could you take a look before I write some tests.