ggtracker/sc2reader

Add time handlers and TimeSeries object

danfulton opened this issue · 4 comments

danfulton commented

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 commented

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

danfulton commented

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 commented

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 commented

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.