-
Notifications
You must be signed in to change notification settings - Fork 15
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
Replays #42
base: target-gd3.2
Are you sure you want to change the base?
Replays #42
Conversation
snapshotdata.gd: Now has a separate "lifetime history" of stored snapshots and EncDecBuffer. New function, _add_to_lifetime_history, that accepts a snapshot, encodes it and appends it to to the end of the lifetime history before clearing the lifetime history buffer's buffer New function, _convert_replay_to_snapshots grabs a replay of encoded snapshot data and decodes it through the lifetime history buffer, replacing each index of the replay with a snapshot before clearing the lifetime history buffer and returning the replay itself. New function, _convert_replay_to_lifetime_history, which is the same as the previous function, but instead of modifying the replay array and returning it, the snapshots are decoded and deposited directly in the _lifetime_history array. replay.gd: stores gzip compressed "replays" of serialized snapshot data to files. If the game is running out of the editor (even if project is not opened specifically), replays are saved to res://replays/ . Otherwise, replays are saved to a replay directory in Godot's userdata folder. gitignore updated to not include .REPLAY files or the replays/ folder.
Cleaned some code that didn't do anything Capitalized print strings Reorganized/changed some of the print strings get_file_name renamed to make_file_name for clarity organized similar file opening functionality into a simple function call removed redundant file_path_normal and file_path_debug functions Saving a replay now has an optional "name" argument (default is just 'Replay')
OK. There are several things that must be mentioned about this PR, both in "general terms" and in code details. I do like the idea of having a replay system and I did consider it in the past. Although never got to work on it. Now, to the "general" things that I must mention: 1 - I know the replay system will work a lot better by using data obtained from the network system. However I would prefer the replay thing to be separated, having a dependency on the networking addon. The reason? Not everything needs a replay system, so having it separated means that it can be enabled/installed only if there is any desire in the project. Unfortunately for projects that would want the replay system but not the networking one, well... there isn't much that can be done. The network system contains a lot of stuff to help take the entire game state. So, yeah. 2 - If a replay system is to be implemented, it should also provide means to playback that stuff. In this regard, I would probably have some code repeating what the replication system does, however it can be greatly simplified since there is no need to deal with player input (other than interacting with the UI itself), nor further replicating any other data. 3 - Usually speaking I don't like hard coded things. Only when it makes sense. That said, the location where the replay data is stored should not be hard coded. In here either the functions that save/load the replays should have additional arguments that indicate the path to the file or those are added as project settings. Requesting extra arguments means slightly harder to use functions but with greatly added flexibility. I believe this is a very good trade of. As an example, with this it becomes possible for the game to provide the possibility for the player to save the replay wherever he/she wants to. 4 - There are a few details regarding the code itself, which I will use the code commenting system to do so. |
.gitignore
Outdated
@@ -12,3 +12,7 @@ export_presets.cfg | |||
# Mono-specific ignores | |||
.mono/ | |||
data_*/ | |||
|
|||
# Replay-specific ignores |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following the idea that functions should expect the path to the replay file, it becomes responsibility of the project itself to deal with version control ignores.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally agree. Also, after sleeping on it, I think:
-
A replay folder in the project itself is a redundant and overly-complex idea. Even if someone wants to separate where replays are stored based on parameters such as debug and/or in-editor builds, other scripts can decide where how files are accessed.
-
On a related note to letting someone choose how they want to structure their file setup, I think another change I should make to how the replay class interacts with the filesystem is to get rid of save path constants, and just let per-project functions take care of choosing where to save and access replay files.
Summary: Probably best to simply not update the .gitignore because its specific ignores will be deprecated soon and storing replay files in a git project is a bad habit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just read the remaining comments you left... We agree the file locations shouldn't be hard coded, haha! Sorry I mentioned that as if you hadn't already...
addons/keh_network/snapshotdata.gd
Outdated
@@ -40,6 +40,8 @@ var _entity_name: Dictionary = {} | |||
|
|||
# Holds the history of snapshots | |||
var _history: Array = [] | |||
var _lifetime_history: Array = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, ideally the replay system should become its own addon. Still, there are a few things that can be considered here. The replay object can internally store all data as arrays of Snapshots, very much like the expected _lifetime_history
. There is no need to have the encoded buffer as class object as it would be needed only to store the data in a file.
Also, the code style I would prefer to be more consistent with the rest of the addons, in which the static typing is very explicit, no like the line bellow using :=
syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decoupling replay functionality into a separate addon is a much better idea. I'll work up another prototype (and probably include a demo) to decouple the recording of replays. Will probably shift how the Replay class works from being a namespace with static functions for storing and reading serialized Arrays, to a replay with a history and its own encdecbuffer
. Come to think of it, replays also need to store some metadata, too.
The only concerns that I have with entirely decoupling Replays from the Network addon is that currently replay data is sent by "intercepting" snapshots as they're being passed through the _add_to_history
function that gets called from _on_snapshot_finished
. Not immediately sure on how best to add a system that grabs the snapshot getting passed through the network singleton's _update_control
without modifying the network script itself. For instance, if replay.gd
becomes some kind of singleton below the network singleton, it could, for instance, swap out _update_control
's sfinished
funcref to an alternative one that calls network.on_snapshot_finished
and adds the snapshot to the replay's lifetime history before or after the call. But that seems like a really weird "spaghetti" way of tacking on replay functionality. But ultimately I want storing replays to be as unintrusive for someone to implement and performant as possible. Ideally someone would just "press a button" and whenever they're in a network session, they're also recording a replay. Of course, what with how replays are currently recorded, this doesn't even work for clients-- only the server/host records replays. My point is, I'll have a tricky time with making an easily activated replay system. Very open to suggestions on "where" in the network loop (if in the network loop at all) snapshots should be recorded.
In regards to syntax changes I can (and should) totally update those! It definitely makes things much clearer. I remember just wanting to prototype code quickly, so I skipped over some of the writing out of type definitions. (I'll also clean up some of the unhelpful comments).
Thanks for all the help!
replay.gd: File moved to new folder Added copyright notice header Replays can now be initialized as their own objects snapshotdata.gd: Removed previous coupling with replay.gd, now 'reverted' to previous. TODO: implement a variable rate at which replays store full snapshots to make playback easier.
Implement description draft
main.gd: replayviewer filepath fixed replayviewer.gd and .tscn: general updates megamain.gd: replay instance variable, records replays if project settings say so replay.gd: has scene_path instance variable (serializes it when saved), added functions to check if a snapshot is a full snapshot based on its relative signature, and to check if a specific snapshot from a replay is a snapshot. also added some helper funcs for accessing the "status" of a replay. (How 'long' a replay is, how far through the replay a given frame index is). made the replay an actual plugin
reading a serialized replay gets up to build_tracker after replayviewer.gd calls read_replay --> load_new_replay --> deserialize_history --> decode_delta
snapshotdata.gd:
Now has a separate "lifetime history" of stored snapshots and EncDecBuffer.
New function, _add_to_lifetime_history, that accepts a snapshot, encodes it and appends it to to the end of the lifetime history before clearing the lifetime history buffer's buffer
New function, _convert_replay_to_snapshots
grabs a replay of encoded snapshot data and decodes it through the lifetime history buffer, replacing each index of the replay with a snapshot before clearing the lifetime history buffer and returning the replay itself.
New function, _convert_replay_to_lifetime_history, which is the same as the previous function, but instead of modifying the replay array and returning it, the snapshots are decoded and deposited directly in the _lifetime_history array.
replay.gd:
stores gzip compressed "replays" of serialized snapshot data to files. If the game is running out of the editor (even if project is not opened specifically), replays are saved to res://replays/ . Otherwise, replays are saved to a replay directory in Godot's userdata folder.
gitignore
updated to not include .REPLAY files or the replays/ folder.