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

new function "TextureTypeToString" should use the *ai...* namespace #4493

Closed
umlaeute opened this issue Apr 19, 2022 · 1 comment · Fixed by #4512
Closed

new function "TextureTypeToString" should use the *ai...* namespace #4493

umlaeute opened this issue Apr 19, 2022 · 1 comment · Fixed by #4512
Labels
Techdebt Technical dept which needs to get solved.

Comments

@umlaeute
Copy link
Contributor

umlaeute commented Apr 19, 2022

Describe the technical debt

All public C-functions exported by libassimp, use a prefix ai.
This is nice, so for the Debian package we even use a version-script to make sure that only proper "public" symbols can be found in the library (for whatever reasons, using -fvisibility=hidden seems to export too many symbols; but anyhow...)

Now, with assimp-5.2.3, there's a new exported symbol TextureTypeToString (which is even used by the assimp cmdline tool), which obviously lacks the ai-prefix.

I figure this is because the function used to be an often-used (and re-implemented) private function, that was turned into a public function for better re-usability.

See #2964 resp #2989

Better solution

Rename the function to aiTextureTypeToString to have a name consistent with other public C-symbols.

Desktop:

  • OS: Debian/GNU
  • Browser: wtf
  • Version 5.2.3
@umlaeute umlaeute added the Techdebt Technical dept which needs to get solved. label Apr 19, 2022
@umlaeute umlaeute changed the title new function "TextureTypeToString" does use the *ai...* namespace new function "TextureTypeToString" should use the *ai...* namespace Apr 19, 2022
@kimkulling
Copy link
Member

This is correct, good catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Techdebt Technical dept which needs to get solved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants