Don't overwrite scratch variables in entity visualizers #6815
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 #6813
The example provided in #6813 was using a
CallbackProperty
forposition
and not using theresult
parameter. We were then overwriting the class scopedposition
scratch variable with the position returned from theCallbackProperty
.BillboardVisualizer
,LabelVisualizer
andPointVisualizer
all had this problem. None of the other visualizers did.I changed the
position
variable to be in the function scope, and renamed all of the scratch variables toxxxScratch
to avoid this kind of problem in the future.I had no idea how to add a unit tests for this, but I think this kind of fix is unlikely to break again so I think it's okay without a new unit test.