Tiled Based Lighting not working up and left

I have an issue where my tile based lighting code is not functioning properly going up and left. It works properly going right and downward (see image), but only goes one tile to the left/up and doesn’t continue like it should. http://i.imgur.com/DAM1pg2.png

My code seems correct; I have placed print statements to check values (removed in sample) and it seems alright.

public void light(){
		
		
		for(int i = 0; i < limity; i++){
			for(int x = 0; x < limitx; x++){
				light[i][x] = 0; //darken to current time of day
			}
		}
		
		int ty = (p.desiredy/32)-16;
		int tx = (p.desiredx/32)-16;
		if(ty > limity){
			ty = limity - 1;
		}
		if(ty < 0){
			ty = 0;
		}
		if(tx > limitx){
			tx = limitx - 1;
		}
		if(tx < 0){
			tx = 0;
		}
		int my = ty + 32;
		int mx = tx + 32;
		if(my > limity){
			my = limity - 1;
		}
		if(my < 0){
			my = 0;
		}
		if(mx > limitx){
			mx = limitx - 1;
		}
		if(mx < 0){
			mx = 0;
		}
		
		for(int by = ty; by < my; by++){
			for(int bx = tx; bx < mx; bx++){//for each block in the area
				if(layer2[by][bx].lightlevel > 0){ //for each LIGHT SOURCE
					light[by][bx] = layer2[by][bx].lightlevel;
					
				}else light[by][bx] = 0;
			}
		}
		
		
		
		for(int by = ty; by < my; by++){
			for(int bx = tx; bx < mx; bx++){//for each block in the area, do adjacent check
				
				if(light[by][bx] > 1){
					
					//now do blocks adjacent
					//left, right, up, down
					//must do a check for blocks, otherwise exception
					//then check if block is DARKER than itself
					if(bx - 1 != -1){
						if(light[by][bx - 1] < light[by][bx]){
							
							light[by][bx - 1] = light[by][bx] - 1;
						}
					}
					if(by - 1 != -1){
						if(light[by - 1][bx] < light[by][bx]){
							light[by - 1][bx] = light[by][bx] - 1;
						}
					}
					
					if(bx + 1 != limitx){
						if(light[by][bx + 1] < light[by][bx]){
							light[by][bx + 1] = light[by][bx] - 1;
						}
					}
					
					if(by + 1 != limity){
						if(light[by + 1][bx] < light[by][bx]){
							light[by + 1][bx] = light[by][bx] - 1;
						}
					}
					
				}
			}
			//System.out.println("end of y loop, " + ty + " " + my);
		}
		
	}

Phew… Can’t read that code that well… I suggest to use recursive methods for that purpose, just how I did it with WorldOfCube.

So it looks like you have these resources:


private int[][] light;
private int[][] light2; // Wow... light values are stored here? Okey...

I don’t know what your [icode]int tx, ty[/icode] and [icode]int mx, my[/icode] are, so I’ll just skip them.
So lets go on building the recursive function. It’s gonna be readable much better :slight_smile:


public void lightSource(int x, int y, int lightValue) {
    if (light[x][y] >= lightValue && lightValue > 0) { // Well, the light has already been set here, we can stop here.
        return;
    }
    light[x][y] = lightValue;
    lightValue -= darkenAmmount; // need to implement yourself
    // recursiveness:
    lightSource(x-1, y, lightValue);
    lightSource(x+1, y, lightValue);
    lightSource(x, y-1, lightValue);
    lightSource(x, y+1, lightValue);
}
public void light() {
    // for each light source:
    lightSource(x, y, brightness);
}

Hope this helps :slight_smile:

Thanks! The very long code section was checking boundries for the world limit >.<

This really helps! Thanks! :smiley:

EDIT: sorry, layer2[][] is a block layer, and each block has a lightvalue of how much they give off.

I implemented the code and it only seems to make a ray of light going up. I’ll look at it again, and fix it :stuck_out_tongue:
EDIT: got a ton of errors of ArrayIndexOutOfBounds -1, and sometimes StackoverFlow errors.

@matheus23: You forgot to stop recursion when lightValue <= 0.

Also, my arrays go [Y][X], and not [X][Y]

Then you should probably make it go [X, Y] like everyone else to avoid confusion.

Works all fine and dandy, also fixed the lightValue <= 0 thing, but now it looks like this:

I also cut out the boundary checker here, but it’s still in the code (to reduce lines)


public void lightSource(int x, int y, int lightValue) {
		if(lightValue <= 0) return;

		if(x == -1 && x > limitx && y == -1 && y > limity){
			//System.out.println("out of bounds " + x + " " + y + " " + limitx + " " + limity);
			return;
		}
	    if ((light[y][x] >= lightValue) && lightValue > 0) { // Well, the light has already been set here, we can stop here.
	       // System.out.println("light already set");
	    	return;
	    }
	   
	    light[y][x] = lightValue;
	    lightValue -= 1; // need to implement yourself
	    // recursiveness:
	    
		
		if(x - 1 != -1){
			lightSource((x - 1), y, lightValue);
		}
		if(y - 1 != -1){
			lightSource(x, (y - 1), lightValue);
		}
		
		if(x + 1 != limitx){
			lightSource((x + 1), y, lightValue);
		}
		
		if(y + 1 != limity){
			lightSource(x, (y + 1), lightValue);
		}
	    
	}
	public void light() {
	    // for each light source:
		for(int i = 0; i < limity; i++){
			for(int x = 0; x < limitx; x++){
				light[i][x] = 0; //darken to current time of day
			}
		}
		
		
		
		for(int by = ty; by < my; by++){
			for(int bx = tx; bx < mx; bx++){//for each block in the area
				if(layer2[by][bx].lightlevel > 0){ //for each LIGHT SOURCE
					lightSource(bx, by, layer2[by][bx].lightlevel);
					
				}else light[by][bx] = 0;
			}
		}
	}

I’m guessing it has something to do with the return statements not processing the rest of the lighting.

This is just an optional question, but is there a way to make the framerate not so pathetic (used to be about 80 fps, now with lighting is 11 fps avg.)?

I have solved the issue. it might be a temporary one, but it works! I just put my original code after the recursive code, and it seems to work. I’ll work out a better solution later.

Argh… I wrote [icode]lightValue > 0[/icode] instead :confused: My mistake…


public void lightSource(int x, int y, int lightValue) {
    if (x < 0 || y < 0 || x >= limitx || y >= limity) {
        // out of bounds
        return;
    }
    // Fixed the > 0, which was wrong and supposed to be <= 0, as gravedev pointed out...
    if ((light[y][x] >= lightValue) && lightValue <= 0) {
        // Well, the light has already been set here, we can stop here.
        return;
    }
    
    light[y][x] = lightValue;
    lightValue -= 1;
    // recursiveness:
       
    // No need to check positions anymore: (I think)
    lightSource((x - 1), y, lightValue);
    lightSource(x, (y - 1), lightValue);
    lightSource((x + 1), y, lightValue);
    lightSource(x, (y + 1), lightValue);
}

Oh yeah… :smiley: I faced this issue myself A LOT. Well, it was pretty hard to get it working fast. I’ve used Threading and only processed visible chunks of my world. So yeah, it was possible to have the game itself run at about ~200fps and the lighting at ~80… but it was very, very hard. Try to optimize it somehow in another way. Threading will definitely help you, but is probably not so easy.