Topic Colored lights
This board is threaded (i.e. has a tree structure). Please use the Reply button of the specific post you are referring to, not just any random button. If you want to reply to the topic in general, use the Post button near the top and bottom of the page.
In Response to PeterW
Please don't use merge - instead squash & rebase and write a good overview commit message for the whole thing.
A few thoughts about the patch:
* LIGHT_DEBUG_COLOR seems to work very differently from LIGHT_DEBUG. A comment explaining the difference would be nice.
* You often put two spaces between type and name in variable declarations. Is that another style thing people have come up with?
* That "lightColorCoord" definition looks really funky. How about just "
* What is the "pink shade for debugging" about? Should that be LIGHT_DEBUG_COLOR?
* Could use better names than "C4DP_First" and "C4DP_Second". But that's probably something I should change afterwards.
* Also huh, the "shadow" naming really doesn't make sense, does it.
* You are declaring "alpha" in "DrawVertex" so it scopes over multiple "case" alternatives. That is a warning for multiple C++ compilers.
* SetColour: We have "GetRedValue" & co for extracting bytes from a RGB word
* The change to C4FoWLight::Update only consists of changing the curly brackets to a different style? You have lots of these. If you feel strongly about the code style, I would suggest making it a separate commit.
* A few commented-out "GL_SCISSOR_TEST" in "C4FoWRegion::BindFramebuf".
There's probably a few conflicts with my current work on the lights, but I'll take care of those.
A few thoughts about the patch:
* LIGHT_DEBUG_COLOR seems to work very differently from LIGHT_DEBUG. A comment explaining the difference would be nice.
* You often put two spaces between type and name in variable declarations. Is that another style thing people have come up with?
* That "lightColorCoord" definition looks really funky. How about just "
lightCoord.st - vec2(0.0,0.5)"?* What is the "pink shade for debugging" about? Should that be LIGHT_DEBUG_COLOR?
* Could use better names than "C4DP_First" and "C4DP_Second". But that's probably something I should change afterwards.
* Also huh, the "shadow" naming really doesn't make sense, does it.
* You are declaring "alpha" in "DrawVertex" so it scopes over multiple "case" alternatives. That is a warning for multiple C++ compilers.
* SetColour: We have "GetRedValue" & co for extracting bytes from a RGB word
* The change to C4FoWLight::Update only consists of changing the curly brackets to a different style? You have lots of these. If you feel strongly about the code style, I would suggest making it a separate commit.
* A few commented-out "GL_SCISSOR_TEST" in "C4FoWRegion::BindFramebuf".
There's probably a few conflicts with my current work on the lights, but I'll take care of those.
Powered by mwForum 2.29.7 © 1999-2015 Markus Wichitill