Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new chapters not showing after v0.2.0 #21

Closed
agilgur5 opened this issue Nov 9, 2019 · 0 comments · Fixed by #22
Closed

new chapters not showing after v0.2.0 #21

agilgur5 opened this issue Nov 9, 2019 · 0 comments · Fixed by #22
Labels
bug Something isn't working

Comments

@agilgur5
Copy link
Owner

agilgur5 commented Nov 9, 2019

Caused by #15's merge logic for persisted chapters.

The logic has 2 issues:

  1. The lists are in chronological, not reverse chronological. Index 0 of the new list may be 1 chapter ahead of index 0 of the old, i.e. old[0] === new[1] (or later index). Should be checking that the identifier, link, is the same for each chapter that's merged
  2. MST throws an error when trying to merge in a node that already exists in the tree, this is what's actually causing new chapters to not show up. This error is swallowed, but it means the whole array of new chapters isn't processed as it'll error out on the very first element (this took a bit of time to debug and find due to the swallowing). Instead of updating each chapter in the list as we go, we should probably just create a new list and then update the whole thing in one go.

The optimal solution to this is probably to create a map of each chapter and then have a list that's just refs in chronological order. This would be like we have with the mangasmap. That way we can just O(1) update each entry in the map and then just create a new ref list from the new chapters. Unfortunately, that would totally break the currently persisted data, so we either need to implement versioning support in mst-persist or do some similar type of workaround for versioning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant