Added warning for transparency collapse

Todo.md

@@ -58,8 +58,6 @@
 + Warnings
   + `-Wpalette-alloc-efficiency`
     + warn user if palette allocation was not 100% efficient
-  + `-Wtransprent-color-precision`
-    + warn user if a color collapsed to the transparent color
   + `-Wno-transparency-present`
     + warn user if the tileset did not have any of the selected transparency color
     + this is a common mistake if user decompiles a vanilla tileset that uses a different transparency color

include/cli_options.h

@@ -237,6 +237,11 @@ const std::string WNO_UNUSED_ATTRIBUTE = W_GENERAL + "no-" + WARN_UNUSED_ATTRIBU
 constexpr int WUNUSED_ATTRIBUTE_VAL = 50060;
 constexpr int WNO_UNUSED_ATTRIBUTE_VAL = 60060;
 
+const std::string WTRANSPARENCY_COLLAPSE = W_GENERAL + WARN_TRANSPARENCY_COLLAPSE;
+const std::string WNO_TRANSPARENCY_COLLAPSE = W_GENERAL + "no-" + WARN_TRANSPARENCY_COLLAPSE;
+constexpr int WTRANSPARENCY_COLLAPSE_VAL = 50070;
+constexpr int WNO_TRANSPARENCY_COLLAPSE_VAL = 60070;
+
 // @formatter:on
 // clang-format on
 

include/errors_warnings.h

@@ -29,12 +29,14 @@ struct ErrorsAndWarnings {
   WarningMode missingAttributesCsv;
   WarningMode missingBehaviorsHeader;
   WarningMode unusedAttribute;
+  WarningMode transparencyCollapse;
 
   ErrorsAndWarnings()
       : errCount{0}, warnCount{0}, printErrors{true}, colorPrecisionLoss{WarningMode::OFF},
         keyFrameTileDidNotAppearInAssignment{WarningMode::OFF}, usedTrueColorMode{WarningMode::OFF},
         attributeFormatMismatch{WarningMode::OFF}, missingAttributesCsv{WarningMode::OFF},
-        missingBehaviorsHeader{WarningMode::OFF}, unusedAttribute{WarningMode::OFF}
+        missingBehaviorsHeader{WarningMode::OFF}, unusedAttribute{WarningMode::OFF},
+        transparencyCollapse{WarningMode::OFF}
   {
   }
 
@@ -47,6 +49,7 @@ struct ErrorsAndWarnings {
     missingAttributesCsv = setting;
     missingBehaviorsHeader = setting;
     unusedAttribute = setting;
+    transparencyCollapse = setting;
   }
 
   void setAllEnabledWarningsToErrors()
@@ -72,6 +75,9 @@ struct ErrorsAndWarnings {
     if (unusedAttribute == WarningMode::WARN) {
       unusedAttribute = WarningMode::ERR;
     }
+    if (transparencyCollapse == WarningMode::WARN) {
+      transparencyCollapse = WarningMode::ERR;
+    }
   }
 };
 
@@ -82,6 +88,7 @@ extern const char *const WARN_ATTRIBUTE_FORMAT_MISMATCH;
 extern const char *const WARN_MISSING_ATTRIBUTES_CSV;
 extern const char *const WARN_MISSING_BEHAVIORS_HEADER;
 extern const char *const WARN_UNUSED_ATTRIBUTE;
+extern const char *const WARN_TRANSPARENCY_COLLAPSE;
 
 // Internal compiler errors (due to bug in the compiler)
 void internalerror(std::string message);
@@ -110,10 +117,6 @@ void error_tooManyUniqueColorsInTile(ErrorsAndWarnings &err, const RGBATile &til
 void error_invalidAlphaValue(ErrorsAndWarnings &err, const RGBATile &tile, std::uint8_t alpha, std::size_t row,
                              std::size_t col);
 
-void error_nonTransparentRgbaCollapsedToTransparentBgr(ErrorsAndWarnings &err, const RGBATile &tile, std::size_t row,
-                                                       std::size_t col, const RGBA32 &color,
-                                                       const RGBA32 &transparency);
-
 void error_allThreeLayersHadNonTransparentContent(ErrorsAndWarnings &err, std::size_t metatileIndex);
 
 void error_invalidCsvRowFormat(ErrorsAndWarnings &err, std::string filePath, std::size_t line);
@@ -206,6 +209,9 @@ void warn_behaviorsHeaderNotSpecified(ErrorsAndWarnings &err, std::string filePa
 void warn_unusedAttribute(ErrorsAndWarnings &err, std::size_t metatileId, std::size_t metatileCount,
                           std::string sourcePath);
 
+void warn_nonTransparentRgbaCollapsedToTransparentBgr(ErrorsAndWarnings &err, const RGBATile &tile, std::size_t row,
+                                                      std::size_t col, const RGBA32 &color, const RGBA32 &transparency);
+
 // Die functions
 void die(const ErrorsAndWarnings &err, std::string errorMessage);
 

src/cli_parser.cpp

@@ -480,7 +480,7 @@ WERROR_DESC + "\n";
 // clang-format on
 
 /*
- * TODO : the warning parsing system here is a dumpster fire
+ * FIXME : the warning parsing system here is a dumpster fire
  */
 static void parseCompile(PtContext &ctx, int argc, char *const *argv)
 {
@@ -534,6 +534,8 @@ static void parseCompile(PtContext &ctx, int argc, char *const *argv)
       {WNO_MISSING_BEHAVIORS_HEADER.c_str(), no_argument, nullptr, WNO_MISSING_BEHAVIORS_HEADER_VAL},
       {WUNUSED_ATTRIBUTE.c_str(), no_argument, nullptr, WUNUSED_ATTRIBUTE_VAL},
       {WNO_UNUSED_ATTRIBUTE.c_str(), no_argument, nullptr, WNO_UNUSED_ATTRIBUTE_VAL},
+      {WTRANSPARENCY_COLLAPSE.c_str(), no_argument, nullptr, WTRANSPARENCY_COLLAPSE_VAL},
+      {WNO_TRANSPARENCY_COLLAPSE.c_str(), no_argument, nullptr, WNO_TRANSPARENCY_COLLAPSE_VAL},
 
       // Help
       {HELP.c_str(), no_argument, nullptr, HELP_VAL},
@@ -571,6 +573,9 @@ static void parseCompile(PtContext &ctx, int argc, char *const *argv)
   std::optional<bool> warnUnusedAttributeOverride{};
   std::optional<bool> errUnusedAttributeOverride{};
 
+  std::optional<bool> warnTransparencyCollapseOverride{};
+  std::optional<bool> errTransparencyCollapseOverride{};
+
   /*
    * Fieldmap specific variables. Like warnings above, we must wait until after all options are processed before we
    * start applying the fieldmap config. We want specific fieldmap overrides to take precedence over the general
@@ -695,6 +700,9 @@ static void parseCompile(PtContext &ctx, int argc, char *const *argv)
         else if (strcmp(optarg, WARN_UNUSED_ATTRIBUTE) == 0) {
           errUnusedAttributeOverride = true;
         }
+        else if (strcmp(optarg, WARN_TRANSPARENCY_COLLAPSE) == 0) {
+          errTransparencyCollapseOverride = true;
+        }
         else {
           fatalerror_porytilesprefix(ctx.err, fmt::format("invalid argument '{}' for option '{}'",
                                                           fmt::styled(std::string{optarg}, fmt::emphasis::bold),
@@ -724,6 +732,9 @@ static void parseCompile(PtContext &ctx, int argc, char *const *argv)
       else if (strcmp(optarg, WARN_UNUSED_ATTRIBUTE) == 0) {
         errUnusedAttributeOverride = false;
       }
+      else if (strcmp(optarg, WARN_TRANSPARENCY_COLLAPSE) == 0) {
+        errTransparencyCollapseOverride = false;
+      }
       else {
         fatalerror_porytilesprefix(ctx.err, fmt::format("invalid argument '{}' for option '{}'",
                                                         fmt::styled(std::string{optarg}, fmt::emphasis::bold),
@@ -774,6 +785,12 @@ static void parseCompile(PtContext &ctx, int argc, char *const *argv)
     case WNO_UNUSED_ATTRIBUTE_VAL:
       warnUnusedAttributeOverride = false;
       break;
+    case WTRANSPARENCY_COLLAPSE_VAL:
+      warnTransparencyCollapseOverride = true;
+      break;
+    case WNO_TRANSPARENCY_COLLAPSE_VAL:
+      warnTransparencyCollapseOverride = false;
+      break;
 
     // Help message upon '-h/--help' goes to stdout
     case HELP_VAL:
@@ -836,6 +853,9 @@ static void parseCompile(PtContext &ctx, int argc, char *const *argv)
   if (warnUnusedAttributeOverride.has_value()) {
     ctx.err.unusedAttribute = warnUnusedAttributeOverride.value() ? WarningMode::WARN : WarningMode::OFF;
   }
+  if (warnTransparencyCollapseOverride.has_value()) {
+    ctx.err.transparencyCollapse = warnTransparencyCollapseOverride.value() ? WarningMode::WARN : WarningMode::OFF;
+  }
 
   // If requested, set all enabled warnings to errors
   if (setAllEnabledWarningsToErrors) {
@@ -927,6 +947,18 @@ static void parseCompile(PtContext &ctx, int argc, char *const *argv)
       ctx.err.unusedAttribute = WarningMode::OFF;
     }
   }
+  if (errTransparencyCollapseOverride.has_value()) {
+    if (errTransparencyCollapseOverride.value()) {
+      ctx.err.transparencyCollapse = WarningMode::ERR;
+    }
+    else if ((warnTransparencyCollapseOverride.has_value() && warnTransparencyCollapseOverride.value()) ||
+             enableAllWarnings) {
+      ctx.err.transparencyCollapse = WarningMode::WARN;
+    }
+    else {
+      ctx.err.transparencyCollapse = WarningMode::OFF;
+    }
+  }
 
   if (disableAllWarnings) {
     /*

src/compiler.cpp

@@ -34,8 +34,7 @@ static std::size_t insertRGBA(PtContext &ctx, const RGBATile &rgbaFrame, const R
      * If we hit this case, it's almost certainly a user mistake so let's push an error. We would prefer to err on the
      * side of forcing the user to be explicit, especially when it comes to transparency handling.
      */
-    // TODO : actually, make this a warn since decompiled tilesets turn 255 0 255 into 248 0 248
-    // error_nonTransparentRgbaCollapsedToTransparentBgr(ctx.err, rgbaFrame, row, col, rgba, transparencyColor);
+    warn_nonTransparentRgbaCollapsedToTransparentBgr(ctx.err, rgbaFrame, row, col, rgba, transparencyColor);
   }
   /*
    * Insert an rgba32 color into a normalized palette. The color will be converted to bgr15 format in the process,

src/errors_warnings.cpp

@@ -26,6 +26,7 @@ const char *const WARN_ATTRIBUTE_FORMAT_MISMATCH = "attribute-format-mismatch";
 const char *const WARN_MISSING_ATTRIBUTES_CSV = "missing-attributes-csv";
 const char *const WARN_MISSING_BEHAVIORS_HEADER = "missing-behaviors-header";
 const char *const WARN_UNUSED_ATTRIBUTE = "unused-attribute";
+const char *const WARN_TRANSPARENCY_COLLAPSE = "transparency-collapse";
 
 static std::string getTilePrettyString(const RGBATile &tile)
 {
@@ -152,19 +153,6 @@ void error_invalidAlphaValue(ErrorsAndWarnings &err, const RGBATile &tile, std::
   }
 }
 
-void error_nonTransparentRgbaCollapsedToTransparentBgr(ErrorsAndWarnings &err, const RGBATile &tile, std::size_t row,
-                                                       std::size_t col, const RGBA32 &color, const RGBA32 &transparency)
-{
-  err.errCount++;
-  if (err.printErrors) {
-    std::string tileString = getTilePrettyString(tile);
-    pt_err("color '{}' at {} subtile pixel col {}, row {} collapsed to transparent under BGR conversion",
-           fmt::styled(color.jasc(), fmt::emphasis::bold), fmt::styled(tileString, fmt::emphasis::bold),
-           fmt::styled(col, fmt::emphasis::bold), fmt::styled(row, fmt::emphasis::bold));
-    pt_println(stderr, "");
-  }
-}
-
 void error_allThreeLayersHadNonTransparentContent(ErrorsAndWarnings &err, std::size_t metatileIndex)
 {
   err.errCount++;
@@ -566,6 +554,22 @@ void warn_unusedAttribute(ErrorsAndWarnings &err, std::size_t metatileId, std::s
   }
 }
 
+void warn_nonTransparentRgbaCollapsedToTransparentBgr(ErrorsAndWarnings &err, const RGBATile &tile, std::size_t row,
+                                                      std::size_t col, const RGBA32 &color, const RGBA32 &transparency)
+{
+  std::string tileString = getTilePrettyString(tile);
+  printWarning(
+      err, err.transparencyCollapse, WARN_TRANSPARENCY_COLLAPSE,
+      fmt::format("color '{}' at {} subtile pixel col {}, row {} collapsed to transparent under BGR conversion",
+                  fmt::styled(color.jasc(), fmt::emphasis::bold), fmt::styled(tileString, fmt::emphasis::bold),
+                  fmt::styled(col, fmt::emphasis::bold), fmt::styled(row, fmt::emphasis::bold)));
+  if (err.printErrors && err.transparencyCollapse != WarningMode::OFF) {
+    // TODO : add any more info via a note?
+    // pt_note("");
+    pt_println(stderr, "");
+  }
+}
+
 void die(const ErrorsAndWarnings &err, std::string errorMessage)
 {
   if (err.printErrors) {
@@ -694,21 +698,6 @@ TEST_CASE("error_animFrameWasNotAPng should trigger correctly when an anim frame
   CHECK(ctx.err.errCount == 1);
 }
 
-TEST_CASE("error_nonTransparentRgbaCollapsedToTransparentBgr should trigger correctly when a color collapses")
-{
-  // TODO : commented out this error for now, will make it a warn
-  // porytiles::PtContext ctx{};
-  // ctx.subcommand = porytiles::Subcommand::COMPILE_PRIMARY;
-  // ctx.fieldmapConfig.numPalettesInPrimary = 1;
-  // ctx.fieldmapConfig.numPalettesTotal = 2;
-  // ctx.srcPaths.primarySourcePath = "res/tests/errors_and_warnings/error_nonTransparentRgbaCollapsedToTransparentBgr";
-  // ctx.err.printErrors = false;
-  // ctx.compilerConfig.assignAlgorithm = porytiles::AssignAlgorithm::DEPTH_FIRST;
-
-  // CHECK_THROWS_WITH_AS(porytiles::drive(ctx), "errors generated during tile normalization", porytiles::PtException);
-  // CHECK(ctx.err.errCount == 2);
-}
-
 TEST_CASE("error_allThreeLayersHadNonTransparentContent should trigger correctly when a dual-layer inference fails")
 {
   porytiles::PtContext ctx{};
@@ -1212,3 +1201,19 @@ TEST_CASE("warn_unusedAttribute should correctly warn")
     CHECK(ctx.err.errCount == 1);
   }
 }
+
+TEST_CASE("warn_nonTransparentRgbaCollapsedToTransparentBgr should trigger correctly when a color collapses")
+{
+  porytiles::PtContext ctx{};
+  ctx.subcommand = porytiles::Subcommand::COMPILE_PRIMARY;
+  ctx.fieldmapConfig.numPalettesInPrimary = 1;
+  ctx.fieldmapConfig.numPalettesTotal = 2;
+  ctx.compilerSrcPaths.primarySourcePath =
+      "res/tests/errors_and_warnings/error_nonTransparentRgbaCollapsedToTransparentBgr";
+  ctx.err.transparencyCollapse = porytiles::WarningMode::ERR;
+  ctx.err.printErrors = false;
+  ctx.compilerConfig.assignAlgorithm = porytiles::AssignAlgorithm::DEPTH_FIRST;
+
+  CHECK_THROWS_WITH_AS(porytiles::drive(ctx), "errors generated during tile normalization", porytiles::PtException);
+  CHECK(ctx.err.errCount == 2);
+}