"by {author}" is a string that will cause a lot of localization-related
problems, which then become much worse when different languages and
levels can also need different fonts:
- If the author name is set to something in English instead of a name,
then it'll come out a bit weird if your VVVVVV is set to a different
language: "de various people", "por various people", etc. It's the
same problem with Discord bots completing "playing" or "watching" in
their statuses.
- Translators can't always fit "by" in two letters, and level creators
have understandably always assumed, and will continue to assume, that
"by" is two letters. So if you have your VVVVVV set to a language that
translates "by" as something long, then:
| by Various People and Others |
...may suddenly show up as something like:
|thorer Various People and Othe|
- "by" and author may need mutually incompatible fonts. For example, a
Japanese level in a Korean VVVVVV needs to be displayed with "by" in
Korean characters and the author name with Japanese characters, which
would need some very special code since languages may want to add
text both before and after the name.
- It's very possible that some languages can't translate "by" without
knowing the gender of the name, and I know some languages even
inflect names in really interesting ways (adding and even replacing
letters in first names, surnames, and anything in between, depending
on gender and what else is in the sentence).
So to solve all of this, the "by" is now replaced by a 10x10 face from
sprites.png, like a :viridian: emote. See it as a kind of avatar next
to a username, to clarify and assert that this line is for the author's
name. It should be a fairly obvious/recognizable icon, it fixes all the
above problems, and it's a bonus that we now have more happy faces in
VVVVVV.
They need to know how wide the text is going to be in a particular
font, so font::string_wordwrap and font::string_wordwrap_balanced now
take a flags argument like all the printing and dimensions-getting
functions. next_wrap and next_wrap_s take a Font* now, they're internal
to Font.cpp so they can take a Font and avoid double flag-parsing. But
if any non-Font.cpp code needs next_wrap/next_wrap_s in the future, I'd
just make a public wrapper that takes a uint32_t flags and passes the
Font* to the internal functions.
The last two deprecated functions are:
- Graphics::Print
- Graphics::PrintWrap
These are used a lot, but they're relatively easy to replace, since the
only flag I probably have to immediately worry about is PR_CEN. I do
often need to add PR_FONT_* flags but I don't need to add any
PR_2X/PR_3X/PR_4X anymore.
Only three deprecated functions remain:
- Graphics::Print
- Graphics::PrintWrap
- Graphics::bigprint
I also fixed multiline transparent textboxes having their outlines
overlap the text itself, and fixed textboxclass::padtowidth assuming
glyph widths of 8 (it made the hints at the start of intermission 1
run offscreen for example)
Only four deprecated functions remain:
- Graphics::Print
- Graphics::PrintWrap
- Graphics::bigprint
- Graphics::bprint
Graphics::bprint is the least-used one of them, and after that, the
other functions are used a LOT, but it'll be a lot faster to go through
them, since I have less and less flags to worry about. I'll probably
start using Vim macros again like I did for loc::gettext()ing strings,
or maybe I'll automate this completely.
I migrated all of them to font::print, so they can now be removed.
Six deprecated print functions left! (Of which some are used a whole
lot, it's simpler if the lesser-used ones are gone first.)
I migrated all graphics.len calls over to font::len (and also migrated
prints mainly surrounding those graphics.len's) so the old len function
is now completely removed.
Also added some borders to more of the text in room name translator
mode, fixed a positioning issue if the interface font is not 8x8, and
migrated the trophy texts to font::print_wrap (including
PR_COLORGLYPH_BRI that still needed to be done)
Activity zones need to be in the interface font if the message is from
the system (like Press ENTER to activate terminal, which may be in a
different language) and in the level font if it's a customized message
(setactivitytext).
Graphics::drawtextbox was counting the textbox width and height in
8x8 characters, even including the borders as characters, so it'd need
to be told what the font for the textbox is, and then probably only the
height needs to be adapted to the font and not the width because that's
adapted to the screen width... So just call Graphics::drawpixeltextbox
directly instead. It was already weird enough how actual cutscene
textboxes called Graphics::drawtextbox with x/8, y/8 before the
previous commit, (when you already have the pixel width and height!)
only to have that be a wrapper for drawpixeltextbox by doing x*8, y*8.
Some textboxes need to be in the level font (like room names, cutscene
dialogue, etc - even in the main game), and some need to be in the
interface font (like when you collect a shiny trinket or crewmate). So
most of these textboxes now have graphics.textboxprintflags(font_flag)
as appropriate.
RoomnameTranslator.cpp is now also migrated to the new print system -
in room name translator mode, the room name is now displayed in the 8x8
font if it's untranslated and the level font if it is.
Level text such as room names, text box content, and the contents of
the script editor need to be displayed in the level-specific font, and
tweaked to look right. This involves displaying less lines in the
script editor, making text boxes bigger, displaying some text higher
and some text lower. This is still unfinished, but it's the real start
of a migration to font::print functions!
The following functions were moved directly:
- next_wrap
- next_wrap_s
- string_wordwrap
- string_wordwrap_balanced
- string_unwordwrap
These ones will probably still need get a flags argument, except for
string_unwordwrap (since they need to know what font we're talking
about.
The implementation of graphics.len has also been moved to Font.cpp,
but graphics.len still exists for now and is deprecated.
graphics.PrintWrap is now also deprecated. An advantage of the new
version (with flags) is that it'll be possible to do things like put
a border around wrapped text, wrap text at larger scales, etc, but
these things don't work perfectly yet.
This commit also has some other fixes, like the default advance of
6 pixels for characters 0x00-0x1F in 8x8 fonts.
There has always been a mess of different print functions that all had
slightly different specifics and called each other:
Print(x, y, text, r, g, b, cen)
nothing special here, just does what the arguments say
PrintAlpha(x, y, text, r, g, b, a, cen)
just Print but with an alpha argument
PrintWrap(x, y, text, r, g, b, cen, linespacing, maxwidth)
added for wordwrapping, heavily used now
bprint(x, y, text, r, g, b, cen)
prints an outline, then just PrintAlpha
bprintalpha(x, y, text, r, g, b, a, cen)
just bprint but with an alpha argument
bigprint(x, y, text, r, g, b, cen, sc)
nothing special here, just does what the arguments say
bigbprint(x, y, text, r, g, b, cen, sc)
prints an outline, then just bigprint
bigrprint(x, y, text, r, g, b, cen, sc)
right-aligns text, unless cen is given in which case it just
centers text like other functions already do?
bigbrprint(x, y, text, r, g, b, cen, sc)
prints an outline, then just bigrprint
We need even more specifics with the new font system: we need to be
able to specify whether CJK characters should be vertically centered or
stick out on the top/bottom, and we sometimes need to pass in
brightness variables for colored glyphs. And text printing functions
now fit better in Font.cpp anyway. So there's now a big overhaul of
print functions: all these functions will be replaced by font::print
and font::print_wrap (the former of which now exists). These take flags
as their first argument, which can be 0 for a basic left-aligned print,
PR_CEN for centered text (set X to -1!!!) PR_BOR for a border (instead
of functions like bprint and bigbprint), PR_2X, PR_3X etc for scaling,
and these can be combined with |.
Some text, for example [Press ESC to return to editor], fades in/out
using the alpha value, which is passed to the print function. In some
other places (like Press ENTER to teleport, textboxes, trophy text...)
text can fade in or out by direct changes to the RGB values. This means
regular color-adjusted white text can change color, but colored button
glyphs can't, since there's no way to know in the print system what the
maximum RGB values of a specific textbox are supposed to be, so the
only thing it can do is draw the button glyphs at full brightness,
which looks bad. Therefore, you can now also pass in the brightness
value via the flags, with PR_COLORGLYPH_BRI(255).
This is still a work in progress, but the existing font system has been
removed and replaced by a new one, in Font.cpp.
Design goals of the new font system include supporting colored button
glyphs, different fonts for different languages, and larger fonts than
8x8 for Chinese, Japanese and Korean, while being able to support their
30000+ characters without hiccups, slowdowns or high memory usage. And
to have more flexibility with fonts in general. Plus, Graphics.cpp was
long enough as-is, so it's good to have a dedicated file for font
storage.
The old font system worked with a std::vector<SDL_Surface*> to store
8x8 surfaces for each character, and a std::map<int,int> to store
mappings between codepoints and vector indexes.
The new system has a per-font collection of pages for every block of
0x1000 (4096) codepoints, that may be allocated as needed. A glyph on
a page contains the index of the glyph in the image (giving its
coordinates), the advance (how much the cursor should advance, so the
width of that glyph) and some flags which would be at least whether the
glyph exists and whether it is colored.
Most of the *new* features aren't implemented yet; it's currently
hardcoded to the regular 8x8 font.png, but it should be functionally
equivalent to the previous behavior. The only thing that doesn't really
work yet is level-specific font.png, but that'll be supported again
soon enough.
This commit also adds fontmeta (xml) support.
Since the fonts folder is mounted at graphics/, there are two main
options for recognizing non-font.png fonts: the font files have to be
prefixed with font (or font_) or some special file extension is
involved to signal what files are fonts. I always had a font.xml in
mind (so font_cn.xml, font_ja.xml, etc) but if there's ever gonna be
a need for further xml files inside the graphics folder, we have a
problem. So I named them .fontmeta instead.
A .fontmeta file looks somewhat like this:
<?xml version="1.0" encoding="UTF-8"?>
<font_metadata>
<width>12</width>
<height>12</height>
<white_teeth>1</white_teeth>
<chars>
<range start="0x20" end="0x7E"/>
<range start="0x80" end="0x80"/>
<range start="0xA0" end="0xDF"/>
<range start="0x250" end="0x2A8"/>
<range start="0x2AD" end="0x2AD"/>
<range start="0x2C7" end="0x2C7"/>
<range start="0x2C9" end="0x2CB"/>
...
</chars>
<special>
<range start="0x00" end="0x1F" advance="6"/>
<range start="0x61" end="0x66" color="1"/>
<range start="0x63" end="0x63" color="0"/>
</special>
</font_metadata>
The <chars> tag can be used to specify characters instead of in a .txt.
The original idea was to just always use the existing .txt system for
specifying the font charset, and only use the XML for the other stuff
that the .txt doesn't cover. However, it's probably better to keep it
simple if possible - having to only have a .png and a .fontmeta seems
simpler than having the data spread out over three files. And a major
advantage: Chinese fonts can have about 30000 characters! It's more
efficient to be able to have a tag saying "now there's 20902 characters
starting at U+4E00" than to include them all in a text file and having
to UTF-8 decode every single one of them.
If a font.txt exists, it takes priority over the <chars> tag, and in
that case, there's no reason to include the <chars> tag in the XML.
But font.txt has to be in the same directory as font.png, otherwise it
is rejected. Same for font.fontmeta. If neither font.txt nor <chars>
exist, then the font is seen as a 2.2-and-below-style ASCII font.
In <special>: advance is the number of pixels the cursor advances after
drawing the character (so the width of the character, without affecting
the grid in the source image), color is whether the character should
have its original colors retained when printed (for button glyphs).
As for <white_teeth>:
The renderer PR has replaced draw-time whitening of sprites/etc
(using BlitSurfaceColoured) by load-time whitening of entire images
(using LoadImage with TEX_WHITE as an argument).
This means we have a problem: fonts have always had their glyphs
whitened at printing time, and since I'm adding support for colored
button glyphs, I changed it so glyphs would sometimes not be whitened.
But if we can't whiten at print time, then we'd need to whiten at load
time, and if we whiten the entire font, any colored glyphs will get
destroyed too. If you whiten the image selectively, well, we need more
code to target specific squares in the image, and it's kind of a waste
when you need to whiten 30000 12x12 Chinese characters when you're only
going to need a handful, but you don't know which ones.
The solution: Whitening fonts is useless if all the non-colored glyphs
are already white, so we don't need to do it anyway! However, any
existing fonts that have non-white glyphs (and I know of at least one
level like that) will still need to be whitened. So there is now a
font property <white_teeth> that can be specified in the fontmeta,
which indicates that the font is already pre-whitened. If not
specified, traditional whitening behavior will be used, and the font
cannot use colored glyphs.
The `point` struct was a relic of ActionScript and was added because of
the Flash 'point' object. However, it seems like Simon Roth didn't
realize that SDL has its own point struct.
With this, `Maths.h` can be un-included from a couple headers, which
exposes the fact that `preloader.cpp` was relying on `Maths.h` being
transitively included from `Graphics.h`.
Ever since VVVVVV was initially ported to C++ in 2.0, it has used surfaces from SDL. The downside is, that's all software rendering. This commit moves most things off of surfaces, and all into GPU, by using textures and SDL_Renderer.
Pixel-perfect collision has been kept by keeping a copy of sprites as surfaces. There's plans for pixel-perfect collision to use masks instead of reading pixel data directly, but that's out of scope for this commit.
- `graphics.reloadresources()` is now called later in `main`, because textures cannot be created without a renderer.
- This commit also removes a bunch of surface functions which are no longer needed.
- This also recaches target textures in certain places for d3d9.
- graphics.images was converted to a fixed-size array.
- fillbox and fillboxabs use SDL_RenderDrawRect instead of drawing an outline using four filled rectangles
- Update my name in the credits
In general, "temp" is a bad name because it could mean anything. In this
case the buffer isn't really temporary and it's only used for drawing
the menu with a certain offset, so I made it use a better name. But also
because I'm going to be adding temporary buffers so I don't want the
names to be confused.
`ct` was used to be a variable that a color was temporarily stored in
before being passed to a draw function. But this is unnecessary and you
might as well just have a temporary of the color directly. I guess this
was the practice used because temporaries were apparently really bad in
Flash.
setcolreal() was added in 2.3 to do basically the same thing (set it
directly from entities' realcol attributes). But it's no longer needed.
Correspondingly, Graphics::setcol has been renamed to Graphics::getcol
and now returns an SDL_Color, and Graphics::huetilesetcol has been
renamed to Graphics::huetilegetcol for the same reason.
Some functions (notably Graphics::drawimagecol and
Graphics::drawhuetile) were relying on the `ct` to be implicitly set and
weren't ever having it passed in directly. They have been corrected
accordingly.
colourTransform is a struct with only one member, a Uint32. The issue
with `Uint32`s is that it requires a bunch of bit shifting logic to edit
the colors. The issue with bit shifting logic is that people have a
tendency to hardcode the shift amounts instead of using the shift amount
variables of the SDL_PixelFormat, which makes it annoying to change the
color masks of surfaces.
This commit fixes both issues by unhardcoding the bit shift amounts in
DrawPixel and ReadPixel, and by axing the `Uint32`s in favor of using
SDL_Color.
According to the SDL_PixelFormat documentation (
https://wiki.libsdl.org/SDL2/SDL_PixelFormat ), the logic to read and
draw to pixels from colors below 32-bit was just wrong. Specifically,
for 8-bit, there's a color palette used instead of some intrinsic color
information stored in the pixel itself. But we shouldn't need that logic
anyways because we don't use colors below 32-bit. So I axed that too.
This makes it so temporary variables have their scopes reduced (if
possible). I also didn't hesitate to fix style issues, such as their
names ("temp" is such a bad name), making them const if possible, and
any code it touched too.
This allows translators to test all text boxes in the scripts. It
doesn't run the scripts themselves - it only shows the basic appearance
of each text box individually, so context may be lost but it's good to
have a way to see any text boxes that might otherwise not be easily
seen because they require specific circumstances to appear.
I wanted to not complicate the system with different string cases (like
cgettext) if possible, and I have been able to keep the main strings a
simple English=Translation mapping thus far, but apparently strings
like "Rescued!" (which are one string in English), have to be
translated for the correct gender in some languages. So this was a good
time to add support for string cases anyway.
It's a number that can be given to a string to specify the specific
case it's used, to disambiguate identical English keys. In the case of
"Rescued!" and "Missing...", male versions of the string are case 1,
female versions are case 2, and Viridian being missing is case 3. Of
course, if a language doesn't need to use different variants, it can
simply fill in the same string for the different cases.
If any other string needs to switch to different cases: distinguish
them in the English strings.xml with the case="N" attribute (N=1 and
higher), sync language files from the translator menu (existing
translations for the uncased string will simply be copied to all cases)
and change loc::gettext("...") to loc::gettext_case("...", 1),
loc::gettext_case("...", 2), etc.
This commit adds most of the code changes necessary for making the game
translatable, but does not yet "unhardcode" nearly all of the strings
(except in a few cases where it was hard to separate added
loc::gettexts from foundational code changes, or all the localization-
related menus which were also added by this commit.)
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
This replaces all calls to SDL_free with a new macro, VVV_free, that
nulls the pointer afterwards. This mitigates any use-after-frees and
also completely eliminates double-frees. The same is done for any
function to free specific objects such as SDL_FreeSurface, with the
VVV_freefunc macro.
No exceptions for any of these calls, even if the pointer is discarded
or zeroed afterwards anyway. Better safe than sorry.
This is a macro rather than a function that takes in a
pointer-to-pointer because such a function would have type issues that
require casting and that's just not safe.
Even though SDL_free and other SDL functions already check for NULL, the
macro has a NULL check for other functions that don't. For example,
FAudioVoice_DestroyVoice does not check for NULL.
FILESYSTEM_freeMemory has been axed in favor of VVV_free because it
functionally does the same thing except for `unsigned char*` only.
Without this, entering in-game and opening the map with missing graphics
will result in a segfault. This is because even if the image doesn't
exist, it's still pushed into the `images` std::vector as a NULL
pointer. And it segfaults because we dereference it (to get things like
their width and height). In contrast, passing them to SDL is fine
because SDL always checks for NULL first.
This removes the magic numbers previously used for controlling the fade
mode, which are really not readable at all unless you already know what
they mean.
0: FADE_NONE
1: FADE_FULLY_BLACK
2: FADE_START_FADEOUT
3: FADE_FADING_OUT
4: FADE_START_FADEIN
5: FADE_FADING_IN
There is also the macro FADEMODE_IS_FADING, which indicates when the
intention is to only check if the game is fading right now, which wasn't
clearly conveyed previously.
I also took the opportunity to clean up the style of any lines I
touched. This included rewriting if-else chains into case-switches,
turning one-liner if-then statements into proper blocks, fixing up
comments, and even commenting the `fademode == FADE_NONE` on the tower
spike checks (which, it was previously undocumented why that check was
there, but I think I know why it's there).
As for type safety, we already get some by transforming the variable
types into the enum. Assignment is prohibited without a cast. But,
apparently, comparison is perfectly legal and won't even give so much as
a warning. To work around this and make absolutely sure I made all
existing comparisons now use the enum, I temporarily changed it to be an
`enum class`, which is a C++11 feature that makes it so all comparisons
are illegal. Unfortunately, it scopes them in a namespace with the same
name as a class, so I had to temporarily define macros to make sure my
existing code worked. I also had to temporarily up the standard in
CMakeLists.txt to get it to compile. But after all that was done, I
found the rest of the places where a comparison to an integer was used,
and fixed them.
As reported by Dav999, Victoria and Vermilion's trophy colors are
swapped again in 2.4. He points to
37b7615b71, 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, 37b7615b71
("Fix surface color masks") was the real fix, and
d271907f8c ("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 d271907f8c.
Fixes#862.
I know earlier I removed the gameScreen extern in favor of using
screenbuffer, but that was only to be consistent. After further
consideration, I have found that it's actually really stupid.
There's no reason to be accessing it through screenbuffer, and it's
probably an artifact of 2.0-2.2 passing stack-allocated otherwise-global
classes everywhere through function arguments. Also, it leads to stupid
bugs where screenbuffer could potentially be NULL, which has already
resulted in various annoying crashes in the past. Although those could
be fixed by simply initializing screenbuffer at the very top of main(),
but, why not just scrap the whole thing anyway?
So that's what I'm doing.
As a nice side effect, I've removed the transitive include of Screen.h
from Graphics.h. This could've been done already since it only includes
it for the pointer anyway, but it's still good to do it now.
VVV_min/max are functions that only operate on ints, and SDL_min/max are
macros that operate on any type but double-evaluate everything.
I know I more-or-less said earlier that SDL_min/max were dumb but I've
changed my mind and think it's better to use them, taking care to make
sure you don't double-evaluate, rather than trying to generate your own
litany of functions with either your own hand-rolled generation macros,
C++ templates, C11 generics, or GCC extensions (that last one you'd
technically use in a macro but it doesn't really matter), all of which
have more downsides than just not double-evaluating.
And the upside of not double-evaluating is that you're disencouraged
from having really complicated single-line min/max expressions and
encouraged to precompute the values beforehand anyway so the final
min/max is more readable. And furthermore you'll notice when you
yourself end up doing double-evaluations anyway. I removed a couple
instances of Graphics::len() being double-evaluated in this commit (as
well as cleaned up some other min/max-using code). Although the only
downside to those double-evaluations was unnecessary computation,
rather than checking the wrong result or having multiple side effects,
thankfully, it's still good to minimize double-evaluations where
possible.
I noticed when going frame-by-frame in Vertigo that sometimes the
wrapping enemies at the top sometimes just "popped" in frame. This is
because the sprite warp code only draws the warping sprite of sprites at
the bottom of the screen if they're below y=210. However, the warp point
starts at y=232, and warp sprites can be at most 32x32, which is exactly
the case with the Vertigo sprites, which are exactly 32x32. So the warp
code should start warping sprites if they're below y=200 (232 - 32)
instead.
Horizontal warping also has this problem; it warps at x=320 and
starts drawing warp sprites at x=300, even though it should start
drawing at x=288 (320 - 32). I've gone ahead and fixed that as well.
Whew, look at all those copy-pasted print statements!
Doing this because of the in-game timer feature. The text would
otherwise clash harshly with the timer otherwise. Even with the outline
it still clashes, but at least there's an outline so it's not as harsh.
These warnings are kinda spammy, and they make sense in principle.
vlog_error takes a format string, so passing it an arbitrary string
(even error messages from libraries) isn't a good idea.
In previous versions, the game mistakenly checked the wrong color
channel of sprites, checking the red channel instead of the alpha
channel. I abuse this in some of my levels. Then I broke it when
refactoring masks so the game now no longer checks the red channel but
seems to check the blue channel instead. So re-fix this to the previous
bug, and preserve the previous bug with a comment explaining why.
It now looks more like the FLIP macro in Render.cpp: The y-position is
simply the height of the area the object is being flipped in, minus the
y-position itself, minus the height of the object. So:
flipped_yp = constant - yp - height
This is just a mathematical simplification of the existing statement,
which is:
flipped_yp = yp + 2 * (constant/2 - yp) - height
Using algebra, the 2 distributes into the parentheses, so
flipped_yp = yp + constant - 2 * yp - height
And the two `yp`s add together, so
flipped_yp = constant - yp - height
It's more readable this way.
Also I am using a named constant instead of a hardcoded one.
Otherwise, the text will be in the wrong position compared to normal
mode.
PrintWrap is not used in Flip Mode yet, but it will be used on the map
screen in an upcoming change of mine. The FLIP macro in Render.cpp can't
help us there, since it would need to know the height of the wrapped
text at compile time, when the height is only figured out at runtime
based off of the string (or, well, right _now_ the string _is_ known,
but we are going to merge localization for 2.4, and it's better to
future-proof...), and only PrintWrap itself can figure out the height of
the text. (Or, well, I suppose you could call it from outside the
function, but that's not very separation-of-concernsy style.)
There's really no need to put the y-multiplication in a lookup table.
The compiler will optimize the multiplication better than putting it in
a lookup table will.
To improve readability and to hardcode things less, the new
SCREEN_WIDTH_TILES and SCREEN_HEIGHT_TILES constant names are used, as
well as adding a new TILE_IDX macro to calculate the index of a tile in
a concatenated-rows (row-major in formal parlance) array. Also, tile
numbers are stored in a temporary variable to improve readability as
well (no more copy-pasting `contents[i + vmult[j]]` over and over
again).
