Improve lobby restrictions

[jauggy]

Improvements

1. If a player doesn't meet lobby play restrictions, the Coordinator will now inform them.
2. When checking rating requirements, the lobby team size and team count is looked at to determine what rating type to use. It is the max team size that matters not number of players in the lobby since the lobby may be getting filled. E.g. a lobby with team size 8 and only 2 players will use "Team" rating. Fixes #85 raised by Randy.
3. When the server thinks you are trying to create a noob lobby, some tips will be given by coordinator
4. minchevlevel and maxchevlevel commands
5. Remove filters in lobby name (and lobby) when everyone leaves. Fixes #171 raised by Fireball.

Chev level commands

While other devs may want to use uncertainty filters, chev restrictions have the benefit that:

1. They are already used (just manually enforced)
2. They are more visual than uncertainty
3. A lot of information is lost due to uncertainty reset. For example I am 3Chev and have played 46 matches. My uncertainty is 5. Someone who has been playing for years and has thousands of games has the same uncertainty as me due to season uncertainty reset.

However, I acknowledge there are flaws with chevs which include: they can grow with spec time and pve time.

Contributors and Tournament Winners

Contributors chev level is not tied to hours played. Therefore I just allow contributors to ignore chev restrictions. This can be revisited in the future.

Tournament winners have chevlevel 8 and are treated as such. I don't think this is a big issue as they are likely to only play in min chev lobbies.

Other nuances

Teifion added code so that you cannot add restrictions for "All welcome" lobbies. It checks the lobby title (and is trivial to bypass). I did the same for chev restrictions.

I currently don't support having both min and maxchevlevels at the same time. Setting one will reset the other. I assume maxchevlevel will be used on noob lobbies and it wouldn't make sense to have a minchevlevel on a noob lobby. Can be revisited in the future if needed.

Merge conflicts

This PR has been written before Lexon has merged his big/small team rating split. If he merges first I only need to change a couple lines in the check_rating_to_play function inside LobbyRestrictions module.

    rating_type =
      cond do
        team_count > 2 && team_size == 1 -> "FFA"
        team_count > 2 && team_size > 1 -> "Team FFA"
        team_size == 1 -> "Duel"
        team_size <= 4 -> "Small Team"
        true -> "Big Team"
      end

Unit Tests

In VSCode, right click the lobby_restrictions_test.ex file and copy its path. Then run

mix test path_to_file

Integration Test Steps

If a player doesn't meet lobby play restrictions, the Coordinator will now inform them. In a lobby use the minratinglevel or maxratinglevel commands. Try and move from spec to player when you don't meet requirements. You will be informed by the Coordinator on the right.

When checking rating requirements, the lobby team size and team count is looked at to determine what rating type to use. It is the max team size that matters not number of players in the lobby since the lobby may be getting filled. E.g. a lobby with team size 8 and only 2 players will use "Big Team" rating. A lobby with team size between 2 and 5 will use "Small Team" rating.

Try the above but try changing team size/team count drop downs and note the warning given by Coordinator changes.

When the server thinks you are trying to create a noob lobby, some tips will be given by coordinator Try $rename noob and see the coordinator message on the right.

minchevlevel and maxchevlevel commands Type $help and look for the new minchevlevel and maxchevlevel commands and how to use them. Test them out. Check that when you don't meet requirements to play the Coordinator tells you. Note that $minchevlevel 1 will not work as that is everyone. If you are a contributor, you ignore all chevron restrictions.

Remove filters in lobby name when everyone leaves While in lobby run $status to see the restrictions you set up earlier. Check that the lobby name has the restrictions. Leave the lobby. Note the change in title. Enter the lobby and run $status and note the lobby restrictions are gone.

[jauggy]

@geekingfrog thanks for review. I think I can do the changes you have requested and will work on it.

Currently I use some formatter in VSCode but it is not set to auto format on save. So I must manually format the document. I could try and set it to autoformat on save.

Also they currently have a staging server for integration tests. It was used for testing small/big team ratings.

[jauggy]

@geekingfrog I tried to do the changes. However, now getting failures on mix dialyzer. Have no idea why. The changes have been pushed. Maybe you have insight.

[geekingfrog]

What is the dialyzer command you run, and what is the error you get?

