Particle System Sandcastle ES6 Update #10045
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the pull request @srothst1!
Reviewers, don't forget to make sure that:
|
ggetz
reviewed
Jan 27, 2022
| const snowUpdate = function (particle, dt) { | ||
| let snowGravityScratch = new Cesium.Cartesian3(); |
There was a problem hiding this comment.
I like the intention of scoping the variable to the block where it's needed, but this was scoped a level up on purpose. See the section on scratch variables in the Coding Guide.
Co-authored-by: Gabby Getz <gabby@cesium.com>
|
Hi @ggetz! Thank you very much for the feedback. I just added various commits and comments that address your suggestions. |
|
Awesome, thanks @srothst1 ! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #10038
The goal of this pull request is to re-work the Particle System Weather sandcastle demo such that it is functional when the
prefer-constES6 rule is enabled.My initial commits have made the following changes:
/* eslint-disable prefer-const */snowGravityScratchandrainGravityScratchare declared.snowSystemandrainSystemvariables.snowSystemand therainSystemobjects were added to the scene as primitives. Their visibility was toggled using theshow. I suspect that this may have caused performance issues. I added a commit that ensures that only the necessary primitives are kept in the scene.I am now able to run the Particle System Weather demo locally without the line:
/* eslint-disable prefer-const */It has been mentioned various times that the current code is not well structured. As seen above, I made various edits. The biggest thing that stood out to me was how primitives were handled. However, if there is anything specific that still needs to be reworked, I would be happy to take a look at it.