Skip to content
This repository has been archived by the owner on Aug 15, 2022. It is now read-only.

Unified asynchronous task usage #332

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ilookha
Copy link

@ilookha ilookha commented Oct 4, 2019

When debugging issue #322, I noticed a bit of inconsistency in task/thread management, especially when it comes to joining all background threads on shutdown/session reset. This pull request is aimed to improve this situation.

  • use BeardedManStudios.Threading.Task for all task needs, instead of System.Threading directly;
  • added thread join support: BMS.Threading.Task.WaitAll();
  • in any infinite loops, use NetWorker.IsActiveSession checks instead of just IsBound, to cover EndingSession requests;
  • NetWorker.EndSession now uses Task.WaitAll instead of an arbitrary 1s wait;
  • fixed NetworkingPlayer.BackgroundServerPing to use non-zero wait time and actually ping the server.

- use BeardedManStudios.Threading.Task for all task needs, instead of System.Threading directly;
- added thread join support: BMS.Threading.Task.WaitAll();
- in any infinite loops, use NetWorker.IsActiveSession checks instead of just IsBound, to cover EndingSession requests;
- NetWorker.EndSession now uses Task.WaitAll instead of an arbitrary 1s wait;
- fixed NetworkingPlayer.BackgroundServerPing to use non-zero wait time and actually ping the server.

Initially meant as a fix for issue BeardedManStudios#322.
@Crazy8ball Crazy8ball self-assigned this Oct 24, 2019
@Crazy8ball
Copy link
Member

Going to require further testing on this to test whether this to verify there are no issues introduced if we were to merge this in.

@Crazy8ball Crazy8ball added the Needs Testing To test and make sure no new bugs are introduced label Oct 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Needs Testing To test and make sure no new bugs are introduced
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants