[Feature] Storage API + Metrics API #113
Conversation
|
|
||
| import Foundation | ||
|
|
||
| extension Double { |
There was a problem hiding this comment.
I think this file should be in the extensions folder
There was a problem hiding this comment.
Locally, it is in the Extensions folder. I think this is a glitch with GitHub.
There was a problem hiding this comment.
is it like that in the Finder aka local machine not in xcode as well?
There was a problem hiding this comment.
Oh nice thanks for letting me know, that seems to be the issue. It's fixed now.
sp4ce-cowboy
changed the title
|
|
||
| if let statsSystem = target.system(ofType: StatisticSystem.self) { | ||
| if player != .ownPlayer { | ||
| statsSystem.notify(for: self) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Would it be better to somehow register the stats system notifier as a handler in the EventManager? So that we select which stats we want to get notified for via the StatsEngine rather than being controlled via the event? I believe this Prof will feel that this current method is kind of hard coding.
There was a problem hiding this comment.
Thanks for noticing this, I forgot to remove the player check here. I've implemented something similar which removes the hard coding:
let eventUpdateClosure: (Statistic, DamageEvent?) -> Void = { statistic, event in
guard let event = event, event.player != .ownPlayer else {
return
}
statistic.updateCurrentValue(by: Double(event.damage))
Logger.log("Updating statistic with event detail: \(String(describing: event))", self)
}This code snippet is from the TotalDamageDealtStatistic, the StatisticUpdateActor (i.e. eventUpdateClosure) related to this Statistic now takes in an Event as opposed to just EventType from the prior implementation (which had the check inside the event itself) and the parsing is done by the StatsEngine when it calls to execute the above closure.
| /*func getStatisticUpdateLinks() -> StatisticUpdateLinkDatabase { | ||
| let eventType = TFEventTypeWrapper(type: DamageEvent.self) | ||
| let updateActor: StatisticUpdateActor = { statistic in statistic.updateCurrentValue(by: ) } | ||
| let eventUpdateDictionary = [eventType: updateActor] | ||
| let statsLink = StatisticUpdateLinkDatabase(statisticUpdateLinks: eventUpdateDictionary) | ||
|
|
||
| return statsLink | ||
| }*/ |
There was a problem hiding this comment.
Fixed, thanks.
Summary
This PR adds the entire Metrics API + Storage API. Added both together given the close link between them (i.e. Storage is needed to see if the Metrics work properly).
MetricsAPI
StorageAPI
Pending & Concerns
PR Status
Ready for review. UIView for Achievements and missions to be move to another PR. Some commented code is retained just in case the newer StatisticsUpdateActors fail and we need to revert back (which we can only tell after some more testing)
Closes #52