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.

I'm implementing game with classical screen wraparound effect. It is very simple: if the player goes out of screen, it reappears on the opposite side (Asteroids is a good example).

float x = getX();
float y = getY();

float screenW = screen.getWidth();
float screenH = screen.getHeight();

if (x > screenW)
    x = 0;
else if (x < 0)
    x = screenW;

if (y > screenH)
    y = 0;
else if (y < 0)
    y = screenH;

setPosition(x,y);

How can I improve this code to make it cleaner? Those multiple similar if statements especially look ugly to me.

share|improve this question
    
Are you sure that floats are appropriate? Screen size would typically be measured in pixels, which is integral. –  200_success Jul 25 at 20:34
1  
@200_success Many game engines use floats instead of ints to represent screen coordinates, because of, for example, Subpixel rendering –  Kao Jul 25 at 21:22
    
Be aware that for objects that have a size greater than one pixel it might be necessary to draw them on both sides of the screen when they pass the border. This avoids a "popping" effect when it vanishes on one side and appears on the other. –  Nobody Jul 26 at 12:49

2 Answers 2

up vote 11 down vote accepted

This situation calls for a modulus operator.

The modulus operator returns the remainder from a division. So 5 % 2 = 1, 10 % 15 = 10 and so on. This is because 5 goes into 2 twice, with remainder 1. This gets a bit tricky with negative numbers, as -5 % 2 could either be -1 or 1, since -5 = -4 + -1 or -6 + 1. However, the Java language specification specifies that it will match the sign of the dividend, making -5 % 2 be -1. However, if we add the divisor, 2, we get the positive result.

Try this:

x %= screenW + 1; // the + 1 is so you get numbers from 0 to screenW
y %= screenH + 1;
/* Below is correction for negative numbers
 * (as -10 % 3 = -1 in java, whereas you want the result to be 2)
 */
if (x < 0)
    x += screenW;
if (y < 0)
    y += screenH;

You can also wrap this in a function:

private float wrapAround(float coordinate, float max) {
    coordinate %= max + 1;
    if (coordinate < 0)
        coordinate += max;
    return coordinate;

Or if you prefer a ternary conditional:

private float wrapAround(float coordinate, float max) {
    coordinate %= max + 1;
    return (coordinate < 0) ? coordinate + max : coordinate;

Then your code is:

x = wrapAround(x, screenW);
y = wrapAround(y, screenH); 
share|improve this answer
1  
You could add screenW and screenH to x and y before the mod, and then you won't ever have a negative to worry about. This gives two adds, two mods, instead of two mods, two compares, and up to two adds. –  phyrfox Jul 25 at 16:57
1  
@phyrfox That wouldn't work in the general case. Try -100 % 3 for example. -100 + 3 = -97, so you still end up with a negative number. Unless you can ensure the all of the objects are moving slowly, then your trick would indeed work. –  mleyfman Jul 25 at 17:03
    
You're assuming that velocity can be higher than screenW, which would make the player appear to travel very slowly. Velocity should likely be capped at no more than screenW/10, depending on the frames per second. x and y should never become so negative that adding screenW and screenY can't be brought positive by simple addition, or the game would be unplayable. –  phyrfox Jul 25 at 17:24
    
If the game has laser or other light based weapons, I could foresee such circumstances possibly occurring. Also, keep in mind there may be lag elsewhere which could cause such circumstances. True it is very rarely that you would see this, but making code more robust at the cost of 2 relatively cheap operations (compares are probably much faster than mods) is a cost that I would be willing to accept. Also, don't forget that the bottlenecks will likely be elsewhere (drawing for instance), so your optimization is likely to be a case of premature optimization. –  mleyfman Jul 25 at 17:35
    
@mleyfman @phyrfox ((x % sw) + sw) % sw is always positive no matter what the original value of x. –  Darkhogg Jul 26 at 9:26

The current code doesn't wrap around in the "traditional" sense, for example:

  • for screenW + 1, it gives 0
  • for screenW + 2, it gives 0 (instead of 1)
  • for screenW + 3, it gives 0 (instead of 2)
  • ...
  • for -1 it gives screenW
  • for -2 it gives screenW (instead of screenW - 1)
  • for -3 it gives screenW (instead of screenW - 2)

If this is the intended behavior then there's no escape from those ifs.

Either way, you should extract the logic of wrapping around to a function rather than repeating it twice:

private float wrapAroundSharply(float coordinate, float max) {
    if (coordinate < 0) {
        return max;
    }
    if (coordinate > max) {
        return 0;
    }
    return coordinate;
}
share|improve this answer
1  
For the wrapAroundSharply method, it can be a one-liner with a nested ternary conditional: return coordinate < 0 ? max : coordinate > max ? 0 : coordinate; However note that this is just for fun. DO NOT write these sorts of things in any code that anyone else will read. –  mleyfman Jul 25 at 20:07
    
According to question whether this algorithm is intentional or not - yes it is. It works good enough for developed game :) –  Kao Jul 25 at 21:41
    
@janos i've ran your non-comparison method, and it is slower than my methods (admittedly, by a small, but significant margin), as well as wrong. It should be `((coordinate % width) + max) % width' –  mleyfman Jul 26 at 19:46
    
I tested too yesterday, but maybe I made a mistake. Thanks for pointing out, I deleted that point, I wouldn't recommend it anyway. –  janos Jul 26 at 19:58

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.