Kuoxsr / spindex

A personal tool to generate sounds.json from a folder structure
0 stars 0 forks source link

Move json sorting process to a separate function so it can sort everything in one step #20

Open Kuoxsr opened 4 months ago

Kuoxsr commented 4 months ago

Sound paths are not ordered correctly during test experiments

Affects commit b51a946

While I was working on issue #19, I discovered that sound paths could come out of the get_combined_events function in an unsorted state. This happens in this edge case:

If the incoming json is in a random order, and there are no existing sounds under that event name in the target, the sort process on line 223 does not get called, because the performance-enhancing shortcut on line 212 short-circuits the combination logic because it doesn't think it needs it.

This should never happen as long as this application is always used to generate the incoming json in the first place, because the generated sound event paths are sorted before creating the staging json file, so this is a low-priority bug. I basically tried something during testing that wouldn't be possible under normal operating conditions.

Note:

When I looked at the code to verify my statement about generated events having a sound path sort routine, I realized that this process takes place for each and every sound that gets added to the json. For example, if there is a folder with 200 sounds in it, the process on line 292 of the get_generated_events function would be called at least 200 times, each time sorting more and more records. As the first record is added, it would "sort" one record. Then it would sort two when the second is added, three when the third is added - all the way up to sorting through 200 items when the last record is added. Even if the above bug is low priority, it's probably going to be a good idea to pull the sorting code out into a function that sorts the entire json table at once, rather than trying to sort individual levels separately. As a result, I should re-title this issue, and call it a feature request instead.

Wish me luck: I originally tried to do this to fix issue #19, but scrapped it entirely because I couldn't make it happen and went with the aforementioned commit instead.