search

mucklet / mucklet-client

Web client for Mucklet.com, a textual world of roleplay.
Apache License 2.0
5 stars 8 forks source link

SortOrder missing in CharLog Overlays causes removal to target wrong indices in Modapp's Collection #275

Open ItsAKelmi opened 3 months ago

ItsAKelmi commented 3 months ago

This one was a doozy to debug...

Context: I experienced a weird issue when removing the 'nav' overlay and then adding it and removing it again, causing the 'mobileNav' overlay to be removed instead.

When the sortOrderCompare function returns NaN, the array binary search functions used by Modapp's Collection do weird things. In this case, this causes the array index 1 to be returned in error, leading to the removal of the wrong entry. This feels like a potentially major problem/footgun for modapp's users, potentially.

Not sure whether the real fix is to remove the compare property from here or to add sortOrder to all Overlays.

anisus commented 2 months ago

It sounds like a bug in Collection indeed. I will try to make a unit test for it and see if I can replicate it.