giawa / opengl4csharp

OpenGL 4 Bindings (partially based on OpenTK) for C#
Other
234 stars 61 forks source link

GenericVBO changes #25

Closed TheAIBot closed 5 years ago

TheAIBot commented 5 years ago

Part 1 of 3 from PR #24.

Add new GenericVBO constructor. Generic VAOs uses the new constructor. Moved IGenericVBO into the OpenGL namespace.

There is a few more changes i would like to make but i want to hear your opinion first. I want to move GenericVBO into the OpenGL namespace as well. If the user has to use it then it should probably be there.

I would also like to change the name of GenericVBO. The only difference between VBO and GenericVBO is that GenericVBO contains a name. So i would like to change the name of GenericVBO and IGenericVBO to something like NamedVBO and INamedVBO. Both VBO and GenericVBO are generic so having generic in the name isn't very descriptive. Afaik i don't think this change would break much as GenericVBO was only recently added.

giawa commented 5 years ago

Looks good, thanks! Since you've broken IGenericVBO into its own file I would suggest doing this with IGenericVAO, etc (if relevant to future PRs). I also noticed the Name property could use an auto property. Since we've been embracing auto properties I think it's worth getting that in there. What do you think?

giawa commented 5 years ago

After the #3 PR has been merged I'll go through and try to separate classes into their own files instead of having several class definitions in a single file. So no worries if any get missed during these PRs. It's easy to take care of later.