Skip to content

Commit

Permalink
Merge pull request #85 from grunt-lucas/bugfix/issue-0060
Browse files Browse the repository at this point in the history 
Closes #60 by presenting warning and possible fix to user
  • Loading branch information
@grunt-lucas
grunt-lucas authored Feb 3, 2025
2 parents 30b44de + bfb68eb commit 4913d31
Show file tree
Hide file tree
Showing 20 changed files with 210 additions and 35 deletions.
5 changes: 5 additions & 0 deletions Porytiles-0.x/lib/include/porytiles/cli_options.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,11 @@ const std::string WNO_MISSING_ASSIGN_CONFIG = W_GENERAL + "no-" + WARN_MISSING_A
constexpr int WMISSING_ASSIGN_CONFIG_VAL = 50100;
constexpr int WNO_MISSING_ASSIGN_CONFIG_VAL = 60100;

const std::string WKEY_FRAME_MISSING_COLORS = W_GENERAL + WARN_KEY_FRAME_MISSING_COLORS;
const std::string WNO_KEY_FRAME_MISSING_COLORS = W_GENERAL + "no-" + WARN_KEY_FRAME_MISSING_COLORS;
constexpr int WKEY_FRAME_MISSING_COLORS_VAL = 50110;
constexpr int WNO_KEY_FRAME_MISSING_COLORS_VAL = 60110;

// Decompilation warnings
const std::string WTILE_INDEX_OUT_OF_RANGE = W_GENERAL + WARN_TILE_INDEX_OUT_OF_RANGE;
const std::string WNO_TILE_INDEX_OUT_OF_RANGE = W_GENERAL + "no-" + WARN_TILE_INDEX_OUT_OF_RANGE;
Expand Down
21 changes: 18 additions & 3 deletions Porytiles-0.x/lib/include/porytiles/errors_warnings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include <cstddef>
#include <string>
#include <unordered_set>

#include "types.h"

Expand All @@ -18,6 +19,7 @@ struct ErrorsAndWarnings {
* counts instead of just a generalized count.
*/
std::size_t errCount;
std::size_t keyFrameMissingColorsErrCount;
std::size_t warnCount;
bool printErrors;

Expand All @@ -32,22 +34,27 @@ struct ErrorsAndWarnings {
WarningMode assignCacheOverride;
WarningMode invalidAssignCache;
WarningMode missingAssignCache;
WarningMode keyFrameMissingColors;

// Decompilation warnings
WarningMode tileIndexOutOfRange;
WarningMode paletteIndexOutOfRange;

ErrorsAndWarnings()
: errCount{0}, warnCount{0}, printErrors{true}, colorPrecisionLoss{WarningMode::OFF},
: errCount{0}, keyFrameMissingColorsErrCount{0}, warnCount{0}, printErrors{true}, colorPrecisionLoss{WarningMode::OFF},
keyFrameNoMatchingTile{WarningMode::OFF}, usedTrueColorMode{WarningMode::OFF},
attributeFormatMismatch{WarningMode::OFF}, missingAttributesCsv{WarningMode::OFF},
unusedAttribute{WarningMode::OFF}, transparencyCollapse{WarningMode::OFF},
assignCacheOverride{WarningMode::OFF}, invalidAssignCache{WarningMode::OFF},
missingAssignCache{WarningMode::OFF}, tileIndexOutOfRange{WarningMode::OFF},
paletteIndexOutOfRange{WarningMode::OFF}
missingAssignCache{WarningMode::OFF}, keyFrameMissingColors(WarningMode::OFF),
tileIndexOutOfRange{WarningMode::OFF}, paletteIndexOutOfRange{WarningMode::OFF}
{
}

[[nodiscard]] std::size_t errTotal() const {
return errCount + keyFrameMissingColorsErrCount;
}

void setAllWarnings(WarningMode setting)
{
// Compilation warnings
Expand All @@ -61,6 +68,7 @@ struct ErrorsAndWarnings {
assignCacheOverride = setting;
invalidAssignCache = setting;
missingAssignCache = setting;
keyFrameMissingColors = setting;

// Decompilation warnings
tileIndexOutOfRange = setting;
Expand Down Expand Up @@ -100,6 +108,9 @@ struct ErrorsAndWarnings {
if (missingAssignCache == WarningMode::WARN) {
missingAssignCache = WarningMode::ERR;
}
if (keyFrameMissingColors == WarningMode::WARN) {
keyFrameMissingColors = WarningMode::ERR;
}

// Decompilation warnings
if (tileIndexOutOfRange == WarningMode::WARN) {
Expand All @@ -122,6 +133,7 @@ extern const char *const WARN_TRANSPARENCY_COLLAPSE;
extern const char *const WARN_ASSIGN_CACHE_OVERRIDE;
extern const char *const WARN_INVALID_ASSIGN_CACHE;
extern const char *const WARN_MISSING_ASSIGN_CACHE;
extern const char *const WARN_KEY_FRAME_MISSING_COLORS;

// Decompilation warnings
extern const char *const WARN_TILE_INDEX_OUT_OF_RANGE;
Expand Down Expand Up @@ -286,6 +298,9 @@ void warn_invalidAssignCache(ErrorsAndWarnings &err, const CompilerConfig &confi

void warn_missingAssignCache(ErrorsAndWarnings &err, const CompilerConfig &config, std::string path);

void warn_keyFrameMissingColors(ErrorsAndWarnings &err, const CompilerSourcePaths &srcs, const CompilerMode &mode,
std::size_t tileIndex, const std::unordered_set<RGBA32> &missingColors, const std::string &animName);

/*
* Decompilation warnings (due to possible mistakes in user input), decompilation can continue
*/
Expand Down
72 changes: 43 additions & 29 deletions Porytiles-0.x/lib/include/porytiles/types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,39 +87,53 @@ template <> struct std::hash<porytiles::BGR15> {
};

namespace porytiles {
/**
* RGBA32 format. 1 byte per color and 1 byte for alpha channel.
*/
struct RGBA32 {
std::uint8_t red;
std::uint8_t green;
std::uint8_t blue;
std::uint8_t alpha;

[[nodiscard]] std::string jasc() const
{
return std::to_string(red) + " " + std::to_string(green) + " " + std::to_string(blue);
}
constexpr std::uint8_t ALPHA_TRANSPARENT = 0;
constexpr std::uint8_t ALPHA_OPAQUE = 255;

auto operator<=>(const RGBA32 &rgba) const = default;
/**
* RGBA32 format. 1 byte per color and 1 byte for alpha channel.
*/
struct RGBA32 {
std::uint8_t red;
std::uint8_t green;
std::uint8_t blue;
std::uint8_t alpha;

[[nodiscard]] std::string jasc() const
{
return std::to_string(red) + " " + std::to_string(green) + " " + std::to_string(blue);
}

friend std::ostream &operator<<(std::ostream &os, const RGBA32 &rgba);
auto operator<=>(const RGBA32 &rgba) const = default;

friend std::ostream &operator<<(std::ostream &os, const RGBA32 &rgba);
};

extern const RGBA32 RGBA_BLACK;
extern const RGBA32 RGBA_RED;
extern const RGBA32 RGBA_GREEN;
extern const RGBA32 RGBA_BLUE;
extern const RGBA32 RGBA_YELLOW;
extern const RGBA32 RGBA_MAGENTA;
extern const RGBA32 RGBA_CYAN;
extern const RGBA32 RGBA_WHITE;
extern const RGBA32 RGBA_GREY;
extern const RGBA32 RGBA_PURPLE;
extern const RGBA32 RGBA_LIME;
}

// TODO : better hash function
template <> struct std::hash<porytiles::RGBA32> {
std::size_t operator()(const porytiles::RGBA32 &rgba) const noexcept {
std::size_t h1 = std::hash<std::uint8_t>{}(rgba.red);
std::size_t h2 = std::hash<std::uint8_t>{}(rgba.green);
std::size_t h3 = std::hash<std::uint8_t>{}(rgba.blue);
std::size_t h4 = std::hash<std::uint8_t>{}(rgba.alpha);
return h1 ^ (h2 << 8) ^ (h3 << 16) ^ (h4 << 24);
}
};

constexpr std::uint8_t ALPHA_TRANSPARENT = 0;
constexpr std::uint8_t ALPHA_OPAQUE = 255;

extern const RGBA32 RGBA_BLACK;
extern const RGBA32 RGBA_RED;
extern const RGBA32 RGBA_GREEN;
extern const RGBA32 RGBA_BLUE;
extern const RGBA32 RGBA_YELLOW;
extern const RGBA32 RGBA_MAGENTA;
extern const RGBA32 RGBA_CYAN;
extern const RGBA32 RGBA_WHITE;
extern const RGBA32 RGBA_GREY;
extern const RGBA32 RGBA_PURPLE;
extern const RGBA32 RGBA_LIME;
namespace porytiles {

BGR15 rgbaToBgr(const RGBA32 &rgba) noexcept;

Expand Down
38 changes: 38 additions & 0 deletions Porytiles-0.x/lib/src/cli_parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,8 @@ std::unordered_map<std::string, std::unordered_set<Subcommand>> supportedSubcomm
{WNO_INVALID_ASSIGN_CONFIG_CACHE, {Subcommand::COMPILE_PRIMARY, Subcommand::COMPILE_SECONDARY}},
{WMISSING_ASSIGN_CONFIG, {Subcommand::COMPILE_PRIMARY, Subcommand::COMPILE_SECONDARY}},
{WNO_MISSING_ASSIGN_CONFIG, {Subcommand::COMPILE_PRIMARY, Subcommand::COMPILE_SECONDARY}},
{WKEY_FRAME_MISSING_COLORS, {Subcommand::COMPILE_PRIMARY, Subcommand::COMPILE_SECONDARY}},
{WNO_KEY_FRAME_MISSING_COLORS, {Subcommand::COMPILE_PRIMARY, Subcommand::COMPILE_SECONDARY}},
// Decompilation warnings
{WTILE_INDEX_OUT_OF_RANGE, {Subcommand::DECOMPILE_PRIMARY, Subcommand::DECOMPILE_SECONDARY}},
{WNO_TILE_INDEX_OUT_OF_RANGE, {Subcommand::DECOMPILE_PRIMARY, Subcommand::DECOMPILE_SECONDARY}},
Expand Down Expand Up @@ -815,6 +817,9 @@ static void parseSubcommandOptions(PorytilesContext &ctx, int argc, char *const
{WMISSING_ASSIGN_CONFIG.c_str(), no_argument, nullptr, WMISSING_ASSIGN_CONFIG_VAL},
{WNO_MISSING_ASSIGN_CONFIG.c_str(), no_argument, nullptr, WNO_MISSING_ASSIGN_CONFIG_VAL},

{WKEY_FRAME_MISSING_COLORS.c_str(), no_argument, nullptr, WKEY_FRAME_MISSING_COLORS_VAL},
{WNO_KEY_FRAME_MISSING_COLORS.c_str(), no_argument, nullptr, WNO_KEY_FRAME_MISSING_COLORS_VAL},

// Decompilation warnings
{WTILE_INDEX_OUT_OF_RANGE.c_str(), no_argument, nullptr, WTILE_INDEX_OUT_OF_RANGE_VAL},
{WNO_TILE_INDEX_OUT_OF_RANGE.c_str(), no_argument, nullptr, WNO_TILE_INDEX_OUT_OF_RANGE_VAL},
Expand Down Expand Up @@ -869,6 +874,11 @@ static void parseSubcommandOptions(PorytilesContext &ctx, int argc, char *const
std::optional<bool> warnMissingAssignCache{};
std::optional<bool> errMissingAssignCache{};

// Default this warning to on, since it can lead to issues
// https://github.com/grunt-lucas/porytiles/issues/60
std::optional<bool> warnKeyFrameMissingColors{true};
std::optional<bool> errKeyFrameMissingColors{};

// Decompilation warnings
std::optional<bool> warnTileIndexOutOfRange{};
std::optional<bool> errTileIndexOutOfRange{};
Expand Down Expand Up @@ -1147,6 +1157,9 @@ static void parseSubcommandOptions(PorytilesContext &ctx, int argc, char *const
else if (strcmp(optarg, WARN_MISSING_ASSIGN_CACHE) == 0) {
errMissingAssignCache = true;
}
else if (strcmp(optarg, WARN_KEY_FRAME_MISSING_COLORS) == 0) {
errKeyFrameMissingColors = true;
}
// Decompilation warnings
else if (strcmp(optarg, WARN_TILE_INDEX_OUT_OF_RANGE) == 0) {
errTileIndexOutOfRange = true;
Expand Down Expand Up @@ -1194,6 +1207,9 @@ static void parseSubcommandOptions(PorytilesContext &ctx, int argc, char *const
else if (strcmp(optarg, WARN_MISSING_ASSIGN_CACHE) == 0) {
errMissingAssignCache = false;
}
else if (strcmp(optarg, WARN_KEY_FRAME_MISSING_COLORS) == 0) {
errKeyFrameMissingColors = false;
}
// Decompilation warnings
else if (strcmp(optarg, WARN_TILE_INDEX_OUT_OF_RANGE) == 0) {
errTileIndexOutOfRange = false;
Expand Down Expand Up @@ -1289,6 +1305,14 @@ static void parseSubcommandOptions(PorytilesContext &ctx, int argc, char *const
validateSubcommandContext(ctx, WNO_MISSING_ASSIGN_CONFIG);
warnMissingAssignCache = false;
break;
case WKEY_FRAME_MISSING_COLORS_VAL:
validateSubcommandContext(ctx, WKEY_FRAME_MISSING_COLORS);
warnKeyFrameMissingColors = true;
break;
case WNO_KEY_FRAME_MISSING_COLORS_VAL:
validateSubcommandContext(ctx, WNO_KEY_FRAME_MISSING_COLORS);
warnKeyFrameMissingColors = false;
break;
// Decompilation warnings
case WTILE_INDEX_OUT_OF_RANGE_VAL:
validateSubcommandContext(ctx, WTILE_INDEX_OUT_OF_RANGE);
Expand Down Expand Up @@ -1430,6 +1454,9 @@ static void parseSubcommandOptions(PorytilesContext &ctx, int argc, char *const
if (warnMissingAssignCache.has_value()) {
ctx.err.missingAssignCache = warnMissingAssignCache.value() ? WarningMode::WARN : WarningMode::OFF;
}
if (warnKeyFrameMissingColors.has_value()) {
ctx.err.keyFrameMissingColors = warnKeyFrameMissingColors.value() ? WarningMode::WARN : WarningMode::OFF;
}
// Decompilation warnings
if (warnTileIndexOutOfRange.has_value()) {
ctx.err.tileIndexOutOfRange = warnTileIndexOutOfRange.value() ? WarningMode::WARN : WarningMode::OFF;
Expand Down Expand Up @@ -1562,6 +1589,17 @@ static void parseSubcommandOptions(PorytilesContext &ctx, int argc, char *const
ctx.err.missingAssignCache = WarningMode::OFF;
}
}
if (errKeyFrameMissingColors.has_value()) {
if (errKeyFrameMissingColors.value()) {
ctx.err.keyFrameMissingColors = WarningMode::ERR;
}
else if ((warnKeyFrameMissingColors.has_value() && warnKeyFrameMissingColors.value()) || enableAllWarnings) {
ctx.err.keyFrameMissingColors = WarningMode::WARN;
}
else {
ctx.err.keyFrameMissingColors = WarningMode::OFF;
}
}
// Decompilation warnings
if (errTileIndexOutOfRange.has_value()) {
if (errTileIndexOutOfRange.value()) {
Expand Down
5 changes: 5 additions & 0 deletions Porytiles-0.x/lib/src/compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,7 @@ TEST_CASE("normalizeDecompTiles should correctly normalize all tiles in the deco
TEST_CASE("normalizeDecompTiles should correctly normalize multi-frame animated tiles")
{
porytiles::PorytilesContext ctx{};
ctx.err.printErrors = false;

REQUIRE(std::filesystem::exists(std::filesystem::path{"Resources/Tests/2x2_pattern_2.png"}));
png::image<png::rgba_pixel> tilesPng{"Resources/Tests/2x2_pattern_2.png"};
Expand Down Expand Up @@ -1243,6 +1244,7 @@ TEST_CASE("normalizeDecompTiles should correctly normalize multi-frame animated
TEST_CASE("buildColorIndexMaps should build a map of all unique colors in the decomp tileset")
{
porytiles::PorytilesContext ctx{};
ctx.err.printErrors = false;

REQUIRE(std::filesystem::exists(std::filesystem::path{"Resources/Tests/2x2_pattern_2.png"}));
png::image<png::rgba_pixel> png1{"Resources/Tests/2x2_pattern_2.png"};
Expand Down Expand Up @@ -1299,6 +1301,7 @@ TEST_CASE("toColorSet should return the correct bitset based on the supplied pal
TEST_CASE("matchNormalizedWithColorSets should return the expected data structures")
{
porytiles::PorytilesContext ctx{};
ctx.err.printErrors = false;

REQUIRE(std::filesystem::exists(std::filesystem::path{"Resources/Tests/2x2_pattern_2.png"}));
png::image<png::rgba_pixel> png1{"Resources/Tests/2x2_pattern_2.png"};
Expand Down Expand Up @@ -1911,6 +1914,7 @@ TEST_CASE("compile function should fill out secondary CompiledTileset struct wit
TEST_CASE("compile function should correctly compile primary set with animated tiles")
{
porytiles::PorytilesContext ctx{};
ctx.err.printErrors = false;
ctx.fieldmapConfig.numPalettesInPrimary = 3;
ctx.fieldmapConfig.numPalettesTotal = 6;
ctx.compilerConfig.primaryAssignAlgorithm = porytiles::AssignAlgorithm::DFS;
Expand Down Expand Up @@ -2124,6 +2128,7 @@ TEST_CASE("compile function should correctly compile primary set with animated t
TEST_CASE("compile function should correctly compile secondary set with animated tiles")
{
porytiles::PorytilesContext ctx{};
ctx.err.printErrors = false;
ctx.fieldmapConfig.numPalettesInPrimary = 3;
ctx.fieldmapConfig.numPalettesTotal = 6;
ctx.compilerConfig.primaryAssignAlgorithm = porytiles::AssignAlgorithm::DFS;
Expand Down
34 changes: 32 additions & 2 deletions Porytiles-0.x/lib/src/errors_warnings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const char *const WARN_TRANSPARENCY_COLLAPSE = "transparency-collapse";
const char *const WARN_ASSIGN_CACHE_OVERRIDE = "assign-cache-override";
const char *const WARN_INVALID_ASSIGN_CACHE = "invalid-assign-cache";
const char *const WARN_MISSING_ASSIGN_CACHE = "missing-assign-cache";
const char *const WARN_KEY_FRAME_MISSING_COLORS = "key-frame-missing-colors";

// Decompilation warnings
const char *const WARN_TILE_INDEX_OUT_OF_RANGE = "tile-index-out-of-range";
Expand Down Expand Up @@ -737,6 +738,35 @@ void warn_missingAssignCache(ErrorsAndWarnings &err, const CompilerConfig &confi
}
}

void warn_keyFrameMissingColors(ErrorsAndWarnings &err, const CompilerSourcePaths &srcs, const CompilerMode &mode,
std::size_t tileIndex, const std::unordered_set<RGBA32> &missingColors, const std::string &animName) {
if (err.keyFrameMissingColors == WarningMode::ERR) {
err.keyFrameMissingColorsErrCount++;
if (err.printErrors) {
pt_err(
"animation `{}' key frame tile `{}' missing essential colors [{}]", fmt::styled(animName, fmt::emphasis::bold), fmt::styled(tileIndex, fmt::emphasis::bold),
fmt::styled(fmt::format("-Werror={}", WARN_KEY_FRAME_MISSING_COLORS), fmt::emphasis::bold | fmt::fg(fmt::terminal_color::red)));
}
}
else if (err.keyFrameMissingColors == WarningMode::WARN) {
err.warnCount++;
if (err.printErrors) {
pt_warn(
"animation `{}' key frame tile `{}' missing essential colors [{}]", fmt::styled(animName, fmt::emphasis::bold), fmt::styled(tileIndex, fmt::emphasis::bold),
fmt::styled(fmt::format("-W{}", WARN_KEY_FRAME_MISSING_COLORS), fmt::emphasis::bold | fmt::fg(fmt::terminal_color::magenta)));
}
}
if (err.printErrors) {
pt_note("the following colors were missing from the key frame:");
for (const auto &color: missingColors) {
pt_println(stderr, " {}", fmt::styled(color.jasc(), fmt::emphasis::bold));
}
pt_println(stderr, "If left uncorrected, this may lead to the issue described here:");
pt_println(stderr, " https://github.com/grunt-lucas/porytiles/issues/60");
pt_println(stderr, "");
}
}

void warn_tileIndexOutOfRange(ErrorsAndWarnings &err, DecompilerMode mode, std::size_t tileIndex,
std::size_t tilesheetSize, const RGBATile &tile)
{
Expand Down Expand Up @@ -801,7 +831,7 @@ void die_errorCount(const ErrorsAndWarnings &err, std::string srcPath, std::stri
{
if (err.printErrors) {
std::string errorStr;
if (err.errCount == 1) {
if (err.errTotal() == 1) {
errorStr = "error";
}
else {
Expand All @@ -819,7 +849,7 @@ void die_errorCount(const ErrorsAndWarnings &err, std::string srcPath, std::stri
std::to_string(err.errCount), errorStr);
}
else {
pt_println(stderr, "{} {} generated.", std::to_string(err.errCount), errorStr);
pt_println(stderr, "{} {} generated.", std::to_string(err.errTotal()), errorStr);
}
pt_println(stderr, "terminating compilation of {}", fmt::styled(srcPath, fmt::emphasis::bold));
}
Expand Down
Loading

0 comments on commit 4913d31

Please sign in to comment.