search

ppy / osu-framework

A game framework written with osu! in mind.
MIT License
1.67k stars 420 forks source link

Fix `RearrangeableListContainer<>` crashing on replacement operations #6424

Closed smoogipoo closed 1 week ago

smoogipoo commented 1 week ago

When handling range replacements, it first does a remove, then a sort, then an add, then a sort. The sort is intended to operate on drawable objects, but these do not yet exist because they haven't been added yet. In other words, the sort in-between the add and remove is causing the crash.

I've gone with the likely least-regressing path of making the sortItems() handle this case. I'd considered moving the events to the callback and doing a sort of "transaction", but there's quite a bit involved due to the sometimes-asynchronous operation of this container.
Maybe things could be better if we removed the asynchronous Add...

bdach commented 1 week ago

Maybe things could be better if we removed the asynchronous Add...

With the kinds of usages we've got of this client-side, I don't think we can.

smoogipoo commented 1 week ago

Yeah I mean I'm not even going to explore that route right now, it's working fine as it is minus this bug... Mentioned it in passing.