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

[WIP -Debug] debugging bad weenie scrolls #2828

Closed
wants to merge 2 commits into from

Conversation

harliq
Copy link
Collaborator

@harliq harliq commented Mar 17, 2020

For catching bad scrolls/weenies

@gmriggs gmriggs requested a review from Mag-nus March 17, 2020 18:00
@gmriggs
Copy link
Collaborator

gmriggs commented Mar 17, 2020

I forwarded this particular error to @Mag-nus, so that he can investigate the best place to insert such error handling and logging code

I think perhaps the inner database lookup code should just be returning null properly / not crashing there, and then it would maybe be the responsibility of the outer game code that fetches the weenie to check if null / handle / error logging

@harliq
Copy link
Collaborator Author

harliq commented Mar 17, 2020

I agree, this just for DnF at the moment for helping find issues in their stuff. Not really meant for master

@harliq harliq changed the title debugging bad weenie scrolls [WIP -Debug] debugging bad weenie scrolls Mar 17, 2020
Copy link
Member

@Mag-nus Mag-nus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is correct.

I believe adapter was working correctly. The null check should be going higher up in the chain, or somewhere else.

@harliq
Copy link
Collaborator Author

harliq commented Mar 23, 2020

no need for this PR, closing

@harliq
Copy link
Collaborator Author

harliq commented Mar 23, 2020

close

@harliq harliq closed this Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants