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

AssertionFailure #5

Closed
mschonvogel opened this issue Feb 1, 2021 · 11 comments
Closed

AssertionFailure #5

mschonvogel opened this issue Feb 1, 2021 · 11 comments
Labels
bug Something isn't working

Comments

@mschonvogel
Copy link

image

Hey, first of all, good work with this library! 🙂 I switched from MessageKit.

I use ChatLayout with RxDataSources and somehow every now and then I'm getting an assertion failure when adding a new message.

I have not yet looked at the layout in detail and wondered if you might know what the problem could be?

@ekazaev
Copy link
Owner

ekazaev commented Feb 2, 2021

@mschonvogel Hi
Thank you very much. Appreciate.

I never checked it with RxDataSources. But it should not be something big. Most likely I am missing something in my math. Can you please provide an example where I can play around with it?

Also, so I can start quicker, can you add print to the ChatLayout.prepare(forCollectionViewUpdates:) and print updateItems for me.
print("\(updateItems)")

I need a log what RXDataSource sends there before you hit an assertion. It seems to be pretty straight forward. For some reason there is less items than the index that is in the item array.

Thank you.

@ekazaev
Copy link
Owner

ekazaev commented Feb 2, 2021

@mschonvogel At the first look it seems that RXDataSources flattens somehow the update commands.

@ekazaev
Copy link
Owner

ekazaev commented Feb 2, 2021

@mschonvogel
Can you also try to replace an else block at line 514 in StateController with the next code:

                } else {
                    guard var item = self.item(for: indexPath.itemPath, kind: .cell, at: .beforeUpdate) else {
                        assertionFailure("Internal inconsistency")
                        return
                    }
                    item.resetSize()
                    var newIndexPath: IndexPath!
                    for (sectionIndex, section) in afterUpdateModel.sections.enumerated() {
                        if let itemIndex = section.items.firstIndex(where: { $0.id == item.id }) {
                            newIndexPath = ItemPath(item: itemIndex, section: sectionIndex).indexPath
                        }
                    }
                    afterUpdateModel.replaceItem(item, at: newIndexPath)
                    reloadedIndexes.insert(newIndexPath)
                }

If ypu wont get a crash It will give me some ideas about what is going on. But of corse an example with the crash is ideal.

@ekazaev ekazaev added the bug Something isn't working label Feb 2, 2021
@mschonvogel
Copy link
Author

@ekazaev thank you for the quick reply

I've added RxDataSources to the example but I'm not able to replicate the bug in there: https://github.com/mschonvogel/ChatLayout/tree/rxdatasources

print("(updateItems)"):

[D(0,0), D(0,1), R(0,30)]
Fatal error: Internal inconsistency: file ChatLayout/SectionModel.swift, line 155

@ekazaev
Copy link
Owner

ekazaev commented Feb 2, 2021

Thank you @mschonvogel
Ill have a look soon. I think I know what is going on. So expect that it will be fixed soon.

@mschonvogel
Copy link
Author

Thank you @ekazaev. Btw replacing the block at line 514 helped, it's not hitting the assertionFailure anymore.

However, prepare(forCollectionViewUpdates:) seems to get called twice now which leads to an ugly insert animation:
print("(updateItems)"):

[D(0,0), D(0,1), R(0,25)]
[I(0,0), I(0,25)]

@ekazaev
Copy link
Owner

ekazaev commented Feb 2, 2021

@mschonvogel That is absolutely normal that the prepare(forCollectionViewUpdates:) is being called twice or more times. It meana that the performBatchUpdates is also being called twice. But why it happens - its already up to you and how you detect the changes in your model. I would recommend you to check your model. and the comparison there. The chain of the commands has no sense. It deletes the cell and [0, 0] and then inserts it [0, 0]. Are you sure it is what suppose to happen? Why its not just calling reload [0, 0].
I cant say exactly how RXDataSources works. But in terms of DifferenceKit the identifier of said cell should stay the same but the content should be flagged as different. To control that is on your side. But Ill include the fix for your reload situation in the upcoming update after the proper testing. Ugly animation usually mean the model problem.

I can offer you to have a call so we can have a look at your particular situation together.

@mschonvogel
Copy link
Author

@ekazaev Okay, I found the problem. 🤦‍♂️ I am using a Firebase Firestore query listener with a limit of 25. Whenever I insert a new message, an older one gets deleted. that's why the deletes were at the beginning of the collection.

Now everything looks fine when I insert a new message...:
[R(0,34)]
[I(0,35)]

I'll test this some more, but everything should work as expected now. I'm happy to jump on a call, to test the changes you made to the lib if it helps

@ekazaev
Copy link
Owner

ekazaev commented Feb 2, 2021

@mschonvogel Good to know. Thank you.
I have just released the 1.1.1 version. It contains performance improvements and also a fix for your issue. I neede to change the processing order. I covered your case with a unit test and it seems to be fine. Please check it out.

Please do not hesitate to contact me on twitter @ekazaev

@ekazaev ekazaev closed this as completed Feb 2, 2021
@mschonvogel
Copy link
Author

Thank you, it works as expected now!

@ekazaev
Copy link
Owner

ekazaev commented Feb 2, 2021

@mschonvogel Fantastic! Happy to hear!

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

No branches or pull requests

2 participants