Feature/#727 refactor matchmaker for 4v4 party algorithm

[cleborys]

Refactors server/matchmaker/algorithm.py such that the matchmaking algorithm is independent from the queue logic, with the intent that @BlackYps can simply subclass a new Matchmaker to propose a new algorithm.

For now I stuck to minimal/minor changes, although I think these files could still benefit a lot from more attention. For example I'm not convinced that these Matchmakers should have instance variables at all but left that as-is for a chance to get feedback first.

Closes #801.

[cleborys]

Github doesn't seem to re-use files for several diffs where a single file has been split into two. To get smaller diffs, you can compare
the deleted test_matchmaker_queue_algorithm.py to test_matchmaker_algorithm_bucket_teams.py and test_matchmaker_algorithm_stable_marriage.py and the deleted matchmaker/algorithm.py to all of the matchmaker/algorithm/*.py files.

[codecov]

Codecov Report

Merging #749 (edaf13d) into develop (b3b7acd) will decrease coverage by 0.03%.
The diff coverage is 98.67%.

Impacted Files	Coverage Δ
server/matchmaker/matchmaker_queue.py	92.42% <93.75%> (-0.06%)	⬇️
server/matchmaker/algorithm/matchmaker.py	96.42% <96.42%> (ø)
server/matchmaker/algorithm/stable_marriage.py	99.03% <99.03%> (ø)
server/matchmaker/algorithm/bucket_teams.py	99.19% <99.19%> (ø)
server/matchmaker/algorithm/random_newbies.py	100.00% <100.00%> (ø)

[Askaholic]

I agree that there is no point in many of the objects used. If they're just holding 1 instance variable then that could really be moved to a function parameter.

[cleborys]

I finally included the suggestion to return a list of unmatched searches from the matchmakers, rather than taking the difference after the fact.
The remaining codacy issues all look like false positives.

[Askaholic]

Should there be a return here?

[cleborys]

No, I wouldn't say so - it will simply ignore what you passed in and the parameter is only there to comply with the Matchmaker interface. By "ignore" I mean it will continue to execute as if you had passed a team_size of 1 (and implicitly assume that all Searches passed in searches correspond to a single player).

[Askaholic]

Oh ok. So really the parameter is always ignored, and here it's just logging that something weird is going on. Wouldn't it be better to just assert team_size == 1 then to avoid programming errors?