Simple implementation of MM map veto #2682
Conversation
| .OrderByDescending(x => x.MapSupportLevel) | ||
| .ThenBy(x => x.InternalName) |
There was a problem hiding this comment.
I think applying a secondary alphabetical sort is a fine change to make in all cases, but I can restrict it to the mapSupportLevel filter case if necessary
Legomenon-gh
force-pushed
the
mm-veto
branch
2 times, most recently
from
4c74256 to
c17e74b
Compare
Infrastructure for users to manage their matchmaking map bans. This commit only includes supporting code with no changes to the matchmaking map picker behavior.
|
I don't have time to review it in detail now, if you want I can merge it so you see how it behaves on the test server. |
|
Thanks! Pushing to the test server would be great, feel free to do that whenever convenient and I'll test the bans more properly there. |
| { | ||
| public class MapBanConfig | ||
| { | ||
| private static Dictionary<int, int> BansPerPlayerByGameSize = new Dictionary<int, int>() { |
There was a problem hiding this comment.
Why is this fixed in the code? Shouldn't the number of bans depend on the pool size and number of players?
There was a problem hiding this comment.
That's an excellent question! I didn't think of using the map pool size dynamically but that's a much nicer solution. The current code technically works because the size of the pool is pretty static and the team pool is bigger than the 1v1 pool, but it's more complicated, more brittle and pretty ugly, so yeah I'll fix it :D
New logic will be "take the number of maps in the pool and find the nearest integer that bans no more than a configurable 75% or 80% of the maps. Configure the number of bans displayed in the UI separately".
| var maps = db.Resources.AsQueryable(); | ||
| if (mapSupportLevel != null) | ||
| { | ||
| maps = maps.Where(x => x.MapSupportLevel == mapSupportLevel); |
There was a problem hiding this comment.
Usually when support level is set somewhere, it's a minimum, i.e. MapSupportLevel >= mapSupportLevel. For MatchMaker this doesn't make a difference.
|
|
||
| <div style="margin-bottom:10px"> | ||
| <span style="padding-right:10px;">Ban @(i+1):</span> | ||
| <span>@Html.TextBox("mapName", mapName, new { data_autocomplete = Url.Action("MatchMakerMaps", "Autocomplete")})</span> |
There was a problem hiding this comment.
This may be a bit cleaner using the Model and TextBoxFor. For an example look at https://stackoverflow.com/questions/38205799/using-html-textboxfor-to-loop-through-items-in-a-model
|
This sanity checks on test.zero-k.info, I'll open a separate PR to address DeinFreund's comments in the next couple of days and this should hopefully be good to go |
Addresses #2444. I have experience in other web frameworks but no previous experience in C#, ASP or this specific codebase, so please check for stupid errors or code improvements I'm not aware of.
The first commit contains all MVC architecture, and the second commit is only the changes to the matchmaker itself. The second commit is the one most likely to cause problems, so it can be reverted on its own.
Testing I've done locally:
I cannot verify actual hosting of a game locally due to insufficient memory on my machine, or more than 1v1. I don't believe there's a working test instance for matchmaking at present, so it's probably worth having someone else sanity check it locally if they can.
Room for future improvement: have separate bans for 1v1 and team games and a less spartan UI. Right now applying bans to maps only used in team games requires "losing" bans for 1v1, and selecting maps is a little confusing since all matchmaking games are returned and it's not clear which ones are only used in 1v1. This is probably fine for now since team game MM is pretty niche, and it can be worked around (by increasing the total number of bans per user or just having users editing their bans to be more targeted at team games).