smogon / pokemon-showdown-client

The client for Pokémon Showdown
http://pokemonshowdown.com
GNU Affero General Public License v3.0
563 stars 792 forks source link

Replay search improvements #1307

Open scheibo opened 5 years ago

scheibo commented 5 years ago

Currently you can only search by either username or format, but not both at the same time. Also searching with two users to find specific battles between the two.

wuhoodude: Replay search improvements


Started looking into this:

scheibo commented 5 years ago

Strawman code to illustrate the approach:

function search($usera = NULL, $userb = NULL, $format = NULL, $byRating = false, $isPrivate = false, $page = 0) {
    if (!$this->db) return [];
    if ($page > 100) return [];

    $limit1 = intval(50*($page-1));
    if ($limit1 < 0) $limit1 = 0;

    $query = "SELECT uploadtime, id, format, p1, p2, rating FROM ps_replays WHERE private = ?";
    $args = [$isPrivate ? 1 : 0];
    if (isset($format)) {
        $query. = " AND formatid = ?";
        $args[] = $this->toId($format);
    }
    if (isset($usera) && isset($userb)) {
        $query. = " AND ((p1id = ? AND p2id = ?) OR (p1id = ? AND p2id = ?))";
        $a = $this->toId($usera);
        $b = $this->toId($userb);
        array_push($args, $a, $b, $b, $a);
    } else if (isset($usera) || isset($userb)){
        $user = $this->toId(isset($usera) ? $usera : $userb);
        $query. = " AND (p1id = ? OR p2id = ?)";
        array_push($args, $user, $user);
    }
    $query. = " ORDER BY ? DESC LIMIT ?, 51";
    array_push($args, $byRating ? "rating" : "uploadtime", $limit1);

    $res = $this->db->prepare($query);
    $res->execute($args);
    return $res->fetchAll();
}

And just like that, I'm a PHP developer. Now I just need to install PHP...

Zarel commented 5 years ago
  • Is there a publicly available schema for the ps_replays table? The other tables on the server have .sql files describing them, but I can't find a similar one for the table replays/replays.lib.php?

Not currently. How do I make one again? I don't have the command memorized (I hate systems administration).

  • It looks like we're currently deliberately not allowing username and format search together, presumably because we dont have a query that supports it.

It's not deliberate, I just didn't realize it should be supported as a feature at the time.

In theory, it shouldn't be hard to construct a query that supports it.

  • Thoughts on UI? My first thought is to remove the double submit button and add a second user textbox to have:

    User(s): ________ ________
    Format: _______
    
    <Submit>

    But I'm not not sure what the best way to denote only one of the text boxes is required outside of literally adding a message (like smalltext which says "At least one field must be filled") which feels kludgey.

You could just have a single search box, and then advanced "Filter by user" "Filter by format" buttons.

Believe me, I was very angry when the query optimizer stopped working.

But the commented-out query used to work fine. And then, one day, I ran OPTIMIZE TABLE. And then it stopped working. I only managed to get the replay database back up by manually adding FORCE INDEX to all my queries.

You understand now why I have learned helplessness around databases?

  • Is there a specific reason username doesn't support ordering by rating or formats don't support $isPrivate?

No, I just never got around to implementing it.

  • I think there should be one search method, which results in the same logic as today if called with the same arguments (ie. should be equivalent to today's searchFormat if just called with $format)

    function search($usera = NULL, $userb = NULL, $format = NULL, $page = 0, $isPrivate = 
    false, $byRating = false) { 
    // validate at least one of $usera / $userb / $format is defined
    ...
    }

    This is the main important thing to get right, and would probably be where I'd start - I'd write this function and have the existing UI call it, ensure there's not change in functionality/regression and then change the UI to allow fully exercising the new options.

This looks like a pretty awkward API, but in general representing abstract search queries in a function-call API is a hard problem. I don't like how most ORMs handle this either. I have no opinion on your proposed version.

  • Finally, which version of PHP do I need/is there any special setup required?

We use PHP 7; you should be fine just using the latest version.

In theory something like XAMPP works?

scheibo commented 5 years ago

Not currently. How do I make one again? I don't have the command memorized (I hate systems administration).

To dump the schema for the ps_replays table:

mysqldump --no-data -u <USER> -p <DATABASE> ps_replays > <FILENAME>.sql

It should prompt you for the password.

You could just have a single search box, and then advanced "Filter by user" "Filter by format" buttons.

Can you elaborate on this? That UI seems like it would only provide the same UI as today, just with one less search box? Or do you mean

User: _____
<Filter By Additional User> <Filter by format> <Submit>

Where the filter buttons will pop up additional inputs when selected?

I think we'll also want toggles for sorting by rating and public/private as well.

And then, one day, I ran OPTIMIZE TABLE. And then it stopped working. I only managed to get the replay database back up by manually adding FORCE INDEX to all my queries.

It seems like this will need to be addressed before I can proceed. I will try to look into this.

This looks like a pretty awkward API, but in general representing abstract search queries in a function-call API is a hard problem.

As discussed on chat, LINQ would surely be better, but this is probably sufficient for the time being.

We use PHP 7; you should be fine just using the latest version.

I just spun up a new VPS to establish a staging environment on, with the goal being to create a working replica of the PS production stack as closely as possible as a sandbox. I'll seed it with some basic data so it can be used to test functionality (but obviously not load/scaling). I can then back it up and let others boot the image or just give trusted people access to the sandbox if they want to experiment with changes to say, the login server (for OAuth), but dont have access to PS's prod env.

Really this is just a way of avoiding having to install Apache/Nginx/TokuDB on my development computers...

Zarel commented 5 years ago

To dump the schema for the ps_replays table:

Done: c38868806895ecd4651b21165

TokuDB is unnecessary, of course: Unless your test dataset is as big as the live dataset, I assume any database will work fine.

Can you elaborate on this? That UI seems like it would only provide the same UI as today, just with one less search box?

_____ (placeholder: player name or format)
<Filter by player> <Filter by format>
<Search>

clicking "Filter by player":

_____
Player: _____ <x>
<Filter by player 2> <Filter by format>
<Search> <Cancel>

clicking: "Filter by player 2":

_____
Player: _____ <x>
Player 2: _____ <x>
<Filter by player 2> <Filter by format>
<Search>

Typing Alice and Bob in the fields would auto-update the main field:

__player: Alice, player2: Bob__
Player: __Alice__ <x>
Player 2: __Bob__ <x>
<Filter by format>
<Search>

(You could also hide the main field while filter fields were open, if that would be easier.)

I think we'll also want toggles for sorting by rating and public/private as well.

That can just be sort buttons on the results page, like they are now. It seems easier-to-use that way.

It seems like this will need to be addressed before I can proceed. I will try to look into this.

Not necessarily. You could just also use FORCE INDEX queries.

Zarel commented 4 years ago

There's API support for this now! All we need is UI for it.