updated sandcastle particle system example for better visual #6132
Conversation
|
@hanbollar, thanks for the pull request! Maintainers, we have a signed CLA from @hanbollar, so you can review this at any time. I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
There was a problem hiding this comment.
Thanks @hanbollar ! Can you clean up the indentation so the diff is readable for a full review?
| @@ -137,298 +137,301 @@ | |||
| </div> | |||
|
|
|||
| <script id="cesium_sandcastle_script"> | |||
| function startup(Cesium) { | |||
| 'use strict'; | |||
| function startup(Cesium) { | |||
There was a problem hiding this comment.
shift-tab this block, or else it will show up with an extra tab in Sandcastle.
CHANGES.md
Outdated
| @@ -31,6 +31,7 @@ Change Log | |||
| * Fixed a bug when creating billboard and model entities without a globe. [#6109](https://github.com/AnalyticalGraphicsInc/cesium/pull/6109) | |||
| * Added support for vertex shader uniforms when `tileset.colorBlendMode` is `MIX` or `REPLACE`. [#5874](https://github.com/AnalyticalGraphicsInc/cesium/pull/5874) | |||
| * Fixed applying a translucent style to a point cloud tileset. [#6113](https://github.com/AnalyticalGraphicsInc/cesium/pull/6113) | |||
| * Fixed sandcastle Particle System example for better visual | |||
There was a problem hiding this comment.
Link the the issue your fixing, (or the PR)
|
Thanks @ggetz ! Updated with your changes. |
| @@ -244,7 +244,7 @@ | |||
|
|
|||
| //Load the Cesium plane model to represent the entity | |||
| model : { | |||
| uri : '../../SampleData/models/CesiumAir/Cesium_Air.gltf', | |||
| uri : '../../../../Apps/SampleData/models/CesiumAir/Cesium_Air.gltf', | |||
There was a problem hiding this comment.
I don't think these need to be changed, this path should work with both the built and non-built versions of sandcastle.
| @@ -254,7 +254,7 @@ | |||
|
|
|||
| var scene = viewer.scene; | |||
| var particleSystem = scene.primitives.add(new Cesium.ParticleSystem({ | |||
| image : '../../SampleData/fire.png', | |||
| image : '../../../../Apps/SampleData/fire.png', | |||
| new Cesium.ParticleBurst({time : 10.0, minimum : 50, maximum : 100}), | ||
| new Cesium.ParticleBurst({time : 15.0, minimum : 200, maximum : 300}) | ||
| ], | ||
|
|
||
| lifeTime : 16.0, | ||
|
|
||
| //emitter : new Cesium.SphereEmitter(5.0), |
| new Cesium.ParticleBurst({time : 10.0, minimum : 50, maximum : 100}), | ||
| new Cesium.ParticleBurst({time : 15.0, minimum : 200, maximum : 300}) | ||
| ], | ||
|
|
||
| lifeTime : 16.0, | ||
|
|
||
| //emitter : new Cesium.SphereEmitter(5.0), | ||
| emitter : new Cesium.CircleEmitter(0.5), |
There was a problem hiding this comment.
If we're starting with the Sphere emitter selected, shouldn't this be a Sphere and not Circle?
| onselect : function() { | ||
| particleSystem.emitter = new Cesium.SphereEmitter(2.5); | ||
| } | ||
| }, { | ||
| text : 'Circle', | ||
| onselect : function() { | ||
| particleSystem.emitter = new Cesium.CircleEmitter(0.5); |
There was a problem hiding this comment.
Do any of these other emitters need tweaks to their size to look good with the changes>
CHANGES.md
Outdated
| @@ -31,6 +31,7 @@ Change Log | |||
| * Fixed a bug when creating billboard and model entities without a globe. [#6109](https://github.com/AnalyticalGraphicsInc/cesium/pull/6109) | |||
| * Added support for vertex shader uniforms when `tileset.colorBlendMode` is `MIX` or `REPLACE`. [#5874](https://github.com/AnalyticalGraphicsInc/cesium/pull/5874) | |||
| * Fixed applying a translucent style to a point cloud tileset. [#6113](https://github.com/AnalyticalGraphicsInc/cesium/pull/6113) | |||
| * Fixed sandcastle Particle System example for better visual [#6132](https://github.com/AnalyticalGraphicsInc/cesium/pull/6132) | |||
…s first emitter used. updated changes.md. modified circle emitter's parameters for vis
|
@ggetz posted with updates and in regards to updating other emitters - updated others already with tweaks; however, latest commit also includes minor change to circle emitter's parameter. |
|
Thanks @hanbollar, just make sure you address #6132 (comment) |
|
@ggetz woops thought I got that. Thanks for the catch. Should be fixed now. |
|
Looks good! Thanks @hanbollar! |
fixes #5583