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

Game Log Improvements #227

Open
4 of 6 tasks
nicolodavis opened this issue Jun 29, 2018 · 14 comments
Open
4 of 6 tasks

Game Log Improvements #227

nicolodavis opened this issue Jun 29, 2018 · 14 comments

Comments

@nicolodavis
Copy link
Member

nicolodavis commented Jun 29, 2018

1. Secret Log Events

We have secret state, but log events all go to a global log. We need some mechanism to allow sending a customized log per player so that secret state isn't leaked.

  • Status: not started

2. Custom payload

Allow insertion of custom messages into each log event.

3. Incremental log updates

Sending the entire log on each sync is going to eventually become a performance problem for large games. Maybe we should just send the log events since the last event that the client has?

4. Store entire state at each log event.

The GameLog cannot correctly replay actions over partial state (which is the case when secret state is used). We should store the state object itself at each log event (which also makes a strong case for incremental log updates). Perhaps the best approach here would be to actually separate out the log object from state so that it can be stored in a separate area of storage and be retrieved independently. That way we also don't read a very large object containing its own history every time we apply an update using the game reducer. This should probably wait on #251 because the server component will start to get a bit more complex and it would be good to be able to test all these interactions accurately.

  • Status: not started

5. Handle endPhase in log

We need to render something in the log for endPhase.

6. Store log in separate area of storage.

Logs are currently stored along with the state object. We should probably store them in a separate table so that they are never read during updates. We could probably also make the storage API more descriptive to do this. It currently has set / get, but we could probably change it to addLogEvents, updateState, getState, getLog etc.

@Stefan-Hanke
Copy link
Contributor

This has been a while. Correct me if I'm wrong. This is just a list of thoughts that I've fished while thinking about the game log.

  • The log currently is just an array that is added to at two locations in reducer.js and flow.js. It contains the raw actions that are sent to the reducer. The action itself already contains the playerID from the player performing the action.

  • I don't know what constitutes credentials but looks like this should be stripped for the log in any case (even for the player who'd performed it).

  • Actually I don't fully understand what you mean by leaking secret state because of an action. Well, maybe that action's arguments contain some secret for that specific player, which of course should not be visible to other players.

  • I'm kind of inclined to using PlayerViews for that. You?

  • The third point looks like not belonging to a V1.0 release. I don't know whether a file can be linked to an issue; do you have specific reasons to not use GitHubs Milestone feature?

  • The fourth point looks like being UI-specific? React? Native? Whatever?

@nicolodavis
Copy link
Member Author

@Stefan-Hanke

  • Yes, we should strip credentials from the log.
  • About leaking secret state, I meant an action could be something "move card A from my hand to my deck", which shouldn't be visible to other players. Yeah, we could use PlayerView for that too, although we'll have to repurpose it to filter out actions as well as state (as it does currently).
  • The third point may not be a release blocker, but we need to do some stress tests before we are sure of that. Not opposed to using GitHub milestones. I will probably start marking issues that specifically target v1.0 as such.
  • Yep, the fourth point only affects the bundled in rendering of the log.

@Stefan-Hanke
Copy link
Contributor

On Point 4 (Handle endPhase in log). Currently, it is just plain rendered. Did you mean to render something special like it's done for a turn?

@nicolodavis
Copy link
Member Author

nicolodavis commented Jul 24, 2018

Yes, something that indicates what the current phase is would be helpful in the log I think. It could potentially be a color coded wrapper around each log event (withsmall text on the top right every time the phase changes).

@nicolodavis nicolodavis added this to the v1.0 milestone Jul 25, 2018
@Stefan-Hanke Stefan-Hanke mentioned this issue Jul 25, 2018
2 tasks
@Stefan-Hanke
Copy link
Contributor

For the incremental log updates: while examining the data that is transferred, we could also strip _initial since this is sent to the client with the initial sync action.

@nicolodavis
Copy link
Member Author

Agree.

Also, I'm not sure if just sending actions back and having the client replay them is going to work in all cases. For example, with secret state, the client lacks the complete picture of G (and is actually incapable of applying actions correctly on partial state).

I think we'll eventually need to store the actual state object at each log event (in addition to the action), which makes an even greater case for incremental log updates.

@Stefan-Hanke
Copy link
Contributor

Stefan-Hanke commented Aug 13, 2018

About 2 (Custom Payload) - just some musings:

A log event is either an action that is triggered by a player, or an event either triggered automatically or not. I'm wondering where the insertion point of a custom message is. It should be at the point where the relevant move/event originates, so e.g. in the TicTacToe example here.

What in the case of a custom endTurnIf method in a flow; these methods currently return a bool. A game developer should be able to return a custom log message also so it can be inserted in the log message.

@nicolodavis
Copy link
Member Author

We can probably change the semantics of endTurnIf to return something like:

{ returnValue: ..., msg: ... }

Taking a step back, the motivation for wanting this was to prevent people from implementing their own logging datastructure. They are still free to implement their own rendering of the log, but ideally they wouldn't duplicate the log itself. Even with rendering, we should probably provide enough customization props in GameLog (CSS, custom rendering etc.), that makes it reusable for a large number of cases.

Having a custom payload in each log event would allow enough customization to prevent maintaining a parallel datastructure in G. That said, given that moves are pure functions (whatever you compute inside them can be derived from the arguments), and given that we are planning to store the entire state inside each log event, maybe this idea is now irrelevant.

@Stefan-Hanke
Copy link
Contributor

Yeah you could in theory run the computation again and rework what actually is going inside the log. However I don't know how this is going to work out. I think for a game dev it is easier to just create log message(s) inside the game logic itself. Otherwise, there would have to be some mechanism that can handle the move, and one that can create the log message. Maybe I'm thinking along the wrong lines...

The game logic itself (a single move) has the task to advance G. Now the log should allow a person to replay the game in his mind. So, I could view the log as accompanying data that is generated in a move. Can you think of any other data that could be the result of a move? This is just a thought experiment, dunno whether this actually makes sense.


Storing the entire state in each log event sounds like it's going to be big - maybe actually store a delta state and pretend to the outside that the whole state is stored?

@nicolodavis
Copy link
Member Author

Can you think of any other data that could be the result of a move?

No, I think you got it spot on. There is the advancement of G, and the log is just a journal entry that allows you to recount what happened.

When I look at real game implementations, the log is typically quite detailed and human readable. For example, there are usually log entries such as "Player A played Card 1" and so on, but also a summary at the end of the turn showing statistics about the move (and an explanation of various scores if necessary).

It's possible that all of this is merely a log rendering problem, but I also do like the idea of providing a hook to add a custom payload to the log event from within the game logic. Maybe we can reuse the ctx object like we have for the Random and Events API's?

move(G, ctx) {
  ...
  ctx.log.setPayload({ ... });
}

Storing the entire state in each log event sounds like it's going to be big - maybe actually store a delta state and pretend to the outside that the whole state is stored?

I'm not too worried about this actually, mostly because it's just storage cost (which is inexpensive). What I am optimizing for mostly is to eliminate the log from the update path (when a move is processed by the server). This is what will occupy most of the compute time on the server. Looks like we've already achieved this thanks to your deltalog change.

The database can just store a series of log events and return:

  • the latest state: for update events
  • the latest state and the log: for sync events

I wouldn't venture into delta states unless there is an easy way to implement them (and a performance concern that's justified by the additional complexity and the extra time it will take to assemble the state along the way since you only have deltas now).

This was referenced Aug 27, 2018
nicolodavis pushed a commit that referenced this issue Mar 7, 2020
Credentials are only used to check if an action is authorized in the 
master and should not then be passed on to the reducer etc.

This contributes towards #227, because now credentials should not leak 
beyond the `Master.onUpdate` method and won’t end up for example in the 
game log.
@tomasvidhall
Copy link
Contributor

I created a PR that will hopefully solve
2. Custom payloads

#865

@flarbear
Copy link

Perhaps an option to just not have logs in the first place? Or does the game logic need them for something?

For my game, I have undo turned off and the clients don't really need to deal with prior state in any way so the log is just superflous to be sending across in updates (and can leak info like the shuffled deck of cards in the initialState).

@delucis
Copy link
Member

delucis commented Dec 26, 2020

It should be possible to add an option to disable logs. The debug panel is the only core use of the logs, but that could be updated to only show the log tab when logging is enabled.

(initialState is separate from logs, that leak should probably be addressed by running it through playerView as we do for state generally.)

@gabrielecastellano
Copy link

Hi!
I am not sure about "2. Custom payload" to be fully functional. I can set the log message calling "ctx.log.setMetadata()" from within a move, but doing it from within a "onBegin" or "onEnd" function seems to not have any effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants