beyond-all-reason / teiserver

Middleware server for online gaming
https://www.beyondallreason.info/
MIT License
50 stars 47 forks source link

Add minimal files for split one chevs balancemode #235

Closed jauggy closed 3 months ago

jauggy commented 5 months ago

Problem

Games like this are unfair and it feels like the match is decided by the balancer. If one side has multiple new players and the other side has none, then if you're a semi-experienced player on the side of the new players you will feel the game is unwinnable.

This PR aims to provide an OPTIONAL balance mode that will make a better distribution of one chevs.

Test Steps and Full Details

To test this please see instructions in #218

jauggy commented 5 months ago

Fixes #218

jauggy commented 5 months ago

@badosu this pull request has less changes than the previous pull request but should give the same results.

badosu commented 5 months ago

Thank you! I'll check out when I'm able to

FIr3baL commented 5 months ago

Tested with previous branch and it lgtm https://github.com/jauggy/teiserver/tree/add-balancemode-split-one-chevs default balance

old balance

$balancemode split_one_chevs

balancemode split_one_chevs
FIr3baL commented 5 months ago

Same visual result with the branch of this PR grafik

jauggy commented 5 months ago

@badosu Given that fireball has tested, I think code review alone would be fine if you don't have a dev environment to test this.

However if you want to setup dev environment let me know if you have any issues with teiserver/spads setup. There's some docs here for SPADS.

Also there is not any documentation currently for launching multiple chobbies so let me know if you need that and I will create it.

badosu commented 5 months ago

@jauggy Just to be clear, I'm not the stakeholder for this project, and don't know who that would be.

That said, I want to have a more active role, unfortunately haven't had time to familiarize with the repo myself yet. As soon as I do I'll get back to here. (There are no blockers from me until I can confidently review a PR here)

jauggy commented 5 months ago

As requested by Lexon, I'll be adding some unit tests to this PR in the next couple of days.

jauggy commented 5 months ago

@L-e-x-o-n Tests have been added. I updated the documentation documents/guides/testing.md which will give instructions on how to test just the balance tests.

geekingfrog commented 5 months ago

I think the most important change here is the introduction of DB calls inside the create_balance function. Seeing there is a timer System.system_time(:microsecond) as well, that makes me think this function is somewhat time sensitive and I'm wary of making it not pure by introducing DB calls inside.

jauggy commented 5 months ago

Thanks for the review @geekingfrog . Yeah you caught something that I also wasn't too happy about but at the time I wanted to write minimal code. Now that I have more spare time, I'll see if I can pull the db call outside.

jauggy commented 5 months ago

@geekingfrog As requested, I've added the new changes to this PR

jauggy commented 4 months ago

@geekingfrog if you do end up getting multi chobby working, I have some sample instructions for testing in the issue as well as a crude way of adding sample users.

geekingfrog commented 4 months ago

After some unrelated struggles, I finally tested this branch as of 6422ac12d9b95597d9c59b6d1e30b61a208521df and it looks good.

Using the default balance algorithm: # balance-normal

And then, switching using $balancemode split_one_chevs I get: balance-new

I think it's probably good to be shipped.