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

Support for splitting and filtering views #59

Open
endel opened this issue Jun 17, 2017 · 31 comments

Comments

Projects
None yet
8 participants
@endel
Copy link
Member

commented Jun 17, 2017

Allow filtering the state before sending to specific clients - which should solve the current issue the framework has with a large amount of data in the state being sent to every client.

Related issue: #58

@seiyria

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2017

In my case, I have a vested interest in this because I am making an RPG. Some background:

my rooms are maps - they can be up to 200x200 in size, tile-wise, but that's just an arbitrary number. however, along with that, there are up to 500 monsters running around (and some are calculating FoV too). so, the only monsters a player really needs to see are the ones in their viewport (9x9) + probably a few tiles outwards as a grace calculation, not every npc on the map. along with that, each tile can have an arbitrary number of items on the ground. that's probably what's killing me right now, to be honest - if someone drops a hundred items on every tile, they probably just killed the game.

however, if each player only gets 9x9 + a few tiles of ground items, then that would probably be fine. the server might slow down because of it, but it wouldn't be the end of the world. currently, I have a garbage collector running every 15 minutes (items last an hour on the ground before they're killed), so it's very aggressive cleanup.

So, my proposal takes this form: a step in the delta process that allows for customizable states sent to each player. How this could work:

toJSON -> customizeStateForClient -> msgpack.encode -> state check -> delta create -> delta broadcast

This means that each client will need their custom state stored and delta'd against, instead of one state being stored and delta'd. However, depending on the size of the global state and the size of the client state, it could overall be less data sent (especially, less data to each client). How this would work for me:

If one player is killing monsters on a map, right now they get every item on the ground as well as every creature wandering around. Instead, they could get all creatures and items in a 15x15 radius (the map is 200x200, for example), which would likely significantly decrease the amount of data sent to them. If another player is across the map, they don't need to know specifically about that players information unless they're nearby to them. This means that they get two individualized small states (which are very small compared to the theoretical global state).

I suspect that in most cases, a global state will be sufficient, but when you have a big state, it might merit breaking up.

Additionally, supposing that multiple players were in the same region, they could also get the same state. That would be more complex to understand, detect, and ship, but on a base level it is not necessary.

@endel endel added the performance label Oct 25, 2017

@endel

This comment has been minimized.

Copy link
Member Author

commented Oct 25, 2017

@seiyria thanks for sharing your interest here :)

Before going for splitting the data, I think there's still room for improvement on the default diff behavior of the framework.

Here are my findings so far:

  • When operating in large datasets, toJSON is a huge bottleneck. (I've made a simple benchmark, using the data you described, and it could perform only 13 times per second.)
  • The next in line is encoding the delta again. This can perform 1000 times per second. This step could be completely skipped by receiving Protocol.ROOM_DATA differently in the client-side.

Maybe we should disable toJSON by default. This is a remaining from early versions, when I didn't know it's possible to make attributes non-enumerable (non-enumerable properties are ignored during serialization). Now there's a @nosync decorator that can be used for this.

I'll continue sharing as I make some progress on this.
Cheers!

@seiyria

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2017

I'm definitely interested in seeing how the latter functionality would work! If it is as performant as you describe, then I might not need to worry about splitting the state for people.

@endel

This comment has been minimized.

Copy link
Member Author

commented Oct 26, 2017

@seiyria I've updated the docs to discourage the usage of toJSON. The latest version (0.5.11) won't use it by default anymore: https://github.com/gamestdio/colyseus/wiki/Room-state#tojson-will-be-deprecated

@seiyria

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2017

@oyed

This comment has been minimized.

Copy link
Member

commented Oct 27, 2017

@endel This change does however force people like myself to move to a build system server-side. Currently I use pure ES6 as Node has great support, but given class decorators are stage 2, it won't support them for a while, meaning I'll have to just import nonenumerable manually and use it a bit differently.

I'm not sure forcing server-side build processes is the best move, but as long as an alternative route (That isn't deprecated, i.e. toJSON) is documented, like using nonenumerable manually, I agree this is a good step forward.

@oyed

This comment has been minimized.

Copy link
Member

commented Oct 27, 2017

Another potential "issue" is that I use underscores to prefix all properties in classes, e.g. _x, _y, and then remove the underscore in the toJSON object. I think it may be a bit strange for the Client to be listening for "private" vars?

@seiyria

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2017

@oyed

This comment has been minimized.

Copy link
Member

commented Oct 27, 2017

@seiyria I meant more that the client is receiving _x instead of x, which is pretty unnecessary, and given the outlook on autosync support on colyseus.js it'd mean changing a lot more client-side for me.

@endel

This comment has been minimized.

Copy link
Member Author

commented Oct 27, 2017

@oyed the autosync will be optional, you won't be forced to use it.

I suggested the decorator because it's easier to understand, but you can always call Object.defineProperty manually, to change the accessor descriptors. Just added this link to the docs as well.

@oyed

This comment has been minimized.

Copy link
Member

commented Oct 28, 2017

@endel I agree that the decorator is much easier to understand, my problem is that I cannot actually use it due to Node not really having support for many Stage 2 features, such as decorators, and I don't want to create a build process just to transpile decorators. I'm not the only person without a build process server-side, and by removing toJSON with no alternative other than decorators, you're sort of forcing a server-side build process.

EDIT: Just saw the docs update - so disregard my comment :) Maybe including a helper function that does the same thing as the decorator would make it a bit easier?

@amir-arad

This comment has been minimized.

Copy link

commented Mar 9, 2018

+1
I'm checking colyseus out for a 3d space game with lots of objects. this feature (the one in the title, not the serialization optimization) seems like something I'll need later on.

@seiyria

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2018

I should add here also how I solved my initial issue: I don't put most of these things in the state to be synced with the client. I have separate functions that add/remove and I send them to the client to be handled accordingly. So, in my NPC example, instead of having a state, with npcs, and doing npcs.push(newNPC), I do:

room.broadcast('add_npc', { npc })
room.broadcast('del_npc', { npc })

This way the server isn't trying to sync everything. I got something like a 20x performance boost (avg. delta between loops from 2s down to under 100ms). I do this with other things in my game as well. The only thing synced by the game loop AFAIR is the players.

@amir-arad

This comment has been minimized.

Copy link

commented Mar 9, 2018

event-sourcing FTW

@oyed

This comment has been minimized.

Copy link
Member

commented Mar 9, 2018

