Problems with No Repetition Code

[FtXCommando]

It seems that the no repetition code that has people not play one of their last 3 maps is no longer functioning with the introduction of the new pools. I'm noticing people playing the same map repeatedly.

I don't think it has anything to do with people transitioning between pools and the code considering the same map in two different pools to be two different maps as the example below consists of two people facing one another for two games in a row on the same map twice in a row. They should have had the same map pool for both games. They were both also 1400 (rounded) in both games, meaning they were not on the edge of switching between two pools.

I also don't think it has anything to do with the code bugging out due to there being no viable map to pick, as the smallest map pool is 7 which means in the worst case scenario, there is still 1 viable map for the code to pick with the 3 map restriction.

[Askaholic]

Thanks FTX!

I think this definitely has something to do with it:

server/server/ladder_service.py

Line 468 in 83a1b48

result.extend([

those lines should be inside the for loop otherwise only the game data for the last player in the list will be queried. We will have to write some more unit tests to cover this condition.

[0x647262]

@Askaholic I'd be happy to take a look into this

[Askaholic]

According to @FtXCommando this is still an issue on the most recent server version.

[0x647262]

@Askaholic mind reassigning this to me?

Looks like https://github.com/FAForever/server/blob/develop/server/matchmaker/map_pool.py#L25 is the culprit.

[Askaholic]

Sure! I have no leads for you this time though; no idea why it's not working.

[Askaholic]

Really, you think it's choose_map? I thought I had written some good tests for that :P

[0x647262]

Just a hunch for now... ladder_service.pyis still a suspect as well, as we could still just be feeding choose_map bad data.

[Askaholic]

That would have been my guess. I also though that maybe the map pool was just too small, but I don't think so. It seems that the smallest map pool has 7 maps which is enough for both players to throw out 3 maps and still have 1 that neither has played recently.

[0x647262]

After a bit of testing I'm 100% certain we're feeding the choose_map function garbage data. The pool (or ladder anti-repetition limit) has to be just small (or large, in the case of the ladder anti-repetition limit) enough to allow choose_map to offer up a repeat map.

[Askaholic]

I think I found the issue. The problem is actually that the map pools themselves are being fetched incorrectly. In particular they are fetching map id's instead of map version id's. See here:

server/server/ladder_service.py

Line 110 in 4a39a48

map_version.c.map_id,

This should be map_version.c.id since map_version.c.map_id is just the foreign key reference to map.id.

So what happens is that the map paths are still correct for a particular map version, but the id's are just completely wrong. Since the game code actually looks up the version id from the map path when writing the game_stats entry, the game history still uses the correct map_version ids. But when the map pool sees those ids they don't show up in the map list, so it ignores them all and just picks a random map.

I looked at the git blame and it looks like this has been happening ever since we implemented the new map pools. Which was almost a year ago.