Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

How can I optimise this game level code?

OR

achieve the same outcome simpler?

It copies the level graphics but uses a scale parameter, If scale is 1 it copies a screen size segment, if 2 it copies a half screen size piece and scales it to the screen size etc.

 public void render(Graphics g){

    int test = (screenwidth/2)-((screenwidth/2)/scale); 
    int test2 = (screenheight/2)-((screenheight/2)/scale); 

    int scaledstartx=test;
    int scaledstarty=test2;
    int scaledwidth=screenwidth-test;
    int scaledheight=screenheight-test2;
    int locationx=(wherex)+halfpwidth-(screenwidth/2);
    int locationy=(wherey)+halfpheight-(screenheight/2);        

    g.drawImage(DrawnLevel,0,0,screenwidth,screenheight,
    scaledstartx+locationx,scaledstarty+locationy, scaledwidth+locationx,scaledheight+locationy, null );    
 }
share|improve this question
    
Can you provide a bit more code? Where the screenwidth, screenheight, wherex, wherey, halfpwidth and halfpheight variables are declared? Where else they are used? What is their intended purpose? How they are initialized? What is the DrawnLevel (its name looks like a class, but it is being used as a variable)? –  Victor Stafusa Nov 13 '13 at 1:01

1 Answer 1

up vote 1 down vote accepted

First, lets eliminate the test and test2 variables, which are uneeded (we already have scaledstartx and scaledstarty). Second keep screenwidth/2 and screenheight/2 in variables to avoid recalculating it over and over (the compiler will already do this, but doing manually the code will become clearer). Third, to increase the readability a bit, lets avoid doing calculations in the parameters.

So, this is your code now:

public void render(Graphics g) {

    int halfScreenWidth = screenwidth / 2;
    int halfScreenHeight = screenheight / 2;
    int scaledStartX = halfScreenWidth - (halfScreenWidth / scale); 
    int scaledStartY = halfScreenHeight - (halfScreenHeight / scale); 

    int scaledWidth = screenwidth - scaledStartX;
    int scaledHeight = screenheight - scaledStartY;
    int locationX = wherex + halfpwidth - halfScreenWidth;
    int locationY = wherey + halfpheight - halfScreenHeight;

    int spriteStartX = scaledStartX + locationX;
    int spriteStartY = scaledStartY + locationY;
    int spriteEndX = scaledWidth + locationX;
    int spriteEndY = scaledHeight + locationY;
    g.drawImage(DrawnLevel, 0, 0, screenwidth, screenheight, spriteStartX,
            spriteStartY, spriteEndX, spriteEndY, null);
 }

We can improve it by observing that the terms in the spriteblabla variables are decomposed in terms that have halfScreenWidth and halfScreenHeight both adding and subtracting. So, we can achieve a simpler expression. To do so, lets replace the variables in the assignment to the spriteblabla variables:

public void render(Graphics g) {

    int halfScreenWidth = screenwidth / 2;
    int halfScreenHeight = screenheight / 2;

    int spriteStartX = halfScreenWidth - (halfScreenWidth / scale) + wherex + halfpwidth - halfScreenWidth;
    int spriteStartY = halfScreenHeight - (halfScreenHeight / scale) + wherey + halfpheight - halfScreenHeight;
    int spriteEndX = screenwidth - (halfScreenWidth - (halfScreenWidth / scale)) + wherex + halfpwidth - halfScreenWidth;
    int spriteEndY = screenheight - (halfScreenHeight - (halfScreenHeight / scale)) + wherey + halfpheight - halfScreenHeight;

    g.drawImage(DrawnLevel, 0, 0, screenwidth, screenheight, spriteStartX,
            spriteStartY, spriteEndX, spriteEndY, null);
 }

Now, lets simplify that, since (halfScreenWidth - halfScreenWidth) and (screenwidth - halfScreenWidth - halfScreenWidth) are zero:

public void render(Graphics g) {

    int halfScreenWidth = screenwidth / 2;
    int halfScreenHeight = screenheight / 2;

    int spriteStartX = -(halfScreenWidth / scale) + wherex + halfpwidth;
    int spriteStartY = -(halfScreenHeight / scale) + wherey + halfpheight;
    int spriteEndX = (halfScreenWidth / scale) + wherex + halfpwidth;
    int spriteEndY = (halfScreenHeight / scale) + wherey + halfpheight;

    g.drawImage(DrawnLevel, 0, 0, screenwidth, screenheight, spriteStartX,
            spriteStartY, spriteEndX, spriteEndY, null);
 }

Now, moving some common subexpressions into variables:

public void render(Graphics g) {

    int halfScaledWidth = screenwidth / 2 / scale;
    int halfScaledHeight = screenheight / 2 / scale;
    int spriteCenterX = wherex + halfpwidth;
    int spriteCenterY = wherey + halfpheight;

    int spriteStartX = spriteCenterX - halfScaledWidth;
    int spriteStartY = spriteCenterY - halfScaledHeight;
    int spriteEndX = spriteCenterX + halfScaledWidth;
    int spriteEndY = spriteCenterY + halfScaledHeight;

    g.drawImage(DrawnLevel, 0, 0, screenwidth, screenheight, spriteStartX,
            spriteStartY, spriteEndX, spriteEndY, null);
 }
share|improve this answer
    
Should assert scale != 0; somewhere in the code. –  Dave Jarvis Nov 13 '13 at 17:40
1  
I am assuming that the given code was correct, so didn't thought about asserting. If that is not the case, we should as well assert screenwidth > 0;, assert screenheight > 0;, assert DrawnLevel != null; and assert g != null;. –  Victor Stafusa Nov 13 '13 at 17:47

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.