[geekingfrog]

Also they currently have a staging server for integration tests. It was used for testing small/big team ratings.

When I said "integration test" I meant more something that is run with mix test, but instead of testing just one small bit of code like the pure functions you did (unit test), crafting some tests that cover a wider range, basically: setup the system, connect a fake client, and test the commands and response you get. Like this kind of test for example.

[jauggy]

What is the dialyzer command you run, and what is the error you get?

mix dialyzer

However, I solved it and have updated the branch. Well actually I didn't solve it - I asked on Elixir forums.

[jauggy]

Also they currently have a staging server for integration tests. It was used for testing small/big team ratings.

When I said "integration test" I meant more something that is run with mix test, but instead of testing just one small bit of code like the pure functions you did (unit test), crafting some tests that cover a wider range, basically: setup the system, connect a fake client, and test the commands and response you get. Like this kind of test for example.

I just tested those spring tests and they don't work. I think we should fix those first (and can be done in a separate ticket) so I have some sort of template to follow and understand how they "should" be written.

[geekingfrog]

FFS, this PR is still not merged.

Well, fuck it, test or not I don't care anymore, ship it.

[StanczakDominik]

FFS, this PR is still not merged.

Well, fuck it, test or not I don't care anymore, ship it.

That's on me and my getting pulled apart in multiple directions. I'm trying to stay on top of things, but it can be hard at times. Thanks for your understanding! 😞

I've just merged that one now.

[jauggy]

@StanczakDominik I've pushed your suggested changes.

These next few days aren't the best time for VC as I'm fairly busy. I'm in the office during the day and for the next couple of evenings I have plans. (Also I'm in Sydney time).

My plan for this PR was to request at some point it to be placed on integration server and then create a thread in the testers channel. There I could ping @GeneralTesters and also individuals who would be interested in these changes. The following individuals should have an interest:

1. Randy raised a ticket that restrictions were using the wrong rating for 1v1. This PR fixes #85
2. Fireball raised a ticket that restrictions were not being cleared when players leave the lobby. This PR fixes #171
3. Spreezy raised a suggestion in Discord for min/max chev level commands.
4. And finally Sun_Tzu wants to get more investment into my balancer and I snuck in an advertisement for it when you create a noob lobby. He also requested chev filter on discord.

Spreezy and Sun_Tzu are pretty active on Discord and those two alone could go through all the test cases. And there would be random GeneralTesters who could probably assist. The tests are straight forward - they don't require starting a match as I didn't change the match library.

As for the automated integration tests, I had a brief look and my skills aren't strong enough to even create them for this PR. I would need to invest more time in levelling up my skills. But if you're planning to look into maintaining Hailstorm then I can look into seeing if I can create some tests for existing work I've done i.e. the balancemode. I did look at Hailstorm in the past and understand a bit but need to revisit.

[jauggy]

Actually maybe it's best that I make some changes that would cause a merge conflict with Lexon. That would make us more cautious when merging together since it would generate an error. I want to add this to match_lib.ex

    cond do
      String.contains?(bot_names, "Scavenger") -> "Scavengers"
      String.contains?(bot_names, "Chicken") -> "Raptors"
      String.contains?(bot_names, "Raptor") -> "Raptors"
      Enum.empty?(bots) == false -> "Bots"
      true -> get_pvp_rating_type(Enum.count(teams), max_team_size)
    end
  end

  def get_pvp_rating_type(team_count, team_size) do
    team_count == 2 and team_size == 1 -> "Duel"
    team_count == 2 -> "Team"
    team_size == 1 -> "FFA"
    true -> "Team FFA"
  end

And then I would add this to lobby_restrictions.ex

      rating_type = MatchLib.get_pvp_rating_type(team_count, team_size)

This would conflict with Lexon's change here. But having the error on merge might be good reminder to adjust it as needed.

[L-e-x-o-n]

Teiserver.Coordinator.ConsulCommandsTest already has some tests for lobby restrictions, including min/max/setranklimit. I suggest updating those or moving them to your new test file.

[L-e-x-o-n]

You mention the possible merge conflict with my changes. This is a pretty big PR, I think it would be nice if it could be split into maybe 2 smaller ones. It would make things easier to merge, specifically the fix for #85, that is something I was planning to solve in my PR to avoid issues with new rating categories.

