Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unified automatch settings #238

Merged
merged 1 commit into from
May 30, 2014
Merged

Unified automatch settings #238

merged 1 commit into from
May 30, 2014

Conversation

theblankman
Copy link
Collaborator

Fixes #229 and prevents #231 from happening if user sets owned-set automatch preference (see comments in #229 for more details).

any game-specific settings whenever an automatch request is submitted.
@aiannacc
Copy link
Owner

Can you elaborate on how this works now? It looks great, but it isn't obvious to me how the various cases are meant to be handled.

Fo example, do the default automatch settings now apply to quick-game and create-game games that have "Use Automatch" enabled? If so, what should happen when their requirements conflict?

@theblankman
Copy link
Collaborator Author

Default automatch settings now apply to automatch requests from any source. For create-game and quick-game games with Use Automatch, requirements parsed from the table name should override the corresponding default requirements.

The new code in GS.AM.submitSeek that deals with "effectiveReqs" pulls in all the default requirements first. Then it applies requirements from the "seek" parameter, which create-game and quick-game populate, overwriting default requirements with the same class, with some special logic for rating requirements (see keyForReq).

Any new sources of automatch requests in the future would have the same behavior. If they call submitSeek with game-specific settings, the game-specific settings will overwrite defaults where there's a conflict, and use defaults where there's not.

@aiannacc
Copy link
Owner

Okay, I think I follow now. #231 is really two different bugs:

  1. You can fail to get a match because the server doesn't realize that you own all sets.
  2. You can get an inappropriate match because the server doesn't realize that a base-only game isn't acceptable.

This doesn't solve the first bug, but it prevents the second one, correct?

@theblankman
Copy link
Collaborator Author

Right, this prevents (2) assuming the user sets their settings appropriately. The server gets the number-of-sets requirement even if the match request was generated by create-game or quick-game.

I'd expect (1) to be relatively uncommon in the wild... as far as we know it only happens if you hit quick-game too fast (before your owned sets info is populated), and if the server thinks you own no sets but want a game with all sets, for example, it could still pair you with an opponent who owns all sets and have that opponent host.

@aiannacc
Copy link
Owner

I thought so too until I checked the logs. Case (1) is actually happening a LOT. About 2.5% of all automatch requests are broken in this way (claiming no sets owned, not even base).

That's no reason to hold off on merging this code, though. I just haven't had a chance to put it through its paces yet.

@theblankman
Copy link
Collaborator Author

Of that 2.5%, how many fail to match entirely, as opposed to matching to an opponent with correct info, and making the opponent host? A request that's broken this way doesn't mean you can't be matched at all, it just means the server would only put you in a game hosted by an opponent, right?

@aiannacc
Copy link
Owner

Correct. So if your potential match has all sets, there's no problem. The errors that arise are:

  1. Your opponent owns base only and will accept a base-only game
  2. Your opponent owns base only and requires an all-cards game

In case 1, you get a base-only game in violation of your automatch request. This probably isn't happening much or we'd be hearing about it loudly and often.

In case 2, you just don't get matched. This could be happening all the time without anyone being aware.

Unfortunately, I don't have a good way to answer to your question without doing actual programming. I'd need to identify the broken requests and then read through the subsequent log to identify the next occurrence of either a case 1 or case 2 error. It'd probably be easier to just fix the bug. :)

@theblankman
Copy link
Collaborator Author

Okay, I think I see. So the root of the problem is that even though I own all cards, I can send a request that looks like I own none, but there are two possible "symptoms."

This code was largely inspired by case 1 happening to me, and should prevent it because the server will now know that I require all-cards, so the opponent who owns base only is not a potential match for me. It should also prevent me from being matched with the opponent of case 2, because the server will think each of us requires cards that neither of us own.

So with this change, both me and the other guy in case 2 just have to wait longer for other opponents to show up who meet our requirement. It might be generally useful to have data that can be parsed/queried for stats like how long it took for a certain type of request to be fulfilled. In this instance, we could learn more about the effects of case 2 by querying whether requests with zero owned sets (not even base) take significantly longer than the average request. Something like a SQL table of request and fulfillment history would probably be useful for diagnostics in general, so maybe it's something to consider in the future.

For the short term, I agree that the easiest thing is to just fix the bug :) I guess the simplest way is if the user has quick-game automatch enabled, gray out quick-game when automatch is still initializing. The more complicated way would be to refactor automatch so requests that come in before it's ready are queued up to send later, but I'm not sure that's worth it.

@aiannacc
Copy link
Owner

The more complicated way would be to refactor automatch so requests that come in before it's ready are queued up to send later, but I'm not sure that's worth it.

It's worth it. It's not even all that hard. It's just that my contributions lately have been lots of talk and very little code.

@theblankman
Copy link
Collaborator Author

my contributions lately have been lots of talk and very little code.

You mean like me for the first couple months after you made me a contributor? :)

@aiannacc
Copy link
Owner

The default user settings result in an automatch request (one with no vpcounter setting). I'm addressing it on the server side because that's easier than finding the cause here.

@aiannacc
Copy link
Owner

I'm still nervous about exceptional cases we haven't thought to test, but it looks good to me!

aiannacc added a commit that referenced this pull request May 30, 2014
@aiannacc aiannacc merged commit e0b0b24 into beta May 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants