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

Empty sequence crash #27

Merged
merged 4 commits into from
Mar 7, 2022
Merged

Empty sequence crash #27

merged 4 commits into from
Mar 7, 2022

Conversation

tcwatson
Copy link
Owner

@tcwatson tcwatson commented Mar 7, 2022

Potential fix for #26. Wasn't able to reproduce the bug, but looking through the code, this could avoid a potential race condition, in which a player leaves the squad or map before the PlayerDisplay has been added to the _playerDisplays.

tcwatson added 3 commits March 6, 2022 17:14
…PlayerDisplay has been created. Retry 3 times to find the display in case it is eventually created, so it can be removed from the current party
Logger.Error($"Caught exception while trying to get PlayerDisplay to remove from Current Squad Members: {ex}");
playerDisplay = null;
return false;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to use a try/catch block, you can replace First by FirstOrDefault, which returns null instead of throwing when nothing has been found

Copy link
Owner Author

Choose a reason for hiding this comment

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

I've pushed a change for this - couldn't see a need to keep the logging statement. Anything you'd log here?

@sonicolasj
Copy link
Collaborator

Also, did you include your WIP README on purpose?

@tcwatson
Copy link
Owner Author

tcwatson commented Mar 7, 2022

Also, did you include your WIP README on purpose?

I didn't, but I don't mind including it now anyway.

@sonicolasj sonicolasj merged commit 02bea33 into main Mar 7, 2022
@sonicolasj sonicolasj deleted the empty-sequence-crash branch March 7, 2022 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants