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

Add time handlers and TimeSeries object #166

Open
danfulton opened this issue Feb 22, 2022 · 4 comments
Open

Add time handlers and TimeSeries object #166

danfulton opened this issue Feb 22, 2022 · 4 comments

Comments

@danfulton
Copy link
Contributor

Issue #130 raises that time tracking changed with LoTV. In general, it's probably best to work in frames throughout project, and convert to seconds or game time using some centralized functionality. Centralizing this would make it easier to version.

Additionally, I have a TimeSeries class that I'm using in my work-in-progress update to the Supply Plugin. I could definitely keep it in the plugin, but I think it could be potentially useful for other plugins. Extracting time series data and then searching those time series seems to be a very common use case of this library.

What do you all think?

@StoicLoofah
Copy link
Collaborator

That could be helpful! Is your TimeSeries in a public branch somewhere that you can link?

@danfulton
Copy link
Contributor Author

danfulton commented Feb 24, 2022

Right now, I have it tucked away in the supply module, but you can see it here: https://github.com/danfulton/sc2reader/blob/supply_fix/sc2reader/engine/plugins/supply.py.
There is a lot of other stuff in here, so look for class TimeSeries(UserList): on line 33.

This is very much WIP, and I would still change a few things:

  1. Right now, when you search using non-frame time units the returned type is "lossy" in that it throws out frames and returns seconds or in-game time strings. I'd like to adjust that so that frames are always retained as the underlying "truth" and time unit requests just change repr.
  2. In combination with this, there should be some internal record of the "display" time unit, so the user can set the default unit for search/display or pass it in with individual search/display.
  3. Haven't implemented lt and gt methods for time slicing yet.
  4. If (2) is implemented, can drop the time format tf() function.

Just wanted to probe interest a little bit before cleaning all this stuff up.

@StoicLoofah
Copy link
Collaborator

Interesting approach!

I'll just say to leave this issue open for comment if anyone else comes across it and thinks it would be useful.

@danfulton
Copy link
Contributor Author

Yeah, I think there's not a lot of use, for me at least, if I can't get some traction on Terran and Protoss with my version of SupplyTracker, but maybe if I get that working, I can make a draft PR of what it might look like to separate it out.

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

No branches or pull requests

2 participants