Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(animation-manager): "avoid animation" is set incorrectly if fallback … #455

Merged
merged 2 commits into from
Nov 17, 2023

Conversation

perryrh0dan
Copy link

@perryrh0dan perryrh0dan commented Nov 17, 2023

…is enabled and the user has only one or less available animation

What's new

I had the issue that even with "Credits Anim" set to OFF. I saw from time to time the thank you animation. The problem was that the avoid_animation string was not set to NULL properly.

Now i am just looping over the animation list and count how many valid animations the user has. If this is less then 2 i clear the avoid_animation variable.

I am not quiet sure what this line is doing:
if(StorageAnimationList_size(animation_list) == dolphin_internal_size + 1 && !fallback) {

because for me it was checking if 27(animation list size) is equal to 3(dolphin internal size), so this branch was never reached in my case. But unfortunately I could not find out where dolphin_internal_size comes from.

This is my first MR ever to an open source project so i hope everything is fine so far.


For the reviewer

  • I've uploaded the firmware with this patch to a device and verified its functionality
  • I've confirmed the bug to be fixed / feature to be stable

…is enabled and the user has only one or less available animation
@Willy-JL
Copy link
Contributor

so someone else in the discord just had a similar issue, and it has a similar fix to what you proposed. still i will improve it a bit.

the check for if(StorageAnimationList_size(animation_list) == dolphin_internal_size + 1 && !fallback) { is supposed to fix this issue. basically, animation_list will contain the list of animations from the manifest, and the list of internal animations. so my thought was that, if the number of animations in animation_list is equal to dolphin_internal_size + 1 then there must be only 1 valid animation, so we should not try to avoid repeating this animation and instead only repeat this one. which works for asset packs with only 1 animation.

problem is that instead animation_list contains the available animations, not the valid animations, so we can get to a similar situation where only 1 animation is valid by locking out the other available animations via level and mood. so yes your fix here would do the trick, but can be improved a bit. ill do just that

@Willy-JL Willy-JL merged commit 362b204 into Flipper-XFW:dev Nov 17, 2023
3 checks passed
@Willy-JL
Copy link
Contributor

also maybe you should set your git config up, commit shows as empty author :D

@Willy-JL Willy-JL added the bug Something isn't working label Dec 2, 2023
@Willy-JL Willy-JL mentioned this pull request Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants