Major bugs in API

Would appreciate it if the fields in the api responses didn’t change suddenly. ‘expiration_delta’ in the clock part of gamedata changed to ‘expiration’ within the last hour or so and broke my just-released program.

It appears that the API is also very flaky: my game http://online-go.com/api/v1/games/604516 returns:

    "clock": {
        "current_player": 908, 
        "title": "T:2472 R:3 (chiahsun vs apoplexy)", 
        "paused_since": 1404518418, 
        "black_player_id": 908, 
        "last_move": 1404603697583, 
        "white_player_id": 39901, 
        "expiration": 1405208497274, 
        "game_id": 604516, 
        "now": 1404518418310, 
        "black_time": {
            "thinking_time": 604800, 
            "skip_bonus": false
        }, 
        "white_time": {
            "thinking_time": 596476.001, 
            "skip_bonus": false
        }
    }, 

I am black, and according to the API I have 7 days left. However, the game page http://online-go.com/game/604516 shows 6 days. Apparently my time is reported as ‘white_time’.

Even more, this time is reported in seconds, while all other times are reported in milliseconds…

No, the 6d thing on the client is a client issue not an api issue, it comes from our lag compensation stuff.

There’s no change in the field name, expiration delta is used for pause offsets.

Ok. Please tell me how this is supposed to work. There is zero documentation for your api, and the obvious things are apparently wrong: black’s remaining time is not really the field named ‘thinking_time’ in ‘black_time’ and ‘expiration’ - ‘now’ is not the time remaining?

Not only that, the fields change – apparently by design – and now we are blocked from sending more than one or two requests per second. Since you do not provide game details with the resource /me/games/, how are we supposed to get time remaining for the games within a reasonable period?

Apoplexy:

  1. Clock objects give you all of the information you need to display an accurate time measurement on the client side until a new move is made. Please only make one request to update this data per move.
  2. Please do not sustain even 1 query per second to the server, polling is not a scalable solution. If you believe you cannot do something without polling so frequently, please pose a polite question about what you’d like to be able to do and we will figure out how to get you there. (btw bursting is ok, just don’t sustain)
  3. Please ensure your clients do not immediately and continuously retry after an error (such as an unauthenticated client hitting /me, us giving you a 429 or 500, etc…). Exponential scale back is much preferred in these scenarios.
  4. We’re working on documentation. If you find yourself in need, please contact us about it. We can’t always just drop everything we’re doing and get you a thorough answer right away as we have 38 active bugs and 120 items in “todo soon” backlog (in addition to putting out fires like finding out our throttling system was not working correctly ;)), but we do want to fill things out and give API users everything they need to make cool things, so your input, questions, and patience is appreciated.
  5. Please be nicer :slight_smile: matburt and I do this in our spare time for fun. It’s not fun to deal with people who have foul attitudes, and it makes us not really want to work with them in our “fun time” here, so hooray to smiles and making cool things :smiley:

On to clocks.

They are a bit complicated, there’s a fair amount going on, and you have to account for a few different situations depending on your time control system and paused state as you’ve discovered. Instead of proper documentation on the subject which i’d spend half a night writing and get half wrong, and which you’d probably just gloss over anyways like any true coder, I offer you the meat - here is the code we use on the OGS client to display the clocks as well as handle updating it in real time without having to go back to the server and poll it for a new clock.

It’ll obviously take some modifications to drop into your own code base, but hopefully it’ll be either easy enough to modify or clear enough to derive a new solution from.

Goban.prototype.setGameClock = function(clock) { /* {{{ */
    if (!this.white_clock || !this.black_clock) return;

    var white_clock = $(this.white_clock);
    var black_clock = $(this.black_clock);

    if (clock == null) {
        white_clock.html("<span style='color: #333; font-weight: normal;'>--:--</span>");
        black_clock.html("<span style='color: #333; font-weight: normal;'>--:--</span>");
        return;
    }

    if ("pause" in clock) {
        if (clock.pause.paused) {
            this.engine.paused_since = clock.pause.paused_since;
            this.engine.pause_control = clock.pause.pause_control;
        } else {
            delete this.engine.paused_since;
            delete this.engine.pause_control;
        }
    }

    var self = this;
    var now_delta = (new Date()).getTime() - clock.now;
    var nextClockUpdate = 60000;
    var now;
    var use_short_format = this.config.use_short_format_clock;
    
    last_sound_played = null;

    function formatTime(clock_div, time, base_time, player_id) { /* {{{ */
        var ms;
        var time_suffix = "";
        var periods_left = 0;

        if (typeof(time) == "object") {
            ms = (base_time + (time.thinking_time)*1000) - now;
            if ("moves_left" in time) { /* canadian */
                if ("block_time" in time) {
                    if (time.moves_left) {
                        time_suffix = "<span class='time_suffix'> + " + shortDurationString(time.block_time) + "/" + time.moves_left + "</span>";
                    }
                }
                if (time.thinking_time > 0) {
                    periods_left = 1;
                }
                if (ms < 0 || (time.thinking_time === 0 && "block_time" in time)) {
                    ms = (base_time + (time.thinking_time + time.block_time)*1000) - now;
                    if (time.moves_left) {
                        time_suffix = "<span class='time_suffix'>/ " + time.moves_left +"</span>";
                    }
                }
            }
            if ("periods" in time) { /* byo yomi */
                var period_offset = 0;
                if (ms < 0 || time.thinking_time === 0) {
                    period_offset = Math.floor((-ms / 1000) / time.period_time);
                    if (period_offset < 0) { period_offset = 0; }

                    while (ms < 0) {
                        ms += time.period_time * 1000;
                    }

                    if (player_id != clock.current_player) {
                        ms = time.period_time * 1000;
                    }
                    periods_left = ((time.periods - period_offset) - 1);
                    if (((time.periods - period_offset) - 1) > 0) {
                        time_suffix = "<span class='time_suffix'>+" + periods_left + "/" + (shortDurationString(time.period_time)).trim() + "</span>";
                    }
                    if (((time.periods - period_offset) - 1) < 0) {
                        ms = 0;
                    }
                } else {
                    periods_left = time.periods;
                    time_suffix = "<span class='time_suffix'>+" + (time.periods) + "x" + (shortDurationString(time.period_time)).trim()+ "</span>";
                }

            }
        } else {
            /* time is just a raw number */
            ms = time - now;
        }


        var seconds = Math.ceil((ms-1) / 1000);
        //var weeks = Math.floor(seconds / (86400*7)); seconds -= weeks * 86400*7;
        var days = Math.floor(seconds / 86400); seconds -= days * 86400;
        var hours = Math.floor(seconds / 3600); seconds -= hours * 3600;
        var minutes = Math.floor(seconds / 60); seconds -= minutes * 60;

        
        function plurality(num, single, plural) { 
            if (num > 0) {
                if (num == 1) return num + " " + single;
                return num + " " + plural;
            }
            return "";
        }

        //if (weeks) {
            //return plurality(weeks, "Week", "Weeks") + " " + (days ? plurality(days, "Day", "Days") : "");
        //    return plurality(weeks, "Week", "Weeks");
        //}

        var html = "";
        var cls = "plenty_of_time";
        if (ms <= 0 || isNaN(ms)) {
            nextClockUpdate = 0;
            cls='out_of_time';
            html = "0.0";
        } else if (days > 1) {
            //return plurality(days, "Day", "Days") + " " + (hours ? plurality(hours, "Hour", "Hours") : "");
            nextClockUpdate = 60000;
            html = plurality(days+1, _("Day"), _("Days"));
        } else if (hours || days == 1) {
            nextClockUpdate = 60000;
            if (days == 1) hours += 24;
            html = days === 0 ? interpolate(pgettext("Game clock: Hours and minutes", "%sh %sm"), [hours, minutes]) : interpolate(pgettext("Game clock: hours", "%sh"), [hours]);
        //} else if (minutes) {
        } else {
            nextClockUpdate = Math.max(50, (ms % 1000)+1); /* once per second, right after the clock rolls over */
            html = minutes + ":" + (seconds < 10 ? "0" : "") + seconds;
            if (minutes === 0 && seconds <= 10) {
                if (seconds % 2 === 0) {
                    cls += " low_time";
                }

                var sound_to_play = null;

                if (self.on_game_screen && player_id) {
                    if (periods_left === 0) {
                        sound_to_play = seconds;
                    } else {
                        if (seconds <= 3) {
                            sound_to_play = "beep" + seconds;
                        }
                    }

                    if (sound_to_play && global_user.id == self.engine.playerToMove() && player_id == global_user.id) {
                        if (last_sound_played != sound_to_play) {
                            last_sound_played = sound_to_play;
                            window['sfx'].play(sound_to_play);
                        }
                    }
                }
            }

        //} else {
        //    //nextClockUpdate = (100-(ms%100))+1; /* 10x per second */
        //    nextClockUpdate = (ms % 1000)+1; /* once per second, right after the clock rolls over */
        //    //html = (ms/1000).toFixed(1);
        //    html = Math.ceil(ms/1000);
        }

        if (clock.start_mode) {
            cls += " start_clock";
        }

        clock_div.html("<span class='" + cls + "'>" + html + "<span>" + (use_short_format ? "" : time_suffix));
    } /* }}} */


    function updateTime() { /* {{{ */
        now = (new Date()).getTime();

        if (self.engine.phase != "play") {
            white_clock.empty();
            black_clock.empty();
            return;
        }

        if (self.__clock_timer) {
            clearTimeout(self.__clock_timer);
            self.__clock_timer = null;
        }

        var white_base_time = clock.current_player == clock.white_player_id ? (clock.last_move + now_delta) : (now);
        var black_base_time = clock.current_player == clock.black_player_id ? (clock.last_move + now_delta) : (now);

        black_clock.empty();
        white_clock.empty();


        if (clock.start_mode) {
            formatTime(clock.black_player_id == clock.current_player ? black_clock : white_clock, clock.expiration+now_delta, clock.last_move);
        } else if (self.engine.paused_since) {
            var w = _("Paused");
            var b = _("Paused");
            if (self.engine.pause_control) {
                var pause_control = self.engine.pause_control;
                if ("weekend" in pause_control) {
                    b = _("Weekend");
                    w = _("Weekend");
                }
                if ("system" in pause_control) {
                    b = _("Paused by Server");
                    w = _("Paused by Server");
                }
                if (("vacation-" + clock.black_player_id) in pause_control) {
                    b = _("Vacation");
                }
                if (("vacation-" + clock.white_player_id) in pause_control) {
                    w = _("Vacation");
                }
            }
            white_clock.empty().append($("<span>").addClass('paused').text(w));
            black_clock.empty().append($("<span>").addClass('paused').text(b));

            nextClockUpdate = 60000;
        } else {
            if (clock.white_time) {
                formatTime(white_clock, clock.white_time, white_base_time, clock.white_player_id);
            }
            var whiteNextClock = nextClockUpdate;
            if (clock.black_time) {
                formatTime(black_clock, clock.black_time, black_base_time, clock.black_player_id);
            }
            nextClockUpdate = Math.min(nextClockUpdate, whiteNextClock);
        }

        if (nextClockUpdate) {
            self.__clock_timer = setTimeout(updateTime, nextClockUpdate);
        }
    } /* }}} */

    try {
        updateTime();
    } catch(e) {
        console.log(e);
    }
}; /* }}} */



function shortDurationString(seconds) { /* {{{ */
    var weeks = Math.floor(seconds / (86400*7)); seconds -= weeks * 86400*7;
    var days = Math.floor(seconds / 86400); seconds -= days * 86400;
    var hours = Math.floor(seconds / 3600); seconds -= hours * 3600;
    var minutes = Math.floor(seconds / 60); seconds -= minutes * 60;
    return "" +
        (weeks ? " " + interpolate(pgettext("Short time (weeks)", "%swk"), [weeks]) : "")+ 
        (days ? " " + interpolate(pgettext("Short time (days)", "%sd"), [days]) : "") + 
        (hours ? " " + interpolate(pgettext("Short time (hours)", "%sh"), [hours]) : "") + 
        (minutes ? " " + interpolate(pgettext("Short time (minutes)", "%sm"), [minutes]) : "") + 
        (seconds ? " " + interpolate(pgettext("Short time (seconds)", "%ss"), [seconds]) : "");
} /* }}} */

Cheers,

  • anoek
5 Likes

Anoek, thanks for the reply.

I think there are two reasons I criticized you guys like this. One is that I still feel attached to ogs as a developer - this I’ll just have to get over. The other is that I think you are doing a great job with the site, and so I have high expectations and the letdown can be steep. I was a little dismayed that things are put online in what appears to be a beta state (it is a little surprising that the set of data members of objects returned in api calls can change).

For the immediate case, I spent about 6 hours this weekend exploring the api and recoding my plugin. The fact that the code broke hours after I uploaded it made me feel like an idiot. I then had to roll back my plugin from my phone on Monday morning because many Google sites are not reachable through our firewall at work.

Again, I think you guys do a great job for many things. I will wait until the api is documented before I attempt to update the plugin, and I won’t let my frustrations creep into bug reports.

1 Like

As an aside; the reason I need to make multiple calls in the first second is that the game time is not available on /me/games/; each game has to be individually queried.

I think we can do a couple of things for you there. We’ll be changing our api throttling a bit to allow for better bursting behavior. Right now we are default to 1 request per second but we’ll be changing that to 60 requests per minute which should allow you to pull a good bit of information in bursts at one time.

The reason we don’t return time and move information in game lists (we don’t do it at /v1/games either) is that we have to fetch the information from the game state database (it’s not stored in our site relational database) so to do that for a full page of games at one time can add a lot of extra time to the api request which we would like to return as fast as possible. With the new bursting behavior you’ll be able to pull the games list and then go query those games without worrying too much about throttling unless your list is big.

We certainly understand what it’s like to have broken code out in production (we’re both professional software engineers and we’ve had it happen to us) no need to feel bad about it.

With a site this large there’s just no way for us to have perfect test coverage and when making huge changes like our 4.0 release bugs will inevitably creep in. I would point out that we put out a call for testers on our public beta site months before we released:

Having said that, we have (and will continue to) worked hard to fix those bugs. As mentioned in that post, this release provides us with a great foundation to continue building the site. We do rely heavily on our community to help us find problems and make suggestions on how things could run better… it’s just the nature of having a large project at this scale with a small team.

3 Likes

Re the availability of the clock information, we might think about packing that into the "yourMove’ notifications, then it’d be available in the notifications list for polling or just get pushed to the clients if they’re hooked up via the websocket. Either way that’d be a lot lighter weight than pulling up the full game if it’s only the clock data you’re after.

Is there any other data you’re wanting from the game state, or just the clock?

1 Like

All I need is

  1. Game id
  2. Whose turn it is
  3. Opponent name
  4. Query time
  5. Time remaining for each player
  6. Pause state

If I can’t get this at a simple api point without separate credentials and without anything fancier than JSON parsing, I probably won’t do it. In particular, the code furnished for computing the time remaining is a little crazy; you are exposing way too many implementation details through the api. It is enough to give three variables; current time, end time (or time remaining) assuming the game is currently live and no pause states will intervene, and pause state. If you really want to get fancy, compute and expose the time the state should change (weekend begins, etc) so that the client can resend the query then.