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

Implementing HISTORY_GAMESTATE_REPLCIATED_TIME_AS_DOUBLE #46

Merged
merged 4 commits into from
Sep 10, 2023

Conversation

chrisai-dev
Copy link
Contributor

For backwards compatibility I created a new double variable.

@@ -108,6 +108,9 @@ public class GameState : INetFieldExportGroup
[NetFieldExport("ReplicatedWorldTimeSeconds", RepLayoutCmdType.PropertyFloat)]
public float? ReplicatedWorldTimeSeconds { get; set; }

[NetFieldExport("ReplicatedWorldTimeSecondsDouble", RepLayoutCmdType.PropertyDouble)]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is/was that property really named ReplicatedWorldTimeSecondsDouble?
I thought it replaced ReplicatedWorldTimeSeconds with a breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, going forward there will only be ReplicatedWorldTimeSecondsDouble in the replays. However, old replays will still have ReplicatedWorldTimeSeconds which is a different datatype.

Options:
1: keeping both (current PR)
2: keep ReplicatedWorldTimeSeconds, and cast the new property from double to float
3: keep ReplicatedWorldTimeSecondsDouble and cast the old property from float to double (will break old code that uses this repo)
4: replace ReplicatedWorldTimeSeconds with ReplicatedWorldTimeSecondsDouble (new versions of this repo won't be able to parse old replays)

It's tricky, I think option 4 is the worst, at least for me I'm parsing old replays as well. It's your call, just let me know and I'll do it.

@Shiqan Shiqan merged commit 99351b1 into Shiqan:master Sep 10, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants