On Tue, Mar 12, 2019 at 11:02 PM Nick Renieris velocityra@gmail.com wrote:
I know a bunch of other code in the same file also does this, but please don't name variables "This". This may be one of those rare cases where "variable" would be a perfectly reasonable name, or perhaps simply "v".
"This" is indeed used everywhere. Can you explain what's different in my case?
"This" is used in the old code. We don't use "This" anymore in D3D code.
It would be great if you could write some tests to go along with this patch. Also, this patch implements 6 different functions; please split it so that each patch only makes a single conceptual change.
I have no issue with doing that, but is it _really_ necessary for something so simple? If so, can I split to 3 commits, grouping Set/Get functions together?
I think that 6 commits are preferred. It should result in a cleaner commit messages, i. e. "Implement ID3D10EffectScalarVariable::GetInt()."
Also, tests would be useful. Your current implementation might be incorrect. Is it legal to call SetFloat() on boolean variables?