3D tiles - always use version query parameter from tileset.json

[hpinkos]

@mramato This is the fix we want, right?

[cesium-concierge]

Thanks for the pull request @hpinkos!

• ✔️ Signed CLA found.
• ❔ CHANGES.md was not updated.
  • If this change updates the public API in any way, please add a bullet point to CHANGES.md.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

---

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

[mramato]

Thanks @hpinkos this fix is definitely what I had in mind. I think we'll need to re-evaluate how Cesium handles query parameters for sub requests, but this definitely makes Cesium more spec compliant when it comes to 3D Tiles.

Looks like you have some failing tests. Once they're fixed, I'll merge.

[hpinkos]

Alright, I think I got everything with tilesetVersion correct now

[mramato]

Why not just move this inside of the if block?

[hpinkos]

I literally put an inline comment above this line explaining that

[mramato]

Wait, I'm supposed to read those things?

Unfortunately, there is still an issue because the code doesn't actually remove the parameter. setQueryParameters only overwrites values if they are supplied, since versionQuery is an empty object, it's essentially a no-op.

//Assume a query parameter
var url1 = new Cesium.Resource('https://cesium.com?v=1');
console.log(url1.url); // https://cesium.com?v=1

//This is basically a no-op
url1.setQueryParameters({});
console.log(url1.url); // https://cesium.com?v=1

//However, this only clears the value, not the parameter
url1.setQueryParameters({ v: undefined});
console.log(url1.url); // https://cesium.com?v

//To actually delete the query parameter
delete url1.queryParameters.v;
console.log(url1.url); // https://cesium.com

So you need an else that deletes it and you can get rid of the tmp object.

[mramato]

And sorry for not reading your comment.

[hpinkos]

Ah, yeah I was looking at how setQueryParamaters works and for some reason I thought that it would remove the value unless you set the useAsDefault argument, but you're right.

[hpinkos]

@mramato I think this is ready now

[mramato]

Thanks @hpinkos! I pushed a minor tweak and will merge when this goes green.