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

Minor inconsistency of parameters in "Get/Set...ForShape" methods of NifFile class #30

Closed
gavrant opened this issue Mar 6, 2022 · 4 comments

Comments

@gavrant
Copy link

gavrant commented Mar 6, 2022

Almost all "Get/Set...ForShape" methods of NifFile class identify the shape by its pointer:

const std::vector<Vector3>* GetVertsForShape(NiShape* shape);
const std::vector<Vector3>* GetNormalsForShape(NiShape* shape);
and so on

But a couple of "Colors" methods go with the name of the shape instead:

const std::vector<Color4>* GetColorsForShape(const std::string& shapeName);
void SetColorsForShape(const std::string& shapeName, const std::vector<Color4>& colors);

And this is a bit confusing. I would suggest adding NiShape* shape overloads of these two for consistency and predictability.

@ousnius
Copy link
Owner

ousnius commented Mar 6, 2022

@gavrant There already is a function bool GetColorsForShape(NiShape* shape, std::vector<Color4>& outColors) const;

EDIT: It seems only SetColorsForShape is missing a pointer version.

@gavrant
Copy link
Author

gavrant commented Mar 6, 2022

@ousnius Unfortunately for me, my code is a derivative of PyNifly, and PyNifly does this:
const std::vector<Color4>* theColors = nif->GetColorsForShape(shape->name.get());
That is, uses the last remaining "named" method that got no pointer version with your commit. I guess I still need to rewrite that code to drop that rather shady shape->name.get() (especially with a .nif file with empty shape names I saw today).

@ousnius
Copy link
Owner

ousnius commented Mar 6, 2022

@ousnius Unfortunately for me, my code is a derivative of PyNifly, and PyNifly does this: const std::vector<Color4>* theColors = nif->GetColorsForShape(shape->name.get()); That is, uses the last remaining "named" method that got no pointer version with your commit. I guess I still need to rewrite that code to drop that rather shady shape->name.get() (especially with a .nif file with empty shape names I saw today).

nifly has all the functions you need (it did already for "Get", and does now for "Set").
You or someone else needs to update PyNifly to use the pointer versions as well.

@gavrant
Copy link
Author

gavrant commented Mar 6, 2022

Yes, I know about the other version of GetColorsForShape that accepts a pointer instead of a string. It's just my inner perfectionist complaining about the sole remaining 'named" Get..ForShape in that block of code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants