HabitRPG / habitica

A habit tracker app which treats your goals like a Role Playing Game.
https://habitica.com
Other
11.8k stars 4.05k forks source link

Batch / Queued API operations, Operational Transformation -style #879

Closed lefnire closed 8 years ago

lefnire commented 11 years ago

We'll need a way for API consumers not to clobber each other. We'll also need a way to queue transformations, in case the API consumer needs offline access. Here are the highlights that @thedecimal and I have come up with. @thedecimal, if there's anything I'm missing do chime in.

PUT /user an array of operations to perform. Example:

[ 
  {op: 'incr', path: 'user.stats.hp', data: 30},
  {op: 'set', path: 'user.tasks.1234', data: {id:1234, type:'habit', value...},
  {op: 'delete', path: 'user.tasks.5678', data: null},
  {op: 'push', path: 'user.history.todos', data: {...}}
]

So the "operational transformation" part of this means client A can send up some stuff, client B can send stuff, and they'll all get performed without overwriting eachother (note the "incr" of the first example op, instead of using "set").

Other benefits: save HTTP requests if we can queue them as a batch. That also means you can store data in localStorage, or whatever other offline mechanism you use, to send up at once. We won't have to change much: pull out all the current API methods into individual functions - the old routes call those functions, the new PUT /user route calls those functions for each queued item, etc.

Anyway, thoughts? I realize @thedecimal (Yaser) and I have been talking about all this stuff offline instead of publicly, so we should get more heads in. @switz @pjf @arscan @yangit @wizonesolutions

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

switz commented 11 years ago

I'm worried about just letting people write arbitrary paths willy nilly. (I've never actually written that in a sentence before)

On Wednesday, May 1, 2013 at 3:23 PM, Tyler Renelle wrote:

We'll need a way for API consumers not to clobber each other. We'll also need a way to queue transformations, in case the API consumer needs offline access. Here are the highlights that @thedecimal (https://github.com/thedecimal) and I have come up with. @thedecimal (https://github.com/thedecimal), if there's anything I'm missing do chime in.
PUT /user an array of operations to perform. Example: [ {op: 'incr', path: 'user.stats.hp', data: 30}, {op: 'set', path: 'user.tasks.1234', data: {id:1234, type:'habit', value...}, {op: 'delete', path: 'user.tasks.5678', data: null}, {op: 'push', path: 'user.history.todos', data: {...}} ]

So the "operational transformation" part of this means client A can send up some stuff, client B can send stuff, and they'll all get performed without overwriting eachother (note the "incr" of the first example op, instead of using "set"). Other benefits: save HTTP requests if we can queue them as a batch. That also means you can store data in localStorage, or whatever other offline mechanism you use, to send up at once. We won't have to change much: pull out all the current API methods into individual functions - the old routes call those functions, the new PUT /user route calls those functions for each queued item, etc.
Anyway, thoughts? I realize @thedecimal (https://github.com/thedecimal) (Yaser) and I have been talking about all this stuff offline instead of publicly, so we should get more heads in. @switz (https://github.com/switz) @pjf (https://github.com/pjf) @arscan (https://github.com/arscan) @yangit (https://github.com/yangit)

— Reply to this email directly or view it on GitHub (https://github.com/lefnire/habitrpg/issues/879).

switz commented 11 years ago

By the way, Derby has OT built in, so you might not even have to send transformations as long as we use the OT functions.

On Wednesday, May 1, 2013 at 3:23 PM, Tyler Renelle wrote:

We'll need a way for API consumers not to clobber each other. We'll also need a way to queue transformations, in case the API consumer needs offline access. Here are the highlights that @thedecimal (https://github.com/thedecimal) and I have come up with. @thedecimal (https://github.com/thedecimal), if there's anything I'm missing do chime in.
PUT /user an array of operations to perform. Example: [ {op: 'incr', path: 'user.stats.hp', data: 30}, {op: 'set', path: 'user.tasks.1234', data: {id:1234, type:'habit', value...}, {op: 'delete', path: 'user.tasks.5678', data: null}, {op: 'push', path: 'user.history.todos', data: {...}} ]

So the "operational transformation" part of this means client A can send up some stuff, client B can send stuff, and they'll all get performed without overwriting eachother (note the "incr" of the first example op, instead of using "set"). Other benefits: save HTTP requests if we can queue them as a batch. That also means you can store data in localStorage, or whatever other offline mechanism you use, to send up at once. We won't have to change much: pull out all the current API methods into individual functions - the old routes call those functions, the new PUT /user route calls those functions for each queued item, etc.
Anyway, thoughts? I realize @thedecimal (https://github.com/thedecimal) (Yaser) and I have been talking about all this stuff offline instead of publicly, so we should get more heads in. @switz (https://github.com/switz) @pjf (https://github.com/pjf) @arscan (https://github.com/arscan) @yangit (https://github.com/yangit)

— Reply to this email directly or view it on GitHub (https://github.com/lefnire/habitrpg/issues/879).

arscan commented 11 years ago

Thanks for the heads up that you are working on this -- super interesting stuff. habitrpg-txt (btw renamed it because i swapped out the ncurses implementation for something that actually works) was written to queue up calls in a disconnected state originally. I ended up scrapping it just because that app is intended to be used in an (almost) always connected environment.

My thoughts, for what they are worth:

A scheme like this certainly could work, but its not a restful design. I'm not sure if saving a few http calls is worth going with a non restful design... at least with those set, delete & push operations (those are just put, delete and post http methods). And the increment operation could be reframed in a restful manner. And a nitpick in comparison, that should be a POST because multiple repeat calls would result in a changed state.

i guess I'd be ok with this if it isn't intended to be a public API. In looking closer at what you've got there, the increment exp operator assumes the client has implemented the game / scoring logic properly, which is fine for the "official" client developed by the core team that understands the game logic, but not so fine for a 3rd party app. And if its not a documented public API then following common patterns like rest isn't so important.

lefnire commented 11 years ago

@switz - We can lock down paths. No need to make our custom OT very clever or flexible, we can make it simple and habit-specific.

Now, yes racer does OT - but this is irrelevant, actually. Racer does OT smartly when two Racer clients (eg, Derby) are sending competing ops. This is useful for example if we built Google Docs with Derby. Well, our only direct Racer client is the website. The other clients (mobile app, desktop, habit-txt, etc) will be using the API instead of tying directly to Racer - so we can't benefit from Racer's OT. Yes we'll be using Racer in the API calls (so essentially using Racer's OT anyway), but what I'm suggesting here is to modify the API so that individual actions can be performed on paths, rather than setting a property whole-sale, which allows for multiple clients' modifications to go through. So instead of PUT /user/tasks/:id with a full update object just to change one property, and which may clobber a similar call in the pipe - we instead send PUT /user: {op: 'set', path: 'user.tasks.123.text', data: 'New Title'}. I'm having trouble explaining, does this make sense?

@arscan - good point about irregular REST design. Since we'll definitely want to preserve the current /api/v1 anyway, we could have this new setup as an unofficial API route, as well as continuing to expand /api/v1 as we build new functions with possible respective routes. I do really think we need something like it, in order to perform micro operations one after another for the sake of data integrity when multiple clients are consuming the API (which is no longer an edge case). My original thought was along the lines of this: in all PUT routes (in the current API), if they don't send a complete object, simply edit the properties sent, rather than replacing the object whole-sale in the database. If they send the obj.del property, delete the object. Etc, etc - but this got kinda complicated the more I tried to work with it, and still had the problem of clobbering a single-property edit made my multiple clients. For example, if the chrome extension scored me up, and I scored the same task down on the mobile app - the mobile app would simply replace the edits made by the chrome extension.

Anyway, I'm rambling - what I'm saying is we need something like this to be sure. If we don't like the current design idea, any thoughts?

As far as this requiring algorithms on the client: originally, I was actually intending this - which is why I started to create the habitrpg-shared repository (which I haven't been keeping up with. We have the "extension" API for up/down scoring, which performs calculations on the server - but for full API consumers, there's a level to which they'd need to replicate website functionality in their own client anyway. In fact, how are you currently doing that in habit-txt? @thedecimal suggested sending all algorithm-based ops up as using the extension-api style, which would essentially "repeat" the algos on the server (still requiring the client to take care of itself). So add {op: 'score', path:'user.tasks.1234', data: 'up'} to the mix, for example. Again, still requires you implement the algos on the client, and we'd need to some how make things like random drops deterministic rather than random, so that if they find an egg on client that op would replicate it on the server.

Ok, I'm going to stop rambling

wizonesolutions commented 11 years ago

@lefnire Check out how the Remember the Milk API is built. It's structured well.

https://www.rememberthemilk.com/services/api/

It's all GET-based, admittedly, but last_sync and stuff like completeTask (vs. arbitrarily updating the task) is golden.

It would mean a departure from the free-for-all, though, although it'd still be nice if there was a way to add custom properties. But probably with a specific route for that rather than just PUTing.

lefnire commented 11 years ago

@wizonesolutions the RTM -style / last_sync method was the original plan, I'm proposing a shift. Problems with the last_sync method are:

This proposed OT solution (1) is very simple to implement (we can just map most of the ops to Racer ops 1-1, with some validation) while achieving full operation merging functionality, (2) removes the need for any custom properties (user.last_sync,user.tasks.*.last_sync, user.timezone, etc), (3) gets around timezone issues.

I'm talking myself into full conviction, better stop me now before I write it! :runner: :dash:

abogalambo commented 11 years ago

and the API responds with the updated user and stats in JSON format so that we don't rely on the client to implement the scoring algorithms correctly or to even implement it at all. Only if the client needs to work offline will it be required to implement the scoring algorithms locally.

arscan commented 11 years ago

The fundamental question to me is how you want to think about gameplay in a disconnected mode. If you are ok thinking of this in a client-server perspective, where the server does all the gameplay logic, then what is in the v1 API is basically good enough (technically things get a little screwy on marking things complete or on renaming, but I don't think its that big a deal for those). Yeah, doing a wholesale sync of the entire profile causes problems, which is why keeping to the "incremental" REST calls is safer (that's what habittxt does). BTW, I think what you were referring to (updating only part of models) is the patch method.

Doing it in client-server is by far the easiest. The clients simply assume that their score, etc, is "stale" after they make local changes in a disconnected state (habittxt puts up a "unsaved" flag to tell the user of that), and they have to reconnect to see their scores change. As long as they replay all the REST calls that they were planning on calling, and they stick to the simple methods (move up a habit, delete, mark as complete... as opposed to trying to sync the entire model) then it should be mostly fine.

If you want to think of this in more of a peer mode, where each client is capable of playing the full game, then things get a lot more complex. Having a web API simply isn't enough -- what is really needed is a client-side library maintained by the core team (and not app developers) so they can implement the game logic consistently on every target language/platform. At which point, talking about the API between the peers isn't what we should be focusing on right now, but rather the API of that client-side library.

So I think a conscious decision needs to be made as to which direction you want to go (simple client-server or peer), understanding the ramifications of going the peer mode, because it makes things a lot harder.

arscan commented 11 years ago

Also, just to be clear, I understand that a certain level of logic needs to be represented on the client side to do things like show newly added tasks, deleted tasks, etc when you are disconnected (otherwise the experience would suck). And I did that in habittxt in a couple of lines of code. But I think there's a big difference between showing that kind of stuff locally when disconnected before hitting the server (which is pretty simple and doesn't really contain any habit-specific biz logic) and trying to implement the game logic stuff to update your score, stats, etc.

Sorry, I'm sure I'm missing a lot of design decisions that were made along the way, so if we are passed this point of discussion and have decided that a peer model is the way to go (which is what derby does technically) then that's fine. Its just that it seems like you can get a pretty good mobile experience in the short term without worrying about all of this stuff if you keep the game logic on the server side. And then maybe on the second pass at the mobile app look into figuring out how to do things like making the score change on disconnected devices.

Anyhow, those are my thoughts. I might be way off though ;-)

switz commented 11 years ago

We should have a serious Google hangout and talk about this.

I for one, think all the game logic should be moved into the API. Plus, I'm not the biggest fan of creating our own pseudo-REST OT API. Sending 5-10 AJAX requests at a time is not going to kill the client or the server on reconnect.

On Thursday, May 2, 2013 at 11:09 AM, Rob Scanlon wrote:

Also, just to be clear, I understand that a certain level of logic needs to be represented on the client side to do things like show newly added tasks, deleted tasks, etc when you are disconnected (otherwise the experience would suck). And I did that in habittxt in a couple of lines of code. But I think there's a big difference between showing that kind of stuff locally when disconnected before hitting the server (which is pretty simple and doesn't really contain any habit-specific biz logic) and trying to implement the game logic stuff to update your score, stats, etc. Sorry, I'm sure I'm missing a lot of design decisions that were made along the way, so if we are passed this point of discussion and have decided that a peer model is the way to go (which is what derby does technically) then that's fine. Its just that it seems like you can get a pretty good mobile experience in the short term without worrying about all of this stuff if you keep the game logic on the server side. And then maybe on the second pass at the mobile app look into figuring out how to do things like making the score change on disconnected devices. Anyhow, those are my thoughts. I might be way off though ;-)

— Reply to this email directly or view it on GitHub (https://github.com/lefnire/habitrpg/issues/879#issuecomment-17343586).

wizonesolutions commented 11 years ago

Of the same opinion. And what if someone's client gets out of sync and tries to send operations that don't make sense? How do we resolve the situation in a non-wtf way?

My thoughts are like @arscan's. Implement simple ops offline that don't change user stats, at least for v1. Tell them to get net for the rest for now.

V1.5 would be up/down scori queueing, but if they forget to sync and interact with the web also, things are gonna get weird... On May 2, 2013 5:59 PM, "Daniel Saewitz" notifications@github.com wrote:

We should have a serious Google hangout and talk about this.

I for one, think all the game logic should be moved into the API. Plus, I'm not the biggest fan of creating our own pseudo-REST OT API. Sending 5-10 AJAX requests at a time is not going to kill the client or the server on reconnect.

On Thursday, May 2, 2013 at 11:09 AM, Rob Scanlon wrote:

Also, just to be clear, I understand that a certain level of logic needs to be represented on the client side to do things like show newly added tasks, deleted tasks, etc when you are disconnected (otherwise the experience would suck). And I did that in habittxt in a couple of lines of code. But I think there's a big difference between showing that kind of stuff locally when disconnected before hitting the server (which is pretty simple and doesn't really contain any habit-specific biz logic) and trying to implement the game logic stuff to update your score, stats, etc. Sorry, I'm sure I'm missing a lot of design decisions that were made along the way, so if we are passed this point of discussion and have decided that a peer model is the way to go (which is what derby does technically) then that's fine. Its just that it seems like you can get a pretty good mobile experience in the short term without worrying about all of this stuff if you keep the game logic on the server side. And then maybe on the second pass at the mobile app look into figuring out how to do things like making the score change on disconnected devices. Anyhow, those are my thoughts. I might be way off though ;-)

— Reply to this email directly or view it on GitHub ( https://github.com/lefnire/habitrpg/issues/879#issuecomment-17343586).

— Reply to this email directly or view it on GitHubhttps://github.com/lefnire/habitrpg/issues/879#issuecomment-17346938 .

abogalambo commented 11 years ago

After going through the thread, I came up with this list of requirements / preferences for the API

Here are my suggestions for the API to meet all these requirements:

This satisfies the needs for the first 2 types of clients For clients that implement the game logic (scoring algorithms and leveling up .. etc), the logic run on the client should not be a replacement for running it on the server. But it's only needed to enable the client to run offline. and I believe this is important to implement. If I am using HabitRPG to track my daily tasks, I should be able to add / remove tasks / mark them as done even when I don't have an internet connection available.

So we can implement it as follows : (this is almost finished)

This will need one more path in the API that takes a batch of operations and applies them in the same sequence they were done on the client. This can be : POST /updates and takes as parameters the list of actions.

Implementing this as a series of requests one for each operation will be hard because we will have to wait until the server responds for the first API call before sending the next one (since we have to ensure that actions are applied on the server in the same order they were done on the client. e.g if the user creates a task then deletes it)

What do you think ? It will be very useful to discuss this in a hangout

lefnire commented 11 years ago

I'll be in internet land starting tomorrow evening. I'm down for a G+ hangout Sunday, if y'all are. GMT+0 here

On Friday, May 3, 2013, thedecimal wrote:

After going through the thread, I came up with this list of requirements / preferences for the API

-

To be restful and have simple clear operations so that it's suitable to expose to 3rd party extensions

To accommodate for different types of clients. Mainly 3 types :

  1. Simple clients who don't implement the game logic, but just need have a user uid and token and need to create tasks or score them up or down.

    1. Simple Clients who don't implement the game logic, but are aware of the game objects and stats (so they act as a display for the player status). These clients can't work offline since they need the server to apply the game logic for them.
    2. Clients who implement the game logic so they can work offline and sync with the server on reconnect. For example : the official mobile app.

    For the first and second types, we need to restrict what the client can do so that it doesn't mess the user's data

Here are my suggestions for the API to meet all these requirements:

  • The API is restful
  • All methods in the API correspond to the same operations that the user can do through the web interface, For example, the API can't directly increase / decrease health points or set the user score for a certain task. So the operations permitted through the API are :
    • POST /tasks Create task
    • PUT /tasks/:task_id + or - a habit (only specifies direction up / down not the amount of increment)
    • PUT /tasks/:task_id mark Todo or daily task as done / undone
    • PUT /tasks/clear_completed clear completed Todos
    • PUT /tasks/:task_id Update task
    • DELETE /tasks/:task_id Delete task
    • PUT Buy Reward / item In addition to retrieving tasks and user data that are already implemented using GET /tasks and GET /user

This satisfies the needs for the first 2 types of clients For clients that implement the game logic (scoring algorithms and leveling up .. etc), the logic run on the client should not be a replacement for running it on the server. But it's only needed to enable the client to run offline. and I believe this is important to implement. If I am using HabitRPG to track my daily tasks, I should be able to add / remove tasks / mark them as done even when I don't have an internet connection available.

So we can implement it as follows : (this is almost finished)

  • The client has a list of pending actions (actions performed on the client but not yet synced to the server).
  • When the user performs any action that will result in changing the score or the game state, the action is logged into the list of actions.
  • If a connection is available, the list of actions are sent to the server to apply them. These changes will also correspond exactly to operations done by the user.
  • The server applies the actions and responds with the new user and game state as a JSON object.
  • When the client receives the game state, it overrides its current state and user data and clears the actions log indicating that it's now synced with the server.

This will need one more path in the API that takes a batch of operations and applies them in the same sequence they were done on the client. This can be : POST /updates and takes as parameters the list of actions.

Implementing this as a series of requests one for each operation will be hard because we will have to wait until the server responds for the first API call before sending the next one (since we have to ensure that actions are applied on the server in the same order they were done on the client. e.g if the user creates a task then deletes it)

What do you think ? It will be very useful to discuss this in a hangout

— Reply to this email directly or view it on GitHubhttps://github.com/lefnire/habitrpg/issues/879#issuecomment-17392121 .

yangit commented 11 years ago

Guys, what about task sorting? Now we have an array of tasks and sort them using some fancy jQuery\bootstrap sortables. Are we going to have /task/move(fromIndex,toIndex) method?

yangit commented 11 years ago

@thedecimal I see a problem with this approach. See it in bold

I can see two approaches.

  1. We implement OT API both ways. When client speaks to server and when server speaks to client. I.e. server do not just submit one huge JSON but rather sends "OK" upon approved changes and sends changes made on other clients using same API. That allows us to create real async app. That one will work on multiple clients simultaneously and would be really cool. And we will all die debugging this event spagetti.
  2. Upon receiving changes client will check if there are any more pending OT's in the queue and if there are, than discard the server's answer and keep waiting. As it has probably already submitted pending OT's and latest answer is on the way.

I'm still a bit worried that some clicks might be lost if user tried to operate the model while it is being updated. Like what we have now in Derby branch. And this update could take a while up to 20 seconds in some circumstances (slow computer, hundreds of tasks, tons of events on each task). We have to think well about this very tiny moment of taking user data on the client, discarding it, and replacing it with JSON from the server.

yangit commented 11 years ago

I just come up with another two nasty scenarios:

We have 1 user, 2 clients(mobile and computer) and a server. User clicks "+ habit" on one client. Than goes to client2. observes there is still no change and "+ habit" again. After all the sync is over, we got "+ habit" event registered twice.

Another scenario: User clicks "+ habit" on client1, gets his gold, buys armor. And feels protected. Before going to bed he takes client2, decides he no longer want this habit and clicks "habit delete". Now server is going to get "habit delete" first. Overnight user get's killed as his HP was low and he did not sync any armor, and there were some unchecked todos. And server is going to WTF tomorrow morning when user is trying to sync "+ habit" on already deleted habit. User is going to WTF even more.

Solution to that might be to keep timestamp on each API call. And store the API calls. And upon receipt any call with timestamp from the past replay all the API calls onto the model and push updated result to everyone. Or even better sync API calls themselves, so like in github every client would have all history of all the changes and it would be easy to roll back to any point.

I should go to sleep now. We are not going to invent github here. We need simple solution :) But maybe...

abogalambo commented 11 years ago

Checking for pending updates (The 2nd approach) is a good idea. But I believe even if we don't do that, we will not lose the user's operations.

consider this scenario :

But still we need to avoid updating the game local state twice as it may confuse the user when he finds his updates undone for a moment then done again when the second response is received. So checking if there are pending actions is also useful

Of course there is no guarantee that the two requests may reach the server in a different order or the server response may reach the client in different order, but it's unlikely. The goal is to minimize the chance of losing updates to make it very unlikely and to make it impossible to reach an inconsistent state where the client and user data are different.

arscan commented 11 years ago

@thedecimal -- I implemented that in habitrpg-txt because it was really annoying seeing the update being undone for a moment after the first refresh came through but before the second one came through. Turns out doing multiple actions in quick succession is a very common scenario in habitrpg (at least for me).

https://github.com/arscan/habitrpg-txt/blob/master/lib/habitapi.js#L58

I can think of more robust solutions, but that seemed to fix the problem 80% of the time and was dead simple to implement. And I poll the server occasionally anyhow, so if there are any inconsistencies anywhere it will get picked up eventually. Also, I let the user hit "r" to manually force a refresh if she thinks that something is fishy.

@yangit -- for that second issue you brought up (trying to do something when disconnected and then going to another client and doing something contradictory), in habitrpg-txt all I did was put up a "UNSAVED" message in the status bar when an action wasn't saved. I think that the user is smart enough to understand the ramifications of that and to make sure to reconnect with the server if that action needs to be registered before bed time.

https://github.com/arscan/habitrpg-txt/blob/master/lib/charmview.js#L273

This just goes back to what mental model our users should have of the game, and that will set their expectations of what should happen. If we set the expectation that its a client-server game, where you need to reconnect for "game stuff" to happen, then simply telling the user that changes are unsaved is good enough.

yangit commented 11 years ago

I'm against this API. It seems to be way to complicated and difficult to maintain.

Frist of all let's decide what problem we are trying to solve. As I understood it we are talking about 3 way merge issue when 2 clients of same user, update same model on a server and some changes become lost.

Introducing granular changes and providing API for them is not complete solution as two clients can still invoke different methods on the same property. And even moving from "set" to "increase" does not do much as we can still have "incr" invoked twice from out of sync clients and just like invoking "set" twice this leads to undesirable result.

What is even worse. Is that for every action. For every event doing model update we are going to have template, model, and now API . And this API has to be smart enough. Which means writing code twice. Very, very bad idea. Are we going to validate API input? Differentiate between habits and todos? What if user tries to set "repeat" property of reward? Making API smart would mean writing validation code twice. And if we are ok with dumb API I have a better solution below.

And we are not going to get away with just POST /tasks/taskId/title

There is going to be a lot more. POST /tasks/taskId/title POST /tasks/taskId/priority POST /tasks/taskId/repeat/m POST /tasks/taskId/repeat/t POST /tasks/taskId/repeat/w POST /tasks/taskId/repeat/th POST /tasks/taskId/repeat/f POST /tasks/taskId/repeat/sa POST /tasks/taskId/repeat/s POST /tasks/taskId/completed POST /tasks/taskId/value POST /tasks/taskId/notes POST /tasks/taskId/text POST /tasks/reorder(index,newIndex)

That is just a part of it. And we have to update it every time there is a change in model.

3-way model merge and DIFF

If we are only trying to address this 3-way merge issue I suggest using following approach. Each client receives a model and stores a copy of it. Than, before submitting changes to server client makes DIFF of the model to be submitted and the model last synced from server. And only submits DIFF to the server. Server, having received two DIFFs from two clients will not have problems if they do not update same properties. And if they do we are in trouble anyway, not even "incr" going to save us.

And that is it. Server can clearly say which properties are to be updated and which are not to.

We have to submit timstamp though. If Client1 gets model, applies changes and stays offline for a month. A lot of changes are made from Client2 and suddenly user comes back from vacation and switches on his Client1. And viola! We are in trouble. 1 month's old changes make their way into the model overriding newer ones.

Another issue I see is Client1 clicks on + task.1 and his gold goes UP. Client2 clicks on + task.2 and his gold goes UP. They both try to sync their task.1 and task.2 statuses, which is fine. And they are trying to sync gold amount. Which will 100% go out of order.

I'm stuck. Having API to set properties is not going to help us. We need API for user actions. Having API call for each possible user action seems sooo overcomplicated and WET (write everything twice). Let's have a G+ hangout?

abogalambo commented 11 years ago

The point of having an increment operation instead of set was mainly to go with the idea of not relying on clients to implement game logic. Clients only + / - a task.

Let me point out the problems with sending a user object (even with partial updates) instead of sending an action (or a list of actions / transforms). The user object contains the user stats as well as tasks list. an action like : scoring up a TODO results in 2 changes :

  1. the todo object being marked done
  2. the user stats updated

1st problem : If the client doesn't implement the game logic, how can it know the value of the user score after marking the Todo as done ? 2nd problem : Imagine this very likely scenario : we have client 1, client 2 and server. Initially, user's score is 20

This is just one of many possible problems we may encounter. It illustrates the problem of sending state instead of transforms. In case of using operations, the two operations will just be applied incrementally. First operation will be : + for the habit. It will be applied, and the score will be updated accordingly, then the second operation will be : mark Todo as done which will be applied and score will be updated accordingly. and the server responds with the final consistent state so that clients can sync to the latest server state.

As for the case of the user clicking + twice, I believe it's not possible because the score will get incremented once he clicks. We shouldn't wait for the server to respond in order to update the score locally. At least for the official mobile client, the game logic is replicated on both client and server. The server response is just a confirmation that the action was applied. Replicating the logic on both client and server is a common thing to do, especially in games to guard against cheating. and I think it can be easily maintained by having a shared repository for the scoring and game logic algorithms so that we don't need to write it twice.

yangit commented 11 years ago

I understand your point about syncing OT vs syncing state. When I said about clicking "+ habit" twice I meant clicking it on two separate clients. Client1 and Client2. Both of them meant to upgrade user.exp from 20 to 25. User clicked it on mobile. Switched mobile off. Went to desktop. Observe item is still not clicked. Click it again. To bring it from 20 to 25. Than goes back to mobile and suddenly he sees his score going all the way up to 30. He never wanted it. Because after sync we will have 2 separate "incr" events. After applying both server will get user.exp=30, and that is not desirable.

Lets speak more about API

wizonesolutions commented 11 years ago

So I feel like this is getting too complicated. We're talking about OT, but not everyone understands OT the same way. I don't really get it at all. I'm just pretending I do, and I understand the issues from context.

What we do understand is that sync conflicts can happen.

So, again, I advocate for no offline access in v1. When they make a change, we indicate visually that the change needs to sync, and we do it on the spot. We then indicate that they are in sync.

If they go offline in any way before they check that things are synced, then hopefully they at least won't expect to see the change elsewhere.

Offline mode is a whole 'nother animal. Then the client does need the game logic (and it could have the game logic to simulate the results, but that gets clobbered once the true state comes back fro the server if it deviates, probably with a notification indicating the same).

Offline mode should be explicitly indicated and limited to certain actions that can be synchronized predictably. When they leave offline mode, the client should first fetch the current state of things from the server. Preferably it'd be RTM-style, where create, modify, and delete actions are transmitted to the client, and it can then go over each of these and figure out what happened. I think each of these actions would need a date stamp too. Then the client would have to reconstruct the proper game state based on how things would have played out if everything had happened online the whole time. So it actually has to rewrite certain parts of the game state, and the server has to understand what happened in the end so it can tell the other clients what they need to sync down.

@lefnire You said earlier that OT was really easy compared to last_sync. Given people's other comments, does that still hold true? Maybe we need a for-dummies version of what you're saying so our objections can be slain.

uvwxy commented 11 years ago

Here's a word from someone working with the APIv1 on Android..

Writing a client that works in offline mode is something I will never do, as I don't want to play "keep up" with fast changes to the scoring system, i.e. updating your code to include a change in the streakGPModifier, etc. Imho, this has to be done by the server, also I need a call to cron the damage at the specified time, otherwise I would have the same problem as above.

Now, waiting for 10 seconds every time I tick a box is not something that is nice to handle on a phone. Here I will go so far as collecting user clicks, and process them in the background and mark them as complete if they stop within a couple of minutes with a reply from the server.

Everything that goes beyond that will make it too complicated, and does not really deserve the naming "API", when I just use it to store my client state, regardless of the correctness of my implementation.

As the API currently works I'm quite happy (I just need the "cronMyDamage" call ;) )..

yangit commented 11 years ago

No offline access means every change is not reflected until server actually sends us reply? Is not it going to be extremely annoying?

Imagine you open the app to click through a few todos and add\modify 3 rewards. You would have to take it really slow.

uvwxy commented 11 years ago

That would be my point of collecting them in a list, and then check them in in the background, or by the click of the button.

I will animate a fighting character, to entertain the user in the meanwhile ;)

yangit commented 11 years ago

To collect them and you need to "work offline" during that 5 seconds it taks to do roundtrip to server. Let's say you want to add a task and edit it right away (for advanced options). Do we have to wait for task to become saved before we can edit it's properties? And that is already "offline mode".

We have to freeze interface, and apply all the changes on server only. Or write full fledged 3-way sync code. There is no between. Am I wrong?

uvwxy commented 11 years ago

I have not added "adding a task" yet. But, then I would make a view where you can setup everything at once. To not make this frustrating I have to save the entry, and try to add it to the server, and add it to the client's tasks once this is completed successfully. (If not, keep it in a list of "things needed to upload to server", which can be restarted by the user)..

I would not go as far as freezing the entire GUI, but make it clear that certain actions have not made their way to the server yet. Do as much as possible in the background, or on command only.

yangit commented 11 years ago

How about this. We have a Model (JSON object), View, List of Transformations, Controller.

And than:

uvwxy commented 11 years ago

how do you model the transformation with JSON, and how do you append to it without reimplementing the scoring on every client?

also they need to exchange which transformation they have or not.

switz commented 11 years ago

I don't see why the user has to know what the result of their actions are while offline. Just go into an "offline" mode and keep track of how many moves the user has made. Make it clear that they're offline, and they have X operations waiting to be sent to the server. When it connects, it send all of the operations individually as it would normally. The user will clearly know that when it reconnects, it will send all of the operations at once.

If we do want logic on the game side, then the only thing I feel comfortable with is a private custom API for the official client only.

We're facing an issue here of splitting the app into two forks - one mobile app and one web app. If we think plenty of people want extended offline usage, then we're going to have some conflicts syncing the two at some point. I think for now, we should take the simplest approach possible. If in v2 we decide want to have a more powerful offline experience, then we can put the necessary time in to build that out. The faster we get this in the hands of the users, the better.

I should add, that this is exactly how the website works right now. If you're offline, it queues up the model changes until it reconnects and then syncs the changes.

yangit commented 11 years ago

@uvwxy whats wrong with having same controller on each client? Just include a JS file. If we need a thin client than it can freeze the interface and ask server to submit calculated model. But what is the point? Freezing interface is not so nice. And w\o model you can't even accept a button click w\o roundtrip to server lag.

Sync of transformations is easy AS LONG AS they are unique. Client<=>server can exchange hashes of array of transformations UUIDs. If hases are equal, then everything is in sync. If not. Each submits his own list of UUIDs and than missing UUIDs accordingly. That is very straightforward, repeatable and fool proof.

uvwxy commented 11 years ago

@switz: this is exactly how I see it. keep it stupid simple for now.

leave the offline/complicated/sync stuff to the official client, or create a proper sync api for each platform.

yangit commented 11 years ago

@switz

I should add, that this is exactly how the website works right now. If you're offline, it queues up the model changes until it reconnects and then syncs the changes.

When you click on a button, interface immediately reflects the change. Is not this offline mode? Changes are still unsyncd. But you can see the difference. You need the client-side model for that.

yangit commented 11 years ago

@switz

I don't see why the user has to know what the result of their actions are while offline.

Because they click, and nothing happens for 1-2-3 seconds. They want to see something changed. And they do with current Derby implementation. Try to open habitrpg.com and disconnect network. You can still operate the interface.

uvwxy commented 11 years ago

.. and this is what makes it complicated. If there is only one client doing this, that's fine. if you want to have multiple different platform clients, you have to keep in sync, which makes this almost a peer to peer approach.

when we have a server why not move the calculations into the server?

also: no one would complain if a website doesn't work anymore because they are disconnected. this is the usual experience ;)

btw: does habit stay in sync when I use multiple tabs/browsers?

wizonesolutions commented 11 years ago

I do like @yangit's idea (well, probably cuz it's similar to mine) for the offline mode. But I support @switz on getting something out sooner. We just have to tell users in BIG FAT LETTERS to make sure the phone app has synced before they do stuff on the web. Usually people look for a known issues section when using beta software, so hopefully they'd read. Maybe pop up an intro notification the first time they run the app. (cc:@litenull)

switz commented 11 years ago

@yangit

Right, but this is handled for us automatically. We had to write absolutely zero code thanks to Derby to allow this to happen.

All I'm saying is that we shouldn't dive down the rabbit hole and spend all this time rebuilding obtuse functionality for the sake of a corner case. We should be thinking about the MVP. Or, we should go ahead and commit to DerbyJS and build an app written in Derby - at which point all of this is handled for us.

First off, we can easily mark off the item when they click it - so it's not that nothing is happening. Second, if they have no internet connection, we can make it clear that a specific number of operations are queued up and waiting to be sent. You know exactly how plus-ing 3-5 dailies/todos/etc are going to affect you.

Frankly, I'm starting to get afraid of how overtly complex we're making this - and until Tyler gets to @SlappyBag's place, we're not moving ahead on anything. I move that we have a hangout then between Tyler, myself, the other developers, and a few of the major API consumers. If people would like to watch it happen – and Tyler doesn't mind, we can make the hangout public.

I love that this is an open company and that our users care enough to take part in the discussion. Thanks guys :+1:

yangit commented 11 years ago

@uvwxy Your concern is that each update in API will make it difficult for multitude of alternative clients to update their code to calculate model? I.e. we have released a new type of transformation which your client is not capable to compute?

Ok. We can have "Compute model for me" API call. And that will return you full JSON. It might include some extra fields. Are you ok with that? Just before you do that, please make sure list of transformations is in sync. And any transformations you submit are in compliance with official list.

Does it sound simple?

uvwxy commented 11 years ago

ACK ;)

yangit commented 11 years ago

@switz I'm too somewhat afraid to dive into stuff that complicated. But take a look at list of critical bugs in our bug-tracker. 80% of them are related to internals of Derby code. Derby is so troublesome because of event model. And things go haywire as soon as we click here and there and tons of events emitted, some of them emit ajax sync and that sync in turn emits more events and they all work with same user object and eventually we end up with messed up user model without any trace how this has happend.

80% of bug-tracker is not an edge case!

Having a stack of transformations will tremendously help to debug problems.

lefnire commented 11 years ago

Ok, I'm back in WiFi zone in Southampton hanging out with @Slappybag. Everyone here that's available tomorrow, let's have a G+ chat about API. @thedecimal and I are having it at some point (undecided time), and we'll be jumping into implementation shortly after - so all who want big say say aye, and any time that works for you (GMT incl)

On Saturday, May 4, 2013, Yan wrote:

@switz https://github.com/switz I'm too somewhat afraid to dive into stuff that complicated. But take a look at list of critical bugs in our bug-tracker. 80% of them are related to internals of Derby code. Derby is so troublesome because of event model. And things go haywire as soon as we click here and there and tons of events emitted, some of them emit ajax sync and that sync in turn emits more events and they all work with same user object and eventually we end up with messed up user model without any trace how this has happend.

80% of bug-tracker is not an edge case!

Having a stack of transformations will tremendously help to debug problems.

— Reply to this email directly or view it on GitHubhttps://github.com/lefnire/habitrpg/issues/879#issuecomment-17439735 .

arscan commented 11 years ago

I'm around in the morning EST for a bit. If I don't make the hangout, I think my views are basically in line with switz and uv and wiz (keep it simple on the public api in the short term, maybe have a private protocal for the official app later that has game logic on the client). My opinions are documented early in the thread so I won't restate them again. But as habitrpg-txt demonstrates, you can get really far with a super simple api... As long as the app sets expectations appropriately.

Thanks for opening the convo up to the community - lots of great perspectives here!

litenull commented 11 years ago

Im not gonna flood the thread with a point of view, Rob, switz, wiz pretty much explain everything. +1 for simple API.

On Sat, May 4, 2013 at 11:36 PM, Rob Scanlon notifications@github.comwrote:

I'm around in the morning EST for a bit. If I don't make the hangout, I think my views are basically in line with switz and uv and wiz (keep it simple on the public api in the short term, maybe have a private protocal for the official app later that has game logic on the client). My opinions are documented early in the thread so I won't restate them again. But as habitrpg-txt demonstrates, you can get really far with a super simple api... As long as the app sets expectations appropriately.

Thanks for opening the convo up to the community - lots of great perspectives here!

— Reply to this email directly or view it on GitHubhttps://github.com/lefnire/habitrpg/issues/879#issuecomment-17441898 .

LP, Tomaž :)

lefnire commented 11 years ago

ok, i'm gonna start a G+ now with @thedecimal and @wizonesolutions and anyone else who wants in, just ping me or hit #habitrpg irc

switz commented 11 years ago

Sorry, I wasn't up that early. What happened during the discussion?

On Sunday, May 5, 2013 at 8:02 AM, Tyler Renelle wrote:

ok, i'm gonna start a G+ now with @thedecimal (https://github.com/thedecimal) and @wizonesolutions (https://github.com/wizonesolutions) and anyone else who wants in, just ping me or hit #habitrpg irc

— Reply to this email directly or view it on GitHub (https://github.com/lefnire/habitrpg/issues/879#issuecomment-17450734).

lefnire commented 11 years ago

Nothing really, sounded like we were all on the same page after this conversation actually lol

On Sunday, May 5, 2013, Daniel Saewitz wrote:

Sorry, I wasn't up that early. What happened during the discussion?

On Sunday, May 5, 2013 at 8:02 AM, Tyler Renelle wrote:

ok, i'm gonna start a G+ now with @thedecimal ( https://github.com/thedecimal) and @wizonesolutions ( https://github.com/wizonesolutions) and anyone else who wants in, just ping me or hit #habitrpg irc

— Reply to this email directly or view it on GitHub ( https://github.com/lefnire/habitrpg/issues/879#issuecomment-17450734).

— Reply to this email directly or view it on GitHubhttps://github.com/lefnire/habitrpg/issues/879#issuecomment-17455037 .

pjf commented 11 years ago

So, I've arrived late to the discussion, so I'm with @litenull for not providing a point of view, but I am reading. :)

@lefnire: Huzzah, we're all on the same page! Any chance for a brief summary of what was decided?

StanLindsey commented 11 years ago

Edit: Went back up the read the rest of this thread and realised y'all had the same idea. Carry on folks.

On the subject of local vs server algo it may be an idea to have a private API that we can use to push/set updates from the client that have had their logic processed on "official" clients. This would need unofficial apps by forcing them to only send up/downs but would allow our official apps to process things locally and run offline.

If that is the case when our official app gains connectivity again we should send the stats prior and after the algos have been ran. E.g. Oldtaskhealth:20, newtaskhealth:30, operation:up. The server can then check for a conflict knowing what the client was attempting. This would help with % based actions, the server would just apply the update if correct or re-run the operation. If conflicts get too deep you could warn the client and ask them whether to replace data with the other,

5:30am, spitballing here. yawn


Posted On Mobile

pjf commented 11 years ago

@SlappyBag : Oh dear. I really don't love the idea of any sort of closed API that's only available for 'official' clients. Besides from the hassle of keeping it secret in an open-source project, it's a serious discouragement to third-party development, some of which will be awesome cool stuff that would never happen 'officially'.

StanLindsey commented 11 years ago

Oh I totally agree @pjf - its just an option in the decision of whether third-party clients have the ability to process algos locally - potentially doing things wrong, misrepresenting us / cheating. I'm thinking outloud here really.

We could go the route of "trusted" API users?