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

TMM: Refactor game launch process for TMM + fix game launch message faction parsing #1777

Closed
Askaholic opened this issue Jun 16, 2020 · 4 comments
Assignees

Comments

@Askaholic
Copy link
Collaborator

Askaholic commented Jun 16, 2020

Right now there are some methods for sending the game_matchmaking commands for starting a ladder search, but they are written specifically for ladder1v1. We want to generalize these methods so that they can work for any queue and any featured mod.

For reference, here is a description of some of the relevant commands:

Client sends

{
    "command": "game_matchmaking",
    "state": <"start" | "stop">
    "mod": <queue technical_name>
    "faction": <"uef" | "seraphim" | "cybran" | "aeon" | 1-4>
}

Server sends

{
    "command": "matchmaker_info",
    "queues": [
        {
            "queue_name": <queue technical_name>,
            "queue_pop_time": <datetime in ISO format>,
            "boundary_80s": [[<low>, <high>]],
            "boundary_75s": [[<low>, <high>]],
        }
    ]
}

At some point we will need to decide where the client pulls queue information from (either sent by the lobby server, or pulled from the API) but for now, the start search methods should take necessary information (such as featured mod to update) as function parameters. We can then call the function like:

startSearch("ladder1v1", KnownFeaturedMod.LADDER_1V1.getTechnicalName());

where "ladder1v1" is currently hardcoded, but will become the name of the queue from the matchmaker_info command.

Implementation Details

There are several methods with this name. First there is

public CompletableFuture<GameLaunchMessage> startSearchLadder1v1(Faction faction) {

Which sends the server message to initiate the search. This needs to be modified to take the queue name as an argument.

Then there is

public CompletableFuture<GameLaunchMessage> startSearchLadder1v1(Faction faction) {

which is just a proxy to the previous function.

Finally there is

public CompletableFuture<Void> startSearchLadder1v1(Faction faction) {

Which handles some UI state, updating the game files, and launching forged alliance.

Some similar refactor needs to happen for the corresponding "stop search" functions.

Wanna have the bug fixed quickly?
Visit Issue hunt...
Issue hunt

@Askaholic Askaholic added this to To do in Team Matchmaker Jun 17, 2020
@Askaholic Askaholic moved this from To do to In progress in Team Matchmaker Jun 20, 2020
@MaxNF MaxNF removed their assignment Jun 21, 2020
@Askaholic Askaholic moved this from In progress to To do in Team Matchmaker Jun 22, 2020
@MaxNF
Copy link

MaxNF commented Jun 28, 2020

Look. I can spend some time on it and complete the issue if we confirm what exactly should be done.
There are my current thoughts:

  1. Create a new model class representing a queue.
  2. Create a new fxml layout for a matchmaking page.
  3. Rewrite controller for this fxml layout from scratch. We will fetch all available queues from a server here (when a player enters matchmaking page) and use these queues in the next step.
  4. Rewrite methods responsible for user registration (and unregistration) in a matchmaking search. Make them accept a queue as an argument. There is no need to send more than one argument to these methods because a queue name is unique for each combination of a queue size + featured mode. All we need is to send a queue object to these methods.

P.S. A Queue class already exists. It's called MatchmakingQueue or smth like this. It is just not used much for some reason.

The hardest part here is to create a new layout for a matchmaking page (for me). Also, I think that these steps can be included in one big issue and can be done by one person (except making a layout, maybe) because all of these parts are quite tied up.

Also, the 4th step can be done a little bit differently. We can create new unified methods for launching a search for any queue but do not delete the old ones. When we are ready to switch to dynamic queues and tmm completely, we will just swap controllers and delete all unused methods (in case you merge the changes in the working client before enabling tmm for users).

@Askaholic
Copy link
Collaborator Author

Those things are all way beyond the scope of this issue, and are tracked in separate issues. Also they have all either been done already or at least started. You can see them on the team matchmaking branch: https://github.com/FAForever/downlords-faf-client/tree/feature/team-matchmaking

I talked with @Geosearchef yesterday, and I think refactoring like this is not the way to go. The whole queuing and game file updating process needs to be rewritten so that it will work even if you are simply a member of a party (not the party leader). So it will end up being triggered by a message from the server, rather than a button click in the UI.

One problem I see is: How do we ensure every player in the party has downloaded the necessary files before we join the queue? Because, we wouldn't want to get matched while one of the players is still downloading updates, since this could cause either a game timeout or even a desync.

@Geosearchef
Copy link
Member

all of this has been modfied in the TMM branch already and there are multiple interactions with the underlying FAFServerAccessor that need to be taken into account. (e.g. FAFServerAccessor contains a state (future) that needs to be set on game request, but there is no game request when simply being a member of a party)

Team Matchmaker automation moved this from To do to Done Oct 4, 2020
@Geosearchef Geosearchef removed this from Done in Team Matchmaker Oct 4, 2020
@Geosearchef Geosearchef changed the title TMM: Refactor startSearchLadder1v1 to work for any queue TMM: Refactor game launch process for TMM + fix game launch message faction parsing Oct 4, 2020
@Geosearchef Geosearchef self-assigned this Oct 4, 2020
@Geosearchef Geosearchef reopened this Oct 4, 2020
@Geosearchef Geosearchef added this to To do in Team Matchmaker via automation Oct 4, 2020
@Geosearchef Geosearchef moved this from To do to Assigned in Team Matchmaker Oct 4, 2020
@Geosearchef Geosearchef moved this from Assigned to In progress in Team Matchmaker Oct 4, 2020
@Geosearchef Geosearchef moved this from In progress to Review in Team Matchmaker Oct 4, 2020
@Askaholic
Copy link
Collaborator Author

This has been solved on the TMM branch

Team Matchmaker automation moved this from Review to Done Jan 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

3 participants