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'd like to know if I translated a piece of code correctly from C++ to Delphi. It looks like it is working, but I have a feeling that I'm reading and writing into memory that I'm not supposed to using Delphi.

Given C++ code:

struct tile_map
{
    int32 CountX;
    int32 CountY;

    uint32 *Tiles;
};

inline uint32
GetTileValueUnchecked(tile_map *TileMap, int32 TileX, int32 TileY)
{
    uint32 TileMapValue = TileMap->Tiles[TileY*TileMap->CountX + TileX];
    return(TileMapValue);
}

uint32 Tiles00[9][17] =
    {
        {1, 1, 1, 1,  1, 1, 1, 1,  0, 1, 1, 1,  1, 1, 1, 1, 1},
        {1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1},
        {1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1},
        {1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1},
        {0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1},
        {1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1},
        {1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1},
        {1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1},
        {1, 1, 1, 1,  1, 1, 1, 1,  1, 1, 1, 1,  1, 1, 1, 1, 1},
    };
// More tile map declarations ...   
// uint32 Tiles01[9][17] = ...
// uint32 Tiles10[9][17] = ...
// uint32 Tiles11[9][17] = ...  

    tile_map TileMaps[2][2];
    TileMaps[0][0].CountX = 17;
    TileMaps[0][0].CountY = 9;
    TileMaps[0][0].Tiles = (uint32 *)Tiles00;

    TileMaps[0][1] = TileMaps[0][0];
    TileMaps[0][1].Tiles = (uint32 *)Tiles01;

    TileMaps[1][0] = TileMaps[0][0];
    TileMaps[1][0].Tiles = (uint32 *)Tiles10;

    TileMaps[1][1] = TileMaps[0][0];
    TileMaps[1][1].Tiles = (uint32 *)Tiles11;

// Usage
    int32 PlayerTileX = 2;
    int32 PlayerTileY = 2;
    uint32 TileMapValue = GetTileValueUnchecked(&TileMap[1][1], PlayerTileX, PlayerTileY);

Delphi translation:

program Project1;

{$APPTYPE CONSOLE}

type
    Puint32 = ^uint32;

    tile_map = record
        CountX : int32;
        CountY : int32;

        Tiles : Puint32;
    end;
    Ptile_map = ^tile_map;

{$POINTERMATH ON}   
function GetTileValueUnchecked(TileMap : Ptile_map; TileX, TileY : int32) : uint32; inline;
begin
    result := TileMap^.Tiles[TileY * TileMap^.CountX + TileX];
end;


const //in the future these will be read from file, so const for now
    Tiles00:  array [0..8, 0..16] of uint32 =
    (
        (1, 1, 1, 1,  1, 1, 1, 1,  0, 1, 1, 1,  1, 1, 1, 1, 1),
        (1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (1, 1, 1, 1,  1, 1, 1, 1,  1, 1, 1, 1,  1, 1, 1, 1, 1)
    );
    // More tile map declarations ...
    //Tiles01:  array [0..8, 0..16] of uint32 = ...
    //Tiles10:  array [0..8, 0..16] of uint32 = ...
    //Tiles11:  array [0..8, 0..16] of uint32 = ...
var 
    TileMaps : array [0..1, 0..1] of  tile_map;
    PlayerTileX, PlayerTileY : int32;
    TileMapValue : uint32;
begin

    TileMaps[0][0].CountX := 17;
    TileMaps[0][0].CountY := 9;
    TileMaps[0][0].Tiles := Addr(Tiles00);

    TileMaps[0][1] := TileMaps[0][0];
    TileMaps[0][1].Tiles := Addr(Tiles01);

    TileMaps[1][0] := TileMaps[0][0];
    TileMaps[1][0].Tiles := Addr(Tiles10);

    TileMaps[1][1] := TileMaps[0][0];
    TileMaps[1][1].Tiles := Addr(Tiles11);

    // Usage
    PlayerTileX := 2;
    PlayerTileY := 2;
    TileMapValue = GetTileValueUnchecked(@TileMaps[1][1], PlayerTileX, PlayerTileY);
end.
share|improve this question

1 Answer 1

Everything there looks right (except for the = on the last line, which should be :=). It's about as close to a literal translation as you can get.

Converting the Delphi code to use dynamic arrays could ease your mind regarding out-of-bounds access, although the initial setup would be more cumbersome. (That might not be a problem if you're ultimately planning on loading the data from a file instead of from constants.) It would look something like this:

program Project1;

{$APPTYPE CONSOLE}

type
    tile_map = array of array of uint32;

function GetTileValueUnchecked(const TileMap: tile_map; TileX, TileY: int32): uint32; inline;
begin
    result := TileMap[TileY][TileX];
end;

const //in the future these will be read from file, so const for now
    Tiles00:  array [0..8, 0..16] of uint32 =
    (
        (1, 1, 1, 1,  1, 1, 1, 1,  0, 1, 1, 1,  1, 1, 1, 1, 1),
        (1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (1, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0,  0, 0, 0, 0, 1),
        (1, 1, 1, 1,  1, 1, 1, 1,  1, 1, 1, 1,  1, 1, 1, 1, 1)
    );
    // More tile map declarations ...
    //Tiles01:  array [0..8, 0..16] of uint32 = ...
    //Tiles10:  array [0..8, 0..16] of uint32 = ...
    //Tiles11:  array [0..8, 0..16] of uint32 = ...
var 
    TileMaps : array [0..1, 0..1] of  tile_map;
    PlayerTileX, PlayerTileY : int32;
    TileMapValue : uint32;
    x, y: Integer;
begin
    SetLength(TileMaps[0][0], Length(Tiles00), Length(Tiles00[0]));
    for y := 0 to High(Tiles00) do
      for x := 0 to High(Tiles00[0]) do
        TileMaps[0][0][y][x] := Tiles00[y][x];

    SetLength(TileMaps[0][1], Length(Tiles01), Length(Tiles01[0]));
    for y := 0 to High(Tiles01) do
      for x := 0 to High(Tiles01[0]) do
        TileMaps[0][1][y][x] := Tiles01[y][x];

    SetLength(TileMaps[1][0], Length(Tiles10), Length(Tiles10[0]));
    for y := 0 to High(Tiles10) do
      for x := 0 to High(Tiles10[0]) do
        TileMaps[1][0][y][x] := Tiles10[y][x];

    SetLength(TileMaps[1][1], Length(Tiles11), Length(Tiles11[0]));
    for y := 0 to High(Tiles11) do
      for x := 0 to High(Tiles11[0]) do
        TileMaps[1][1][y][x] := Tiles11[y][x];

    // Usage
    PlayerTileX := 2;
    PlayerTileY := 2;
    TileMapValue := GetTileValueUnchecked(TileMaps[1][1], PlayerTileX, PlayerTileY);
end.

Test with that code for a little while to convince yourself that the array accesses are right. If they're not, Delphi's range checking will alert you. Once you're confident, you can disable range checking inside GetTileValueUnchecked so that it's true to its name:

function GetTileValueUnchecked(const TileMap: tile_map; TileX, TileY: int32): uint32; inline;
begin
    {$R-}
    result := TileMap[TileY][TileX];
    {$R+}
end;
share|improve this answer
    
Thanks for the tip about the range checking. Will it improve performance if switched off? –  Stefan Badenhorst Jul 23 at 5:42

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.