Skip to content

Commit

Permalink
Fix memory leaks and code quality in autotile code
Browse files Browse the repository at this point in the history
  • Loading branch information
mrkubax10 committed Aug 20, 2023
1 parent 1ffe32d commit fc5f1b5
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 109 deletions.
67 changes: 21 additions & 46 deletions src/supertux/autotile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.

#include <bitset>

#include "supertux/autotile.hpp"

//#include "supertux/autotile_parser.hpp"
#include <bitset>

#include "util/log.hpp"

// AutotileMask.

Expand All @@ -34,17 +34,11 @@ AutotileMask::matches(uint8_t mask, bool center) const
return mask == m_mask && center == m_center;
}

uint8_t
AutotileMask::get_mask() const
{
return m_mask;
}

// Autotile.

Autotile::Autotile(uint32_t tile_id, std::vector<std::pair<uint32_t, float>> alt_tiles, std::vector<AutotileMask*> masks, bool solid) :
Autotile::Autotile(uint32_t tile_id, const std::vector<std::pair<uint32_t, float>>& alt_tiles, const std::vector<AutotileMask>& masks, bool solid) :
m_tile_id(tile_id),
m_alt_tiles(std::move(alt_tiles)),
m_alt_tiles(alt_tiles),
m_masks(std::move(masks)),
m_solid(solid)
{
Expand All @@ -55,20 +49,14 @@ Autotile::matches(uint8_t num_mask, bool center) const
{
for (auto& l_mask : m_masks)
{
if (l_mask->matches(num_mask, center))
if (l_mask.matches(num_mask, center))
{
return true;
}
}
return false;
}

uint32_t
Autotile::get_tile_id() const
{
return m_tile_id;
}

uint32_t
Autotile::pick_tile(int x, int y) const
{
Expand Down Expand Up @@ -116,35 +104,28 @@ uint8_t
Autotile::get_first_mask() const
{
if (!m_masks.empty())
return m_masks[0]->get_mask();
return m_masks[0].get_mask();
return 0;
}

std::vector<std::pair<uint32_t, float>>
Autotile::get_all_tile_ids() const
{
return m_alt_tiles;
}

bool
Autotile::is_solid() const
{
return m_solid;
}


// AutotileSet.

std::vector<AutotileSet*>* AutotileSet::m_autotilesets = new std::vector<AutotileSet*>();
std::vector<std::unique_ptr<AutotileSet>> AutotileSet::m_autotilesets;

AutotileSet::AutotileSet(std::vector<Autotile*> tiles, uint32_t default_tile, std::string name, bool corner) :
m_autotiles(std::move(tiles)),
AutotileSet::AutotileSet(const std::vector<Autotile*>& tiles, uint32_t default_tile, const std::string& name, bool corner) :
m_autotiles(tiles),
m_default(default_tile),
m_name(std::move(name)),
m_name(name),
m_corner(corner)
{
}

AutotileSet::~AutotileSet()
{
for (Autotile* autotile : m_autotiles)
delete autotile;
}

/*
AutotileSet*
AutotileSet::get_tileset_from_tile(uint32_t tile_id)
Expand Down Expand Up @@ -210,7 +191,7 @@ AutotileSet::get_autotile(uint32_t tile_id,
if (top_left) num_mask = static_cast<uint8_t>(num_mask + 0x80);
}

for (auto& autotile : m_autotiles)
for (auto* autotile : m_autotiles)
{
if (autotile->matches(num_mask, center))
{
Expand All @@ -221,12 +202,6 @@ AutotileSet::get_autotile(uint32_t tile_id,
return center ? get_default_tile() : 0;
}

uint32_t
AutotileSet::get_default_tile() const
{
return m_default;
}

bool
AutotileSet::is_member(uint32_t tile_id) const
{
Expand Down Expand Up @@ -258,7 +233,7 @@ AutotileSet::is_solid(uint32_t tile_id) const
if (!is_member(tile_id))
return false;

for (auto& tile : m_autotiles)
for (auto* tile : m_autotiles)
{
if (tile->get_tile_id() == tile_id)
{
Expand Down Expand Up @@ -286,7 +261,7 @@ AutotileSet::is_solid(uint32_t tile_id) const
uint8_t
AutotileSet::get_mask_from_tile(uint32_t tile) const
{
for (auto& autotile : m_autotiles)
for (auto* autotile : m_autotiles)
{
if (autotile->is_amongst(tile)) {
return autotile->get_first_mask();
Expand All @@ -307,7 +282,7 @@ AutotileSet::validate() const
uint32_t tile_nonsolid = 0; // Relevant only for non-corner autotiles.
uint32_t tile_with_that_mask = 0; // Used to help users debug.

for (auto& autotile : m_autotiles)
for (auto* autotile : m_autotiles)
{
if (autotile->matches(num_mask, true))
{
Expand Down
40 changes: 13 additions & 27 deletions src/supertux/autotile.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,11 @@
#ifndef HEADER_SUPERTUX_SUPERTUX_AUTOTILE_HPP
#define HEADER_SUPERTUX_SUPERTUX_AUTOTILE_HPP

#include <algorithm>
#include <memory>
#include <stdint.h>
#include <string>
#include <algorithm>

#include "math/rect.hpp"
#include "math/rectf.hpp"
#include "math/size.hpp"
#include "object/path_object.hpp"
#include "object/path_walker.hpp"
#include "squirrel/exposed_object.hpp"
#include "scripting/tilemap.hpp"
#include "supertux/game_object.hpp"
#include "video/color.hpp"
#include "video/flip.hpp"
#include "video/drawing_target.hpp"
#include <vector>

class AutotileMask final
{
Expand All @@ -41,29 +30,25 @@ class AutotileMask final

bool matches(uint8_t mask, bool center) const;

uint8_t get_mask() const;
uint8_t get_mask() const { return m_mask; }

private:
uint8_t m_mask;
bool m_center; // m_center should *always* be the same as the m_solid of the corresponding Autotile

private:
AutotileMask(const AutotileMask&) = delete;
AutotileMask& operator=(const AutotileMask&) = delete;
};

class Autotile final
{
public:
Autotile(uint32_t tile_id,
std::vector<std::pair<uint32_t, float>> alt_tiles,
std::vector<AutotileMask*> masks,
const std::vector<std::pair<uint32_t, float>>& alt_tiles,
const std::vector<AutotileMask>& masks,
bool solid);

bool matches(uint8_t mask, bool center) const;

/** @deprecated Returns the base tile ID. */
uint32_t get_tile_id() const;
uint32_t get_tile_id() const { return m_tile_id; }

/** Picks a tile randomly amongst the possible ones for this autotile. */
uint32_t pick_tile(int x, int y) const;
Expand All @@ -75,15 +60,15 @@ class Autotile final
uint8_t get_first_mask() const;

/** Returns all possible tiles for this autotile */
std::vector<std::pair<uint32_t, float>> get_all_tile_ids() const;
const std::vector<std::pair<uint32_t, float>>& get_all_tile_ids() const { return m_alt_tiles; }

/** Returns true if the "center" bool of masks are true. All masks of given Autotile must have the same value for their "center" property.*/
bool is_solid() const;
bool is_solid() const { return m_solid; }

private:
uint32_t m_tile_id;
std::vector<std::pair<uint32_t, float>> m_alt_tiles;
std::vector<AutotileMask*> m_masks;
std::vector<AutotileMask> m_masks;
bool m_solid;

private:
Expand All @@ -98,7 +83,8 @@ class AutotileSet final
//static AutotileSet* get_tileset_from_tile(uint32_t tile_id);

public:
AutotileSet(std::vector<Autotile*> autotiles, uint32_t default_tile, std::string name, bool corner);
AutotileSet(const std::vector<Autotile*>& autotiles, uint32_t default_tile, const std::string& name, bool corner);
~AutotileSet();

/** Returns the ID of the tile to use, based on the surrounding tiles.
* If the autotileset is corner-based, the top, left, right, bottom and
Expand All @@ -112,7 +98,7 @@ class AutotileSet final
) const;

/** Returns the id of the first block in the autotileset. Used for erronous configs. */
uint32_t get_default_tile() const;
uint32_t get_default_tile() const { return m_default; }

/** true if the given tile is present in the autotileset */
bool is_member(uint32_t tile_id) const;
Expand All @@ -133,7 +119,7 @@ class AutotileSet final
void validate() const;

public:
static std::vector<AutotileSet*>* m_autotilesets;
static std::vector<std::unique_ptr<AutotileSet>> m_autotilesets;

private:
std::vector<Autotile*> m_autotiles;
Expand Down
30 changes: 14 additions & 16 deletions src/supertux/autotile_parser.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// SuperTux
// Copyright (C) 2008-2020 A. Semphris <[email protected]>,
// Matthias Braun <[email protected]>,
// Ingo Ruhnke <[email protected]>
// Copyright (C) 2020 A. Semphris <[email protected]>
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -29,7 +27,7 @@
#include "util/reader_mapping.hpp"
#include "util/file_system.hpp"

AutotileParser::AutotileParser(std::vector<AutotileSet*>* autotilesets, const std::string& filename) :
AutotileParser::AutotileParser(std::vector<std::unique_ptr<AutotileSet>>& autotilesets, const std::string& filename) :
m_autotilesets(autotilesets),
m_filename(filename),
m_tiles_path()
Expand Down Expand Up @@ -71,7 +69,7 @@ AutotileParser::parse()
void
AutotileParser::parse_autotileset(const ReaderMapping& reader, bool corner)
{
std::vector<Autotile*>* autotiles = new std::vector<Autotile*>();
std::vector<Autotile*> autotiles;

std::string name = "[unnamed]";
if (!reader.get("name", name))
Expand All @@ -91,28 +89,28 @@ AutotileParser::parse_autotileset(const ReaderMapping& reader, bool corner)
if (iter.get_key() == "autotile")
{
ReaderMapping tile_mapping = iter.as_mapping();
autotiles->push_back(parse_autotile(tile_mapping, corner));
autotiles.push_back(parse_autotile(tile_mapping, corner));
}
else if (iter.get_key() != "name" && iter.get_key() != "default")
{
log_warning << "Unknown symbol '" << iter.get_key() << "' in autotile config file" << std::endl;
}
}

AutotileSet* autotileset = new AutotileSet(*autotiles, default_id, name, corner);
std::unique_ptr<AutotileSet> autotileset = std::make_unique<AutotileSet>(autotiles, default_id, name, corner);

if (g_config->developer_mode)
{
autotileset->validate();
}

m_autotilesets->push_back(autotileset);
m_autotilesets.push_back(std::move(autotileset));
}

Autotile*
AutotileParser::parse_autotile(const ReaderMapping& reader, bool corner)
{
std::vector<AutotileMask*> autotile_masks;
std::vector<AutotileMask> autotile_masks;
std::vector<std::pair<uint32_t, float>> alt_ids;

uint32_t tile_id;
Expand Down Expand Up @@ -143,11 +141,11 @@ AutotileParser::parse_autotile(const ReaderMapping& reader, bool corner)

if (corner)
{
parse_mask_corner(mask, &autotile_masks);
parse_mask_corner(mask, autotile_masks);
}
else
{
parse_mask(mask, &autotile_masks, solid);
parse_mask(mask, autotile_masks, solid);
}
}
else if (iter.get_key() == "alt-id")
Expand Down Expand Up @@ -177,11 +175,11 @@ AutotileParser::parse_autotile(const ReaderMapping& reader, bool corner)
}
}

