Skip to content

Conversation

@fgc
Copy link
Contributor

@fgc fgc commented May 16, 2025

Ok, this one is hairy:

  • It seems to work, only tried one real race, the good news end here.

  • 100% vibe coded, handle with care.

  • I don´t like the vibes of some of the things that it did in src\frontend\components\Standings\hooks\useDriverStandings.tsx but don´t understand the code well enough to revert them, might be a good ask the LLM to justify it or clean it up

  • I had very bad performance problems in the second race I did with this. I checked and the app was taking 10% CPU, which seems high to me. No more problems after removing it. Not sure how often this is calculated, ideally it would be when positions change

  • Numbers were accurate for the one race I tested, but should test more.

  • There's some weirdness with the concept of starter and non-starter drivers, not sure what´s that about

@tariknz
Copy link
Owner

tariknz commented May 18, 2025

I'll need to take a look a closer at this.

re non-starters basically there's a bigger penalty if you register and don't start so there's a bigger loss for those players which gets distributed to the rest of the grid. How we decide who is a non-starter is the tricky part, its likely something with taking the qualifying driver list and compare them with the ones not on track in the race? something like that maybe. It all depends on how accurate you want it in the end.

re the performance issues, I'm pretty sure its likely to do with functions getting recreated on every render and reexecuting. It really should only recalculate during position changes.

@fgc
Copy link
Contributor Author

fgc commented May 19, 2025

Yeah this needs lots of work before even thinking about merging.

I've been looking a bit into where would be best to limit the calculation of ratings only to position changes, but I'm not really even sure about everything that triggers an update of the overlay now. I know generally how React works but not well enough to tell with confidence from the code, as this one gets data from many places.

Is there a set policy for when to update of Standings overlay now? My own preference when I watch races is when they update on position changes and when people complete laps. When it's only completed laps I find it a bit confusing

PS1. Re: the performance problems, I'm starting to think it might more general than just this, as in with or without this changes, the app is a bit CPU hungry and there might be some low hanging fruit to pick in this matter. I tried to spawn the overlays with the dev tools open and get a look into it, but ran out of time.

PS2. I couldn't race yesterday, but I spectated a couple races with the overlay on, and the iR results matched what was shown in the racelabs of one of the drivers that was on Twitch, that's encouraging.

PS3. From that a note for a little QoL enhancement: using the Standings overlay as spectator only showed the top 3 drivers, I quickly hacked the slicing code to show everyone if my position was -1, might be something to look into to set a better default. Not sure about the -1 position holding if there are more spectators, or if you get -2 or -3 if there were more before you entered.

@tariknz
Copy link
Owner

tariknz commented May 21, 2025

Is there a set policy for when to update of Standings overlay now? My own preference when I watch races is when they update on position changes and when people complete laps. When it's only completed laps I find it a bit confusing

Yep I would like to update it to the live positions, just haven't got around to it yet.

PS3. From that a note for a little QoL enhancement: using the Standings overlay as spectator only showed the top 3 drivers, I quickly hacked the slicing code to show everyone if my position was -1, might be something to look into to set a better default. Not sure about the -1 position holding if there are more spectators, or if you get -2 or -3 if there were more before you entered.

Yeah for sure this hasn't had a lot of TLC since its initial implementation, definitely never tried it while spectating so that would be a great change for sure. I think I just need to just show at least top 10 or 15 (we can make it configurable) if the driver isn't on track at all.

@tariknz
Copy link
Owner

tariknz commented May 21, 2025

On performance issues you are right, I have definitely noticed it is CPU hungry I just haven't looked too hard at it. Wanted to get to a good place with features then go back and refine. I suspect right now its possibly because we publish ALL the data to every overlay wether they use it or not, even though its not rerendering or anything like that its just probably still sitting in the background accepting data in memory then reducing it to what it needs.

So one place I'd like to tackle is the IPC piece where electron is publishing data we probably need to pass in a query or something like that instead. But yeah requires a massive refactor.

@fgc
Copy link
Contributor Author

fgc commented May 21, 2025

About the performance, I started looking into things like "throttling" and "debouncing", envisioning how for example something like the weather overlay could well live with very infrequent updates, while the inputs overlay needs them all. But I've been a bit under the weather these last few days and have had a hard time breaking through some brain fog.
While I initally blamed the addition of the iR calculations for my perf troubles, I've logged them and they don´t run on every telemetry frame as I feared.

@tariknz
Copy link
Owner

tariknz commented May 23, 2025

I've just added some tidy ups I hope that's all good, main thing is using icons instead of the unicodes and make sure we only show it for race sessions:

image

@fgc
Copy link
Contributor Author

fgc commented May 23, 2025

Looks great and tidy!

It will be a while until I can do much about this. As I said I was somewhat ill (hay fever/allergies) but today I had a bad asthma bout and actually fainted, hit my tail bone pretty hard in the fall, so now I can't even sit at the computer... :(

@tariknz
Copy link
Owner

tariknz commented May 23, 2025

Looks great and tidy!

It will be a while until I can do much about this. As I said I was somewhat ill (hay fever/allergies) but today I had a bad asthma bout and actually fainted, hit my tail bone pretty hard in the fall, so now I can't even sit at the computer... :(

Oh no problem at all mate I will merge it as is and we'll tweak and improve. I don't see any perf issue related to this I'm pretty sure this is fine. What I'll do is just add the settings once this is merged to switch on/off and we should be good. Appreciate it though looks great! Rest up chat soon 🙏

@tariknz tariknz merged commit 3767979 into tariknz:main May 24, 2025
1 check 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.

2 participants