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

Default name assignment to animations without names #413

Merged
merged 5 commits into from
Oct 26, 2022

Conversation

onurtore
Copy link
Contributor

@onurtore onurtore commented Aug 7, 2022

🦟 Bug fix

Related to #393

Summary

Current implementation for the assimp loader does not check the names of the animations loaded through assimp. This PR adds default names for the animations without names. This approach is similar to how ColladaLoader names the animations.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

Assings names to animations without names

Signed-off-by: Onur Berk Tore <[email protected]>
Signed-off-by: Onur Berk Tore <[email protected]>
@codecov
Copy link

codecov bot commented Aug 7, 2022

Codecov Report

Merging #413 (60819b8) into main (cfff058) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #413   +/-   ##
=======================================
  Coverage   81.06%   81.07%           
=======================================
  Files          79       79           
  Lines        9675     9679    +4     
=======================================
+ Hits         7843     7847    +4     
  Misses       1832     1832           
Impacted Files Coverage Δ
graphics/src/AssimpLoader.cc 89.17% <100.00%> (+0.16%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@chapulina chapulina added the bug Something isn't working label Aug 8, 2022
@mjcarroll
Copy link
Contributor

The macOS warning here is due to an ffmpeg update and can be ignored.

@onurtore
Copy link
Contributor Author

onurtore commented Aug 18, 2022

@ahcorde,
What I can do for CI/CD, it looks failure reason doesnt comes from my code?

@mjcarroll
Copy link
Contributor

What I can do for CI/CD, it looks failure reason doesnt comes from my code?

I agree, the GDAL errors are not because of this PR. With approval, we can go ahead and merge here.

@mjcarroll
Copy link
Contributor

Is the intention to still to target this to main, or should it target gz-common5 ?

@onurtore
Copy link
Contributor Author

onurtore commented Oct 6, 2022

Is the intention to still to target this to main, or should it target gz-common5 ?

I have no preference.

@mjcarroll mjcarroll changed the base branch from main to gz-common5 October 26, 2022 14:13
@mjcarroll mjcarroll changed the base branch from gz-common5 to main October 26, 2022 14:14
@mjcarroll
Copy link
Contributor

ABI checker likely to fail until there is some release from main, so this is good to go.

@mjcarroll mjcarroll merged commit 9ece5b5 into gazebosim:main Oct 26, 2022
@iche033 iche033 mentioned this pull request Sep 16, 2024
8 tasks
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.

4 participants