[jauggy]

@L-e-x-o-n If you can make a function in your PR that takes team count and team size and returns a rating type then I'll just wait for you to merge and use that. From what I read of your PR, we actually currently only conflict once in consul_server. It's not hard for me to resolve later.

[L-e-x-o-n]

@L-e-x-o-n If you can make a function in your PR that takes team count and team size and returns a rating type then I'll just wait for you to merge and use that. From what I read of your PR, we actually currently only conflict once in consul_server. It's not hard for me to resolve later.

I missed this comment, but that's exactly what I did :D MatchLib.game_type/2 PR has been merged now

[jauggy]

Have now merged with master. Main change is here calling Lexon's recently made function https://github.com/beyond-all-reason/teiserver/pull/303/commits/c7515acc8b9012200ce5ae6b579e1135492d6377

Have tested and works locally

[StanczakDominik]

Test results from the integration server

``` ######################################## This lobby has the following play restrictions: Min rating: 20 ######################################## --------------------------- You don't meet the rating requirements for this lobby (Min rating: 20). Your Big Team match rating is 16.67. Learn more about rating here: https://www.beyondallreason.info/guide/rating-and-lobby-balance#openskill --------------------------- You don't meet the rating requirements for this lobby (Min rating: 20). Your Big Team match rating is 16.67. Learn more about rating here: https://www.beyondallreason.info/guide/rating-and-lobby-balance#openskill --------------------------- Balance logs, mode: solo Tried grouped mode, got a deviation of 55 and reverted to solo mode Picked (...) for team 1, adding 41.94 points for new total of 41.94 Picked (...) for team 2, adding 18.8 points for new total of 18.8 Deviation of: 55 Team 1 - sum: 41.9, mean: 41.9, stdev: 0.0 Team 2 - sum: 18.8, mean: 18.8, stdev: 0.0 --------------------------- You don't meet the rating requirements for this lobby (Min rating: 20). Your Duel match rating is 16.67. Learn more about rating here: https://www.beyondallreason.info/guide/rating-and-lobby-balance#openskill --------------------------- You don't meet the rating requirements for this lobby (Rating between: 20 - 21). Your Big Team match rating is 16.67. Learn more about rating here: https://www.beyondallreason.info/guide/rating-and-lobby-balance#openskill --------------------------- You don't meet the rating requirements for this lobby (Rating between: 20 - 21). Your Big Team match rating is 16.67. Learn more about rating here: https://www.beyondallreason.info/guide/rating-and-lobby-balance#openskill --------------------------- You don't meet the chevron requirements for this lobby (Min chev: 4). Your chevron level is 1. Learn more about chevrons here: https://www.beyondallreason.info/guide/rating-and-lobby-balance#rank-icons --------------------------- You don't meet the rating requirements for this lobby (Min rating: 40). Your Big Team match rating is 21.37. Learn more about rating here: https://www.beyondallreason.info/guide/rating-and-lobby-balance#openskill --------------------------- You don't meet the rating requirements for this lobby (Min rating: 40). Your Small Team match rating is 18.78. Learn more about rating here: https://www.beyondallreason.info/guide/rating-and-lobby-balance#openskill --------------------------- You don't meet the rating requirements for this lobby (Min rating: 40). Your Duel match rating is 31.48. Learn more about rating here: https://www.beyondallreason.info/guide/rating-and-lobby-balance#openskill --------------------------- # switch to FFA preset You don't meet the rating requirements for this lobby (Min rating: 40). Your Duel match rating is 31.48. Learn more about rating here: https://www.beyondallreason.info/guide/rating-and-lobby-balance#openskill --------------------------- # switch to teamsize = 1 You don't meet the rating requirements for this lobby (Min rating: 40). Your FFA match rating is 19.67. Learn more about rating here: https://www.beyondallreason.info/guide/rating-and-lobby-balance#openskill ```

In short, the only thing I was able to find was, immediately after switching to FFA preset, it was trying to gate on Duel rating (teamsize = 1), but explicitly switching to teamsize = 1 (or possibly changing the number of teams?) seemed to fix it.

Overall, I think it's great - I'll just confirm a few things before merging this.