Review rlights helper module #1489

2 files changed, 34 insertions, 33 deletions

diff --git a/examples/models/rlights.h b/examples/models/rlights.h
index 19504473..66185bb4 100644
--- a/examples/models/rlights.h
+++ b/examples/models/rlights.h
@@ -11,7 +11,7 @@
 *
 *   LICENSE: zlib/libpng
 *
-*   Copyright (c) 2017 Victor Fisac and Ramon Santamaria
+*   Copyright (c) 2017-2020 Victor Fisac (@victorfisac) and Ramon Santamaria (@raysan5)
 *
 *   This software is provided "as-is", without any express or implied warranty. In no event
 *   will the authors be held liable for any damages arising from the use of this software.
@@ -36,24 +36,21 @@
 //----------------------------------------------------------------------------------
 // Defines and Macros
 //----------------------------------------------------------------------------------
-#define         MAX_LIGHTS            4         // Max lights supported by shader
-#define         LIGHT_DISTANCE        3.5f      // Light distance from world center
-#define         LIGHT_HEIGHT          1.0f      // Light height position
+#define         MAX_LIGHTS            4         // Max dynamic lights supported by shader
 
 //----------------------------------------------------------------------------------
 // Types and Structures Definition
 //----------------------------------------------------------------------------------
-typedef enum {
-    LIGHT_DIRECTIONAL,
-    LIGHT_POINT
-} LightType;
 
-typedef struct {
-    bool enabled;
-    LightType type;
+// Light data
+typedef struct {   
+    int type;
     Vector3 position;
     Vector3 target;
     Color color;
+    bool enabled;
+    
+    // Shader locations
     int enabledLoc;
     int typeLoc;
     int posLoc;
@@ -61,20 +58,21 @@ typedef struct {
     int colorLoc;
 } Light;
 
+// Light type
+typedef enum {
+    LIGHT_DIRECTIONAL,
+    LIGHT_POINT
+} LightType;
+
 #ifdef __cplusplus
 extern "C" {            // Prevents name mangling of functions
 #endif
 
 //----------------------------------------------------------------------------------
-// Global Variables Definition
-//----------------------------------------------------------------------------------
-int lightsCount = 0;                     // Current amount of created lights
-
-//----------------------------------------------------------------------------------
 // Module Functions Declaration
 //----------------------------------------------------------------------------------
-Light CreateLight(int type, Vector3 pos, Vector3 targ, Color color, Shader shader);         // Defines a light and get locations from PBR shader
-void UpdateLightValues(Shader shader, Light light);                                         // Send to PBR shader light values
+Light CreateLight(int type, Vector3 position, Vector3 target, Color color, Shader shader);   // Create a light and get shader locations
+void UpdateLightValues(Shader shader, Light light);         // Send light properties to shader
 
 #ifdef __cplusplus
 }
@@ -106,7 +104,7 @@ void UpdateLightValues(Shader shader, Light light);
 //----------------------------------------------------------------------------------
 // Global Variables Definition
 //----------------------------------------------------------------------------------
-// ...
+static int lightsCount = 0;    // Current amount of created lights
 
 //----------------------------------------------------------------------------------
 // Module specific Functions Declaration
@@ -117,8 +115,8 @@ void UpdateLightValues(Shader shader, Light light);
 // Module Functions Definition
 //----------------------------------------------------------------------------------
 
-// Defines a light and get locations from PBR shader
-Light CreateLight(int type, Vector3 pos, Vector3 targ, Color color, Shader shader)
+// Create a light and get shader locations
+Light CreateLight(int type, Vector3 position, Vector3 target, Color color, Shader shader)
 {
     Light light = { 0 };
 
@@ -126,15 +124,20 @@ Light CreateLight(int type, Vector3 pos, Vector3 targ, Color color, Shader shade
     {
         light.enabled = true;
         light.type = type;
-        light.position = pos;
-        light.target = targ;
+        light.position = position;
+        light.target = target;
         light.color = color;
 
+        // TODO: Below code doesn't look good to me, 
+        // it assumes a specific shader naming and structure
+        // Probably this implementation could be improved
         char enabledName[32] = "lights[x].enabled\0";
         char typeName[32] = "lights[x].type\0";
         char posName[32] = "lights[x].position\0";
         char targetName[32] = "lights[x].target\0";
         char colorName[32] = "lights[x].color\0";
+        
+        // Set location name [x] depending on lights count
         enabledName[7] = '0' + lightsCount;
         typeName[7] = '0' + lightsCount;
         posName[7] = '0' + lightsCount;
@@ -148,13 +151,15 @@ Light CreateLight(int type, Vector3 pos, Vector3 targ, Color color, Shader shade
         light.colorLoc = GetShaderLocation(shader, colorName);
 
         UpdateLightValues(shader, light);
+        
         lightsCount++;
     }
 
     return light;
 }
 
-// Send to PBR shader light values
+// Send light properties to shader
+// NOTE: Light shader locations should be available 
 void UpdateLightValues(Shader shader, Light light)
 {
     // Send to shader light enabled state and type
@@ -170,8 +175,9 @@ void UpdateLightValues(Shader shader, Light light)
     SetShaderValue(shader, light.targetLoc, target, UNIFORM_VEC3);
 
     // Send to shader light color values
-    float diff[4] = { (float)light.color.r/(float)255, (float)light.color.g/(float)255, (float)light.color.b/(float)255, (float)light.color.a/(float)255 };
-    SetShaderValue(shader, light.colorLoc, diff, UNIFORM_VEC4);
+    float color[4] = { (float)light.color.r/(float)255, (float)light.color.g/(float)255, 
+                       (float)light.color.b/(float)255, (float)light.color.a/(float)255 };
+    SetShaderValue(shader, light.colorLoc, color, UNIFORM_VEC4);
 }
 
 #endif // RLIGHTS_IMPLEMENTATION
\ No newline at end of file
diff --git a/examples/shaders/rlights.h b/examples/shaders/rlights.h
index f9727c9e..66185bb4 100644
--- a/examples/shaders/rlights.h
+++ b/examples/shaders/rlights.h
@@ -11,7 +11,7 @@
 *
 *   LICENSE: zlib/libpng
 *
-*   Copyright (c) 2017-2019 Victor Fisac (@victorfisac) and Ramon Santamaria (@raysan5)
+*   Copyright (c) 2017-2020 Victor Fisac (@victorfisac) and Ramon Santamaria (@raysan5)
 *
 *   This software is provided "as-is", without any express or implied warranty. In no event
 *   will the authors be held liable for any damages arising from the use of this software.
@@ -69,11 +69,6 @@ extern "C" {            // Prevents name mangling of functions
 #endif
 
 //----------------------------------------------------------------------------------
-// Global Variables Definition
-//----------------------------------------------------------------------------------
-int lightsCount = 0;    // Current amount of created lights
-
-//----------------------------------------------------------------------------------
 // Module Functions Declaration
 //----------------------------------------------------------------------------------
 Light CreateLight(int type, Vector3 position, Vector3 target, Color color, Shader shader);   // Create a light and get shader locations
@@ -109,7 +104,7 @@ void UpdateLightValues(Shader shader, Light light);         // Send light proper
 //----------------------------------------------------------------------------------
 // Global Variables Definition
 //----------------------------------------------------------------------------------
-// ...
+static int lightsCount = 0;    // Current amount of created lights
 
 //----------------------------------------------------------------------------------
 // Module specific Functions Declaration