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

Request: Better name for VMA_? #23

Closed
SirJson opened this issue Sep 22, 2018 · 3 comments
Closed

Request: Better name for VMA_? #23

SirJson opened this issue Sep 22, 2018 · 3 comments

Comments

@SirJson
Copy link

SirJson commented Sep 22, 2018

The problem I see is for people who are writing bindings to your library and are new to the Project.

I don't think you can know what VMA means but you have to re-implement the macro that calls VMA_ in some way.

I actually also had to think for a moment what VMA means and I already wrote working bindings to the library. Unfortunatelly VMA_ doesn't make it better. With VM_ArgPtr it was quite clear what the function was doing.

If you want to keep the VMA_ I think it would be of great help for people writing bindings if you explain those three letters. I know there is already documentation for the macro but you can easily overlook it because you usually can't get bindings to them.

I would do it myself and send you a pull request but I'm not sure why you changed it to VMA_ since there is already a convenience macro with the same name. I would just go back to VM_ArgPtr because for me it was immediately clear what it does but maybe someone else got a better name?

Also doesn't break the pseudo namespacing that is happening in vm.c? Almost everything is prefixed with VM_

@jnz
Copy link
Owner

jnz commented Sep 22, 2018

I think you are right. Going back to VM_ArgPtr is also consistent with Quake3. I‘ll change it. Thanks for the feedback.

jnz added a commit that referenced this issue Sep 22, 2018
@jnz
Copy link
Owner

jnz commented Sep 22, 2018

Commit a18c027 goes back to the old name (from VMA_ to VM_ArgPtr)

@SirJson
Copy link
Author

SirJson commented Sep 22, 2018

Cool! I think you saved someone in the future from some headache. I will update my crate as soon as possible to reflect that change.

@SirJson SirJson closed this as completed Sep 22, 2018
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