From e93d8989d3e82e5afc52e09fd7aeae3d41644e64 Mon Sep 17 00:00:00 2001 From: Misa Date: Sat, 12 Feb 2022 00:39:30 -0800 Subject: [PATCH] Revert "Fix Secret Lab Time Trial trophies having wrong colors" As reported by Dav999, Victoria and Vermilion's trophy colors are swapped again in 2.4. He points to 37b7615b71c3a2f44e03c47894383107850812ff, the commit where I fixed the color masks of every single surface to always be RGB or RGBA. It sounded plausible to me, because it did have to do with colors, after all. However, it didn't make sense to me, because I was like, I didn't touch the trophy colors at all after I originally fixed them. After I ruled out the RGBf() function as a confounder, I decided to see whether intentionally reversing the color order in RGBf() to be BGR would do anything, and to my surprise it actually swapped the colors back around and it didn't actually look bad. And then I realized: Swapping the trophy colors between RGB and BGR ordering results in similar colors that still look good, but are simply wrong, but not so wrong that they take on a color that no crewmate uses, so it'd appear as if the crewmates were swapped, when in reality the only thing that was swapped was actually the color order of the colors. Trying to fix this by swapping the colors again, I actively confused colors 33 and 35 (Vermilion and Victoria) with colors 32 and 34 (Vitellary and Viridian), so I was confused when Vermilion and Victoria weren't swapping. Then as a debugging step, I only changed 34 to 32 without swapping 32 as well, and then finally noticed that I was swapping Vitellary and Viridian, because there were now two Vitellarys. And then I was reminded that Vitellary and Viridian were also wrongly swapped since 2.0 as well. And so then I finally realized: The original comments accompanying the colors were correct after all. The only problem was that they were fed into a function, RGBf(), that read the colors backwards, because the codebase habitually changed the color order on a whim and it was really hard to reason out which color order should be used at a given time, so it ended up reading RGB colors as BGR, while it looked like it was passing them through as-is. So what happened was that in the first place, RGBf() was swapping RGB to BGR. Then I came and swapped Vermilion and Victoria, and Vitellary and Viridian around. Then later I fixed all the color masks, so RGBf() stopped swapping RGB and BGR around. But then this ended up swapping the colors of Vermilion and Victoria, and Vitellary and Viridian once again! Therefore, swapping Vermilion and Victoria, and Vitellary and Viridian was incorrect. Or at least, not the fix to the root cause. The root cause would be to swap the colors in RGBf(), but this would be sort of confusing to reason about - at least if I didn't bother to just type the RGB values into an image editor. But that doesn't fix the real issue, which is that the game kept swapping RGB and BGR around in every corner of the codebase. I further confirmed that there was no more RGB or BGR swapping by deleting the plus-one-divide-by-three transformation in RGBf() and seeing if the colors looked okay. Now with the colors being brighter, I could see that passing it straight through looked fine, but intentionally reversing it to be BGR resulted in colors that at a distance looked okay, but were either washed out or too bright. At least finally I could use my 8 years of playing this game for something. So in conclusion, actually, 37b7615b71c3a2f44e03c47894383107850812ff ("Fix surface color masks") was the real fix, and d271907f8c5d84308a3cf9323ac692199b8685a6 ("Fix Secret Lab Time Trial trophies having wrong colors") was the real regression. It's just that the regression came first, but it wasn't really a regression until I did the other fix, so the fix isn't the regression, the regression is... this is hurting my brain. Or the real regression was the friends we made along the way, or something like that. This is the most trivial bug ever caused by the technical debt of those god-awful reversed color masks. --- This reverts commit d271907f8c5d84308a3cf9323ac692199b8685a6. Fixes #862. --- desktop_version/src/Entity.cpp | 8 ++++---- desktop_version/src/Graphics.cpp | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/desktop_version/src/Entity.cpp b/desktop_version/src/Entity.cpp index 501257d2..49ad55de 100644 --- a/desktop_version/src/Entity.cpp +++ b/desktop_version/src/Entity.cpp @@ -1818,21 +1818,21 @@ void entityclass::createentity(int xp, int yp, int t, int meta1, int meta2, int if(game.bestrank[1]>=3) { entity.tile = 186 + meta1; - entity.colour = 33; + entity.colour = 35; } break; case 3: if(game.bestrank[2]>=3) { entity.tile = 184 + meta1; - entity.colour = 35; + entity.colour = 33; } break; case 4: if(game.bestrank[3]>=3) { entity.tile = 184 + meta1; - entity.colour = 30; + entity.colour = 32; } break; case 5: @@ -1846,7 +1846,7 @@ void entityclass::createentity(int xp, int yp, int t, int meta1, int meta2, int if(game.bestrank[5]>=3) { entity.tile = 184 + meta1; - entity.colour = 32; + entity.colour = 30; } break; diff --git a/desktop_version/src/Graphics.cpp b/desktop_version/src/Graphics.cpp index d488d1df..452a0902 100644 --- a/desktop_version/src/Graphics.cpp +++ b/desktop_version/src/Graphics.cpp @@ -2904,7 +2904,7 @@ void Graphics::setcol( int t ) break; //Trophies - //Yellow + //cyan case 30: ct.colour = RGBf(160, 200, 220); break; @@ -2912,11 +2912,11 @@ void Graphics::setcol( int t ) case 31: ct.colour = RGBf(220, 120, 210); break; - //cyan + //Yellow case 32: ct.colour = RGBf(220, 210, 120); break; - //Blue + //red case 33: ct.colour = RGBf(255, 70, 70); break; @@ -2924,7 +2924,7 @@ void Graphics::setcol( int t ) case 34: ct.colour = RGBf(120, 220, 120); break; - //red + //Blue case 35: ct.colour = RGBf(75, 75, 255); break;