Require full tileset JSON path #6678
Conversation
|
Welcome to the Cesium community @OmarShehata! Can you please send in a Contributor License Agreement (CLA) so that we can review and merge this pull request? I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
|
@OmarShehata, welcome aboard! P.S. You can ignore the CLA message, someone forgot to add you to the list, I'll get it taken care of. |
|
Thanks @OmarShehata! You've been added under AGI's CLA, so you're good to go. |
|
Thanks @OmarShehata, I'm about to review. In the meantime check out CesiumGS/3d-tiles-samples#26. |
|
@OmarShehata you may want to edit your .gitconfig file so that it uses the same email as your github account. That way your commits will show up with your github picture and link to your account. |
|
You can also add additional email addresses to your GitHub profile. |
|
Added an additional email, looks like that fixed the commit profile links! |
| @@ -90,7 +90,7 @@ define([ | |||
| * @constructor | |||
| * | |||
| * @param {Object} options Object with the following properties: | |||
| * @param {Resource|String|Promise<Resource>|Promise<String>} options.url The url to a tileset.json file or to a directory containing a tileset.json file. | |||
| * @param {Resource|String|Promise<Resource>|Promise<String>} options.url The url to a tileset JSON file. | |||
There was a problem hiding this comment.
Also make similar adjustments to the wording (tileset.json -> tileset JSON) in other 3D Tiles related files - Cesium3DTile, Cesium3DTilesetStatistics, and Cesium3DTilesInspectorViewModel
Source/Scene/Cesium3DTileset.js
Outdated
| tilesetResource = resource.getDerivedResource({ | ||
| url: 'tileset.json' | ||
| }); | ||
| basePath = resource.url; | ||
| } | ||
|
|
||
| that._url = resource.url; | ||
| that._tilesetUrl = tilesetResource.url; |
There was a problem hiding this comment.
I think we can just remove _tilesetUrl altogether now.
There was a problem hiding this comment.
And similarly, remove tilesetResource.
| @@ -90,7 +90,7 @@ define([ | |||
| * @constructor | |||
There was a problem hiding this comment.
There are a bunch of examples in the documentation that need to be fixed. Search for url : 'http://localhost:8002/tilesets/Seattle' - I couldn't find other cases.
|
I've resolved all your comments @lilleyse except for removing Does it still seem like it needs to be removed? |
|
For those cases var that = this;
var resource;
when(options.url)
.then(function(url) {
var basePath;
resource = Resource.createIfNeeded(url); |
|
Should be all good now! @lilleyse |
|
Yes, looks good. Thanks @OmarShehata. |
This PR drops support for directory URLs when loading tilesets to match the updated 3D Tiles spec. Updated documentation to remove references to "tileset.json" and to just say "tileset JSON files" instead.
Fixes #6502
@lilleyse can you review when you get a chance?