Cleanup animationinfo #7707
Cleanup animationinfo #7707
Conversation
| @@ -82,6 +82,8 @@ class AnimationInfo { | |||
| */ | |||
| [[nodiscard]] uint8_t getAnimationProgress() const; | |||
|
|
|||
| void configureFrameDistribution(AnimationDistributionFlags flags, int8_t numberOfFrames, int8_t distributeFramesBeforeFrame, int8_t numSkippedFrames, int8_t ticksPerFrame, uint8_t previewShownGameTickFragments); | |||
There was a problem hiding this comment.
Could this be a private method?
| @@ -27,9 +27,8 @@ int8_t AnimationInfo::getFrameToUseForRendering() const | |||
| if (currentFrame >= relevantFramesForDistributing_) | |||
| return currentFrame; | |||
|
|
|||
| int16_t ticksSinceSequenceStarted = ticksSinceSequenceStarted_; | |||
| int16_t ticksSinceSequenceStarted = std::max<int16_t>(0, ticksSinceSequenceStarted_); | |||
There was a problem hiding this comment.
Personally, I'd leave it as it is, as the log and the local variable are less confusing that way.
For clarity we could also change the if from ticksSinceSequenceStarted_ to ticksSinceSequenceStarted.
But maybe that is just personal taste 😉.
| @@ -82,6 +82,8 @@ class AnimationInfo { | |||
| */ | |||
| [[nodiscard]] uint8_t getAnimationProgress() const; | |||
|
|
|||
| void configureFrameDistribution(AnimationDistributionFlags flags, int8_t numberOfFrames, int8_t distributeFramesBeforeFrame, int8_t numSkippedFrames, int8_t ticksPerFrame, uint8_t previewShownGameTickFragments); | |||
There was a problem hiding this comment.
I'm still not sure if this is a good move. On the one hand the function is smaller. On the other hand, the new function has only 21 lines less than setNewAnimation and now we have another function with many parameters. 🤷
Maybe we could remove some of the parameters. If we use the ticksPerFrame member (assigned in setNewAnimation) instead of passing it as a variable. The same for numberOfFrames.
We could also replace numSkippedFrames with currentFrame, but that might be going a bit too far.
There was a problem hiding this comment.
You may be right. I was too concerned with salvaging the potentially useful parts of the original PR, but I didn't take enough time to consider the best way to approach cleaning up this file. I think I may just close it instead.
| @@ -82,6 +82,8 @@ class AnimationInfo { | |||
| */ | |||
| [[nodiscard]] uint8_t getAnimationProgress() const; | |||
|
|
|||
| void configureFrameDistribution(AnimationDistributionFlags flags, int8_t numberOfFrames, int8_t distributeFramesBeforeFrame, int8_t numSkippedFrames, int8_t ticksPerFrame, uint8_t previewShownGameTickFragments); | |||
There was a problem hiding this comment.
Should this function include documentation, e.g. `@brief'?
Improved version of: #7550
setNewAnimationinto a new member function ofAnimationInfoclass to reduce function size with no logic changes.