return new Autotile(tile_id, alt_ids, autotile_masks, !!solid);
return new Autotile(tile_id, alt_ids, autotile_masks, solid);
}

void
AutotileParser::parse_mask(std::string mask, std::vector<AutotileMask*>* autotile_masks, bool solid)
AutotileParser::parse_mask(std::string mask, std::vector<AutotileMask>& autotile_masks, bool solid)
{
if (mask.size() != 8)
{
Expand Down Expand Up @@ -224,12 +222,12 @@ AutotileParser::parse_mask(std::string mask, std::vector<AutotileMask*>* autotil

for (uint8_t val : masks)
{
autotile_masks->push_back(new AutotileMask(val, solid));
autotile_masks.push_back(AutotileMask(val, solid));
}
}

void
AutotileParser::parse_mask_corner(std::string mask, std::vector<AutotileMask*>* autotile_masks)
AutotileParser::parse_mask_corner(std::string mask, std::vector<AutotileMask>& autotile_masks)
{
if (mask.size() != 4)
{
Expand Down Expand Up @@ -272,7 +270,7 @@ AutotileParser::parse_mask_corner(std::string mask, std::vector<AutotileMask*>*

for (uint8_t val : masks)
{
autotile_masks->push_back(new AutotileMask(val, true));
autotile_masks.push_back(AutotileMask(val, true));
}
}

Expand Down
Loading

0 comments on commit fc5f1b5

Please sign in to comment.