Add WinnerIndex to OverallType #25
Conversation
src/stats/overall.ts
Outdated
| @@ -67,6 +69,23 @@ export function generateOverallStats( | |||
| return overall; | |||
| } | |||
|
|
|||
| function determineWinner(playerStocks: StockType[], opponentStocks: StockType[]) : number { | |||
|
|
|||
| if (playerStocks.length == 0 && opponentStocks.length == 0) return -1; //unknown winner; | |||
There was a problem hiding this comment.
can we avoid returning undefined by taking out the if statement and moving the return -1 to the end of the function, will avoid weird edge cases or missing data errors later.
|
Updated this. Still need tests and some cleanup. Different approach, need some feeback. |
|
For starters, I'm not really sure I agree with having the winner index as a part of the overall stats object. Since it's not really a stat. But some comments on the code itself: I think it would be better to export a helper function that gets imported instead of having the winner determination code in the The key issue with adding the winner (I assume for the purpose of displaying in the desktop app) is that it requires processing the entire file for all the frames |
|
@vinceau Solid feedback. A Is this worth implementing at the moment? Should we hold off? |
|
@a-sink-a-wait I think it would be best to hold off for the time being. Fizzi has talked about potentially adding caching of sorts and pagination which would allow processing entire SLP files to be less costly in the long run. But until then, even if this was to be merged you probably wouldn't see its use in the desktop app because the performance hit would be too costly. |
|
I know there's this PR as well that aims to improve performance of the desktop App. project-slippi/slippi-launcher#58 Would it be worth trying to combine these two implementations? |
It would be worth know what kind of performance we would be getting if combined. |
|
I'll take a look into this. |
NikhilNarayana
force-pushed
the
master
branch
2 times, most recently
from
0d29d48
to
e19718d
Compare
Mainly looking for feedback; am I on the right track here? Don't want to commit any more time if I am way off base.
I've implemented a
winnerIndexproperty from my understanding of just digging through the code. This is in an effort to display the winner in the replays folder of the desktop application.