4
\$\begingroup\$

I'm building a "sliding puzzle" game. This is the animation code for when you click a Tile.

This "loop" function is running all the time.

When you click a tile, if it's available to move, I set the "isAnimating" property to true and then set the target left position, and the step (calculated with the starting and end position). The step is just an integer (negative or positive, depending on if the movement is to the left or to the right).

procedure TForm3.Loop(sender: tobject);
var
  i : integer;
begin
  for I := 0 to Tiles.Count - 1 do
  begin
    if(tiles[i].isAnimating) then
    begin
      if(tiles[i].animateProperty = 'left') then
      begin
        if(tiles[i].step > 0) then
        begin
          if tiles[i].left < tiles[i].animateTo - tiles[i].step then
          begin
            tiles[i].Left := tiles[i].Left + tiles[i].step;
          end else begin
            emptyTile.Left := tempX;
            emptyTile.Visible := true;
            tiles[i].isAnimating := false;
            tiles[i].Left := tiles[i].animateTo;
          end;
        end else begin
          if tiles[i].left > tiles[i].animateTo - tiles[i].step then
          begin
            tiles[i].Left := tiles[i].Left + tiles[i].step;
          end else begin
            emptyTile.Left := tempX;
            emptyTile.Visible := true;
            tiles[i].isAnimating := false;
            tiles[i].Left := tiles[i].animateTo;
          end;
        end;
      end else begin
        if(tiles[i].step > 0) then
        begin
          if tiles[i].top < tiles[i].animateTo - tiles[i].step then
          begin
            tiles[i].top := tiles[i].top + tiles[i].step;
          end else begin
            emptyTile.top := tempY;
            emptyTile.Visible := true;
            tiles[i].isAnimating := false;
            tiles[i].top := tiles[i].animateTo;
          end;
        end else begin
          if tiles[i].top > tiles[i].animateTo - tiles[i].step then
          begin
            tiles[i].top := tiles[i].top + tiles[i].step;
          end else begin
            emptyTile.top := tempy;
            emptyTile.Visible := true;
            tiles[i].isAnimating := false;
            tiles[i].top := tiles[i].animateTo;
          end;
        end;
      end;
    end;
  end;
end;

This is how it works:

  1. Go through each tile, and check if the isAnimating property is true;

  2. If it is, check if the animation is a left or top one.

  3. If it's left, check if the step is greater than 0 (move to the right) or less than 0 (move to the left).

  4. If it's less than 0, check if the current left position is lower than the target (animateTo) position.

  5. If it is, then keep animating, if not, then stop the animation (and do a couple more things.

The problem is there's a lot of repeating code but I can't seem to find a way to refactor it so that I avoid repeating myself.

Of course I could use procedures for the same actions but I feel like there's a better way to do it than the way I do it (the animation).

\$\endgroup\$
  • \$\begingroup\$ Consdider using a local procedure definition with an appropriate callable function variable. \$\endgroup\$ – πάντα ῥεῖ May 20 at 21:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.