@seiyria Do you batch these in to patch-like broadcasts though? For example, if you were to add 100 NPCs at the same time (Just a use-case, literally wouldn't happen), are these batched in to a single broadcast? If not, might be worth looking in to doing a separate syncing process that stores what should be synced in a local state not tracked my Colyseus, then every X milliseconds see if anything is in there and if there is, broadcast it all and wipe the local state.

@seiyria

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2018

@oyed Yes, I think I have support for batching in my (primitive) setup - but this was a "do it and forget I did it" sort of thing. I don't know if it would be worth having a sync process, because of how I do it. Once I've given the NPC to the client, I have a separate set of "volatile" data, mostly positional and visual data, and only that is ever updated. The other NPC data is essentially immutable.

Also, I've tried using tools like Deepstream to manage syncing. It kinda worked, but ultimately gave me a ton of headaches.

@oyed

This comment has been minimized.

Copy link
Member

commented Mar 9, 2018

@seiyria Fair enough - it's a very use-case scenario, if it works for you then it works.

@endel endel modified the milestones: 0.9.0, 1.0.0 Mar 11, 2018

@IamCarbonMan

This comment has been minimized.

Copy link

commented Mar 22, 2018

Another reason this would be helpful is for games where not everything is public knowledge- for example card games where all players know the number of cards in each others' hands, but only the owner knows the value of those cards.

@amir-arad

This comment has been minimized.

Copy link

commented Mar 22, 2018

@IamCarbonMan that's only relevant if you try to defend against destructuring the client or socket.

Naively, I'd say it's the job of the game client to hide whatever needs hiding from being displayed to the player. This way you leave the client with the ability to run predictions, which is important in real-time games running over a network.

@IamCarbonMan

This comment has been minimized.

Copy link

commented Mar 22, 2018

@amir-arad In some cases including the card game example, latency is a much lower priority than preventing cheating. Just because something isn't displayed in the UI doesn't mean someone won't find a way to dig it out using the console or an external script/extension.

Also, according to the FAQ on colyseus.io (and the source code as far as I can tell), Colyseus doesn't do any form of client-side prediction:

Colyseus does not provide any client-prediction solution out of the box. Games such as wilds.io and crashracing.com do not use any form of client-prediction. lerping user coordinates usually gives reasonable results.

@seiyria

This comment has been minimized.

Copy link
Contributor

commented Mar 22, 2018

I can confirm that players are pretty happy to reverse engineer and dig things out with the right motivation. Happens to me with my games, which are already thankfully open source.

@IamCarbonMan

This comment has been minimized.

Copy link

commented Mar 22, 2018

As @seiyria said, when sufficiently motivated your client-side code will be modified. That's why security has to happen on the server. That being said, this feature isn't extremely necessary, but I see it as something that will need to be added for Colyseus to eventually be considered feature-complete.

@endel

This comment has been minimized.

Copy link
Member Author

commented Mar 22, 2018

@IamCarbonMan I agree.

The problem, though, is with the current state patch system. It would need to be changed completely in order to sync data with particular clients. It would consume too much CPU having to compute one binary diff per client.

In the meanwhile, I like the solution @seiyria provided here #59 (comment). You can also send data directly to a single client, by using send().

Any data sent from the server through broadcast() or send() will arrive in the room.onData signal on the client-side.

@IamCarbonMan

This comment has been minimized.

Copy link

commented Mar 23, 2018

I don't think performance is a reason to dismiss a useful optional feature. Especially since #58 is tracking pluggable serialization / diff-patch algorithms. Documentation needs to point out that this can degrade performance, of course. You also have to keep in mind that the performance argument doesn't apply to the example use case; Colyseus trades server-side performance for bandwidth because it's designed to support realtime physics-based games that need low-latency data sync. But that trade-off isn't always the right choice, for example in the case of turn-based strategy games like @seiyria and myself have mentioned.

It would need to be changed completely in order to sync data with particular clients.

Not really. If the Room subclass implements a function clientView (client) -> state then we make a few changes:

  • Here we calculate the currentState using the return value of clientView, called with each client in the room.
  • Here we generate a patch for each such currentState.
  • Here we use send() instead of broadcast(), sending the matching state to each user.

I may be missing some things but implementing this seems to be very easy, with no performance impact when not enabled.

@IamCarbonMan

This comment has been minimized.

Copy link

commented Mar 23, 2018

I would also like to point out that in reality, you're not trading off much by using binary deltas and msgpack on every websocket message. Unless the diff you're sending is 1K or more in size, it's going to fit into about the same number of websocket frames using the default ws options. In fact, you could even send the entire state object every time and in many cases you wouldn't notice the difference.

@kanecko

This comment has been minimized.

Copy link

commented Apr 26, 2018

Is there any ETA on this?

@MadeByMike

This comment has been minimized.

Copy link

commented Apr 29, 2018

I made a super naive attempt at this: master...MadeByMike:master.

It's a long way off a PR as I'm totally unsure what the performance impacts might be. I'd love some feedback, suggestions and help testing.

Existing unit tests are passing and I've added an additional test for a simple filter condition. I mostly followed @IamCarbonMan's suggestions.

BTW: Sorry for all the unnecessary formatting changes but I could not find an eslint configuration that matched the current formatting. (will hopefully figure this out later).

@MadeByMike

This comment has been minimized.

Copy link

commented May 22, 2018

I've improved this slightly. Because the previousState mutates, it quite often matches the currentState and because of that I can't tell if an individual clients view has changed.

Currently this is managed by storing the previousStateEncoded and comparing to the currentStateEncoded neither of which can mutate. I need to do the same so I needed to keep track of each clients previous encoded state in an array called previousClientViewStateEncoded.

I found I was able to get rid of this._previousState entirely. I'm not quite sure why sendState() attempts to send the previous state. I assume it would work just as well to send this.state but I followed the existing convention.

This is drifting from master a little bit, I'd love to get some feedback @endel

@austinkelleher

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2018

I also have a very similar use case as @IamCarbonMan described here. I am building a game where there is data that is private to each individual client. The individual clients should be aware of values of this data, but the entire room should not be aware of the values (to prevent cheating). People will indeed reverse engineer the client. I have seen this done as well.

I'm new to using colyseus, but I wonder if using the @nosync decorator on a player property and sending the data directly to that client using send would be an appropriate method of tackling this problem. What do you think @endel?

@endel

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2018

Hi @austinkelleher, sure! That's the best way to achieve that for now!

endel added a commit that referenced this issue Mar 10, 2019

endel added a commit to colyseus/schema that referenced this issue Mar 10, 2019

@endel

This comment has been minimized.

Copy link
Member Author

commented Mar 10, 2019

Hey all! I'm starting some experiments to have this hopefully on version 0.10.x, using @colyseus/schema.

The API so far is looking like this:

Filtering entities by distance:

import { Schema, type, filter } from "@colyseus/schema";

export class State extends Schema {
  @filter(function(this: State, client: any, value: Entity) {
    const currentPlayer = this.entities[client.sessionId]

    var a = value.x - currentPlayer.x;
    var b = value.y - currentPlayer.y;

    return (Math.sqrt(a * a + b * b)) <= 10;

  })
  @type({ map: Entity })
  entities = new MapSchema<Entity>();
}

Filtering data only for its owner:

import { Schema, type, filter } from "@colyseus/schema";

export class Player extends Schema {
    @filter(function(this: Player, client: any, value: number, root: State){
        return root.players[client.sessionId] === this;
    })
    @type("number")
    secretNumber: number;
}

export class State extends Schema {
  @type({ map: Player })
  players = new MapSchema<Player>();
}

I'm not sure of all use cases we can have for this, let me know if you have any thoughts about the API!

The signature for the filter callback so far has this as the instance of the class @filter has been applied to; the client instance as second argument; value of the filtered field as third argument; and last is the root object of the state tree:

type FilterCallback = (this: Schema, client: Client, value: any, root?: Schema) => boolean;

@endel endel self-assigned this Mar 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.