yeah little things like that I have to avoid bringing up in PR reviews because it becomes a waste of
yeah little things like that I have to avoid bringing up in PR reviews because it becomes a waste of time unless it is egregious.
Mit is a waste of time but you have to fix it eventually anyway
Mi think you can set it up to fix automatically but i don't know how
Plike you can tell when beyley has been in a file when there’s no space after a //, you can tell when devvoid has been in a file because of that brace style (though I think they’re all gone now and I don’t mind telling beyley off for it given that most of their PRs are too perfect that I don’t want to merge them first try)
KI did it in like 10 minutes on the github web ui lol, i want the blunt feedback. will correct and go further in the morning after actually cloning it
Mquestion: why's BreakneckSleep a struct, especially a disposable one
Pperf
Pit’s a thin wrapper over a sleep primitive
Mis it repeatedly allocated
Pno if the user uses it properly
Mthen what perf benefit does it add to be a struct?
Palright you’ve got me there
Pbut this has no reason to be a class
Pgiven it is a thin wrapper over a sleep primitive
Mwell it's disposable and khitiara wants it to track state
Pyes and I’ve said to Not Do That
Pand structs can be disposable, see ComPtr
Myeah fair point
Kyeah which is fair and the question is now how to actually go about this
Mthey can be but since they have no lifetime tracking i hate doing so unless there's a really good reason to
Ktbh half of why i went and made this is i wanted something to progress with this branch
P- do the sleep
- figure out how much we’ve overshot the sleep and return it
- user passes it in for next time, overshoot is subtracted from the sleep duration
- figure out how much we’ve overshot the sleep and return it
- user passes it in for next time, overshoot is subtracted from the sleep duration
Mso you're tracking state via a ref... i do believe you've reinvented classes
Pbut we’re not tracking state?
Kok but how do we figure out how much weve overshot, because the issue that leads to this is that stuff outside Sleep() is also taking time
Kmy half implementation doesnt even compile since its a readonly struct lol
Myou have a variable that is passed to the method and then returned from the method to be passed again for the next call
Pahhhhh, good point
Pyeah, what part of that is us [BreakneckSleep] tracking state
Kan idea then:
- return GetTimeStamp() and take the last timestamp as a parameter
- return GetTimeStamp() and take the last timestamp as a parameter
Kinstead of passing around TimeSpans
Myou're moving that responsibility to the caller but by having that parameter at all you're getting most of the theoretical drawbacks of using a class
Mso i reckon a class is the occam's razor thing to do here
Pyes, with the breakneck types literally any and all responsibility falls on the user
Pthey’re designed to do whatever the user wants them to do and to do it without any overhead
Pand for this reason I encourage most users to stay away from them
Plike no one should use BreakneckLock it’s outright dangerous
Kthis is implementation detail type stuff generally
Mi'm not recommending a class to make it easier to use as much as to avoid overcomplicating the type's design
Pcan return the last timestamp, but I am hesitant to have the parameter be the timestamp as this implies the previous sleep and the next sleep are related which they may not be in every case.
Kif we did go that direction id suggest renaming to something like
LoopRateLimiter to not associate with Breakneck, or split that out into a seperate class and keep the native stuff seperate in BreakneckSleepKtrue but at that point why have the duration property at all?
P…fair point.
Kif youre disconnecting from a sequence of timestamps then this should take the sleep duration as a parameter and lose the inherent periodicity
Kand then do as Maxine suggests and have a wrapper type to handle the timestamp part or just do that in the window implementation
Mthat would get it a lot closer to what its name implies
Kid rather have the loop rate limiting aspect be separated so it can be used in other code too but theres no real reason to prefer that
Munrelated; the OS and architecture checks not being guarded in if-else if feels weird to me, is there a particular reason for that
Ldoes it matter
Mdoes what matter
PI guess the main thing is the use case of this breakneck type is quite complicated. It needs to sleep for 1/UpdatesPerSecond, 1/FramesPerSecond, or 1/MaxThrottle, and needs to be able to resume sleeping for the next event. e.g. if we resume for render but update is not due yet, then we just need to sleep for the remaining time until update is due or the max throttle (whichever is smaller)