p5.js icon indicating copy to clipboard operation
p5.js copied to clipboard

Fix the Array Comparison bug in outputs.js

Open nikhilkalburgi opened this issue 1 year ago • 5 comments

Signed-off-by: nikhilkalburgi [email protected]

Resolves: #6660

PR Checklist

  • [x] npm run lint passes
  • [ ] [Inline documentation] is included / updated
  • [ ] [Unit tests] are included / updated

nikhilkalburgi avatar Dec 26 '23 13:12 nikhilkalburgi

Hi @ayushanand308, What if we sort them and check?

JSON.stringify(this.ingredients.shapes[f].slice().sort()) !== JSON.stringify([include].slice().sort())

This will create a copy , sort then stringify to compare

nikhilkalburgi avatar Jan 01 '24 13:01 nikhilkalburgi

@nikhilkalburgi That will be very inefficient and considering this will run on every frame, may significantly impact performance if the drawings get complex enough.

limzykenneth avatar Jan 12 '24 13:01 limzykenneth

Hi @limzykenneth, Do you think that the initial condition is necessary as it always returns true? How can it get resolved?

nikhilkalburgi avatar Jan 14 '24 05:01 nikhilkalburgi

In the subsequent for loop there is already a check for existence of the element in the list so the check before that is either not needed (ie. be an else instead of else if or be something that can be completed very quickly. I think for this case not having that earlier check should be fine and is how is has worked until now effectively. The loop can be optimized a bit by breaking early as well.

limzykenneth avatar Jan 21 '24 12:01 limzykenneth

maybe here we can use isEqual function of lodash library to compare them.

HusainModiwala avatar Jan 25 '24 08:01 HusainModiwala