search

ShokoAnime / ShokoDesktop

Repository for Shoko Desktop
http://shokoanime.com/shokodesktop/
105 stars 25 forks source link

Fixed MLHVM.UpdateAll change processing bug #482

Closed pmcleish closed 8 years ago

pmcleish commented 8 years ago

Modified MainListHelperVM.UpdateAll so that it processes Groups/Series in the correct order. When UpdateAll was called due to executing "Recreate All Groups" it was processing Series changes before Group changes. This resulted in KeyNotFoundException being thrown because the AnimeSeriesVM was referencing AllGroupsDictionary, which was out of date.

This change makes it so that Group changes are processed first so that AllGroupsDictionary is up to date before processing the Series changes.

(I also removed a few redundant dictionary lookups with just Remove/TryGetValue)

da3dsoul commented 8 years ago

Cool I'll test it. Did you figure out the WPF thing?

pmcleish commented 8 years ago

No. I tracked it down to the lbGroups Listbox in GroupFilterAdmin.xaml. I think it is a pre-existing issue. I can do a RecreateAllGroups from the client 10 or more times without issue, but at random that particular listbox's view of the data goes out of sync with the underlying data. I'm unable to reliably reproduce it. It could be a timing thing, I'm not really sure. But I'm somewhat sure it isn't related to my change since all I've done is correctly update a couple of dictionaries.

da3dsoul commented 8 years ago

OK, if it's like that, it's prolly just a design error from long ago

ElementalCrisis commented 8 years ago

I agree, one of the reasons for Client V2. :)

da3dsoul commented 8 years ago

Right PMC isn't in the dev list yet is he. He prolly hasn't heard much conjecture about such topics.

ElementalCrisis commented 8 years ago

He is, I added him awhile back.

da3dsoul commented 8 years ago

@elementalcrisis he's earned his spot, you should grant him a golden name

da3dsoul commented 8 years ago

Oh

da3dsoul commented 8 years ago

I prolly should've merged this into recreateallgroupsrewrite, but the fix should be independent, while necessary, for it.

pmcleish commented 8 years ago

Yeah, there isn't anything specific to do with RecreateAllGroups, It was a bug that was always there, just happened to show up white testing RecreateAllGroups.