Currently, all game-gamestate variables are just ints. This is not
particularly type-safe, in case the number of enums changes. To verify
that all current uses of the game-gamestate variables actually use the
enums, change them to be typed with the enum instead.
(As an aside, we should probably rename this so that it can't be
confused with Terry's state machine that has several different ways to
exploit to warp you to the credits, but that's something to do later.)
You'll note that getting in to the glitchy state of the game (the state
where you could play the game after it had hardreset() called on it)
required the player to quit to menu with ingame_titlemode set to true.
Well, quitting to menu calls hardreset(). So if hardreset() is called
when quitting, then you can no longer preserve ingame_titlemode that
way. This is a bit overkill, but I'm just taking precautions.
The game will now assert if the main menu is created while
ingame_titlemode is true, or if we attempt to load into a mode while
it's true. And if assertions are disabled then it just stops doing it
anyway.
I don't think there's any way to get a glitched ingame_titlemode again,
ever since I removed save data deletion taking you back to the main
menu. But I've had enough bugs with the fact that we more-or-less use
the same state for main menu options and in-game options, and that
glitched ingame_titlemode bug DID just happen, so I'm taking
precautions.
The next commit will add logic that more-or-less quits the whole block
if ingame_titlemode, and instead of adding another layer of indentation
I will just pull this into its own function so we can use a return
statement.
While I was testing deleting data while you were in-game, I noticed that
deleting data gave you all the "Win with less than X deaths" trophies,
even if you never got any of them before deleting data. Well, it turns
out that if you have the best game death count of 0, then you win every
trophy, and if you have the best game death count of -1 then that means
you haven't completed the game yet.
This reset was added in e3bfc79d4a, so at
least it's not in 2.3, but I only have myself to blame for making this
mistake. Whoops.
Going back to the main menu allowed for glitchiness to occur if you
deleted your save data while in in-game options. This meant you could
then load back in to the game, and then quit to the menu, then open the
options and then jump back in-game, exploring the state of the game
after hardreset() had been called on it. Which is: pretty glitchy.
For example, this meant having your room coordinates be 0,0 (which is
different from 100,100, which is the actual 0,0, thanks for the
100-indexing Terry), which caused some of the room transitions to be
disabled because room transitions were disabled if the
game.door_up/down/left/right variables were -2 or less, and they were
computed based on room coordinates, which meant some of them went
negative if you were 0,0 and not 100,100. At least this was the case
until I removed those variables for, at best, doing nothing, and at
worst, being actively harmful.
Anyways, so deleting your save data now just takes you back to the
previous menu, much like deleting custom level data does. I don't know
why deleting save data put you back on the main menu in the first place.
It's not like the options menu needed to be reloaded or anything. I
checked and this was the behavior in 2.0 as well, so it was probably
added for a dumb reason.
I considered prohibiting data deletion if you were ingame_titlemode, but
as of the moment it seems to be okay (if albeit weird, e.g. returning to
menu while in Secret Lab doesn't place your cursor on the "play"
button), and I can always add such a prohibition later if it was really
causing problems. Can't think of anything bad off of the top of my head,
though.
Btw thanks to Elomavi for discovering that you could do this glitch.
Okay, so, this is the elephant sprite, right?
https://i.imgur.com/dtS70zk.png
This is how it looks in the actual game, when you stitch all the rooms
together:
https://i.imgur.com/aztVnFT.png
Looks kind of messed-up, doesn't it?
Okay, so, in the bottom two rooms (11,9) and (12,9), the elephant is
placed at y-position -152. But in (11,8) and (12,8), it's placed at
y-position 96. This is despite the fact that -152 plus 240 is 88, not
96.
Similarly, in the left two rooms (11,8) and (11,9), the elephant is
placed at x-position 64, but in the right two rooms (12,8) and (12,9),
the elephant is placed at -264. This is despite the fact that 64 minus
320 is -256, not -264.
All of this stems from the calculations in Otherlevel.cpp using offsets
of -248 and -328 instead of -240 and -320.
So there's an 8-pixel offset that causes the elephant to be chopped off
when viewed with all the rooms stitched together. Simple enough to fix.
For the y-position fixes, I decremented the initial 8-pixel multiplier
as well, else the elephant would sink into the floor.
And this is what the elephant looks like now after stitching:
https://i.imgur.com/27ePLm1.png
Thanks to Tzann for pointing this out.
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.
Dvoid from Discord just reported a crash when trying to load a
custom tiles2.png that was encoded weirdly.
The problem is that we don't check the return value from LodePNG, so
LodePNG gives us a null pointer, and then
SDL_CreateRGBSurfaceWithFormatFrom doesn't check this null pointer,
which then propagates until we crash in SDL_ConvertSurfaceFormat (or
rather, one of its sub-functions), and we would probably crash somewhere
else anyway if it continued.
After properly checking LodePNG's return value, along with printing the
error, it turns out that Dvoid's custom tiles2.png had an "invalid CRC".
I don't know what this means but it sounds worrying. `feh` can read the
file correctly but it also reports a "CRC error".
While we can't fix Canonical, we can at least work around them, and help
people on Ubuntu out by linking them to my comment listing the
currently-known workarounds.
SDL_GetTicks64() is a function that got added in SDL 2.0.18, which is
just an SDL_GetTicks() without a value that wraps every ~49 days,
instead wrapping after the sun explodes and kills us all. Oh sorry,
didn't mean to get existential.
For now, put this behind an SDL_VERSION_ATLEAST guard, which will be
removed when SDL 2.0.18 officially releases and we can update to it.
My latest rebase of #624 (refactoring/splitting editor.cpp) accidentally
overwrote #787 and essentially reverted it entirely. So, add it back in.
This is the same as #787 except it uses the new names, uses SDL_INLINE
to inline the function, and uses named constants.
GCC warns on casting `void*` to function pointers. This is because the C
standard makes a clear distinction between pointers to objects (`void*`)
and pointers to functions (function pointers), and does not specify
anything related to being able to cast object pointers to function
pointers.
The warning message is factually wrong, though - it states that it is
forbidden by ISO C, when in fact it is not, and is actually just
unspecified.
We can't get rid of the cast entirely, because we need the explicit cast
(the C standard _does_ mandate you need an explicit cast when converting
between object pointers and function pointers), and at the end of the
day, this is simply how `SDL_LoadFunction()` works (and more
importantly, how `dlsym()` works), so we can't get rid of it, and we
have no reason to anyways since it means we don't have a hard runtime
dependency on Steam (unlike some other games) and casting `void*` to
function pointers always behaves well on every single platform we ship
on that supports Steam.
Unfortunately, this warning seems to be a part of -Wpedantic, and
there's no way to disable this warning specifically without disabling
-Wpedantic. Luckily, I've found a workaround - just cast to `intptr_t`
before casting to the function pointer. Hopefully the compiler doesn't
get smarter in the future and this ends up breaking or anything...
* Add `setactivityposition(x,y)`, add new textbox color `transparent`
This commit adds a new internal command as a part of the visual activity zone changes I've been making.
This one allows the user to reposition the activity zone to anywhere on the screen.
In addition, this commit adds the textbox color `transparent`, which just sets r, g and b to 0.
rgb(0, 0, 0) normally creates the color black, however in VVVVVV textboxes, it makes the background
of them invisible, and makes the text the off-white color which the game uses elsewhere.
* add new variables to hardreset
* Fix unwanted text centering; offset position by 16, 4
It makes sense for `setactivityposition(0, 0)` to place the activity zone in the default position,
so the x has been offset by 16, and the y has been offset by 4.
Text was being automatically centered, meaning any activity zone which wasn't centered had misplaced text.
This has been fixed by calculating the center manually, and offsetting it by the passed value.
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.
This broke when I was refactoring things earlier, because we no longer
have a direct reference to the contents array, instead using a copied
int. But we have a settile() function anyway, so why not use it?
It is impossible to get on the quicksave screen in time trials, because
Enter is always bound to restarting time trials in a time trial, and
there's no way to open the map screen otherwise.
So, I've decided to add a fun little message in case someone somehow
manages to get to this screen in a time trial.
As is typical, the code was copy-pasted to account for Flip Mode, and
then copy-pasted again to account for custom levels, leading to four
instances of the same code.
I clean this up while also improving code style. This is where the new
FLIP macro and the fixed PrintWrap help a lot - otherwise the "Game
saved ok!" screen would look really wrong without the height
corrections.
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.)
Flipping objects in Flip Mode needs to account for the heights of those
objects (that's why flipme text boxes in Flip Mode in 2.2 were
positioned wrongly).
Also, turn it into a macro instead of an inline function.
This changes the positions of all existing de-duplicated map menu text
in Flip Mode, but it'll be more correct.
I misread SDL's code and thought that SDL's `begin_code.h` was internal
only to SDL. It turns out you get it when you include basically any
header, such as `SDL_stdinc.h`. So use it directly instead of copying it
for our own.
Between accounting for Flip Mode and custom levels, this code was
copy-pasted three times, leading to _four_ instances of one code!
Anyways, I've cleaned it up. The position of the text in Flip Mode is
going to differ by 4 pixels from how it was previously, but that really
shouldn't matter.
While dying in No Death Mode was fixed to no longer say "One trinkets"
in 2.3, if you win in No Death Mode with one trinket, the game would say
"One trinkets".
So to fix this, just slot a ternary in there. The code is already kind
of bad anyways and is going to be refactored/de-STLed in the future
regardless, so I'm not feeling too badly about shoving a ternary in
there like that.
This macro is to make it so it won't be error-prone to write the
semi-confusing `(a % b + b) % b` statement, and you can just use an easy
macro instead.
Currently, the only places a positive modulo is needed is when switching
tilesets, enemies, and warp directions in the editor, as well as when
getting a tile in the tower, since towers just repeat themselves
vertically. Towers used this weird while-loop to sort of emulate a
modulo, which isn't half-bad, but is unnecessary, and I don't think any
compiler would recognize it as a modulo. (And if it's not optimized to a
proper modulo... what happens if the number being moduloed is really,
really big?)
Believe it or not, there are still some remnants of the ActionScript
coding standards in the codebase! And one of them sometimes pops up
whenever an integer division happens.
As it so happens, it seems like division in ActionScript automatically
produces a decimal number. So to prevent that, the game sometimes
subtracts off the remainder of the number to be divided before
performing the division on it.
Thus, we get statements that look like
(a - (a % b)) / b
And probably more parentheses surrounding it too, since it would be
copy-pasted into yet another larger expression, because of course it
would.
`(a % b)` here is subtracting the remainder of `a` divided by `b`, using
the modulo operator, before it gets divided by `b`. Thus, the number
will always be divisible by `b`, so dividing it will mathematically not
produce a decimal number.
Needless to say, this is unnecessary, and very unreadable. In fact, when
I saw these for the first time, I thought they were overcomplicated
_modulos_, _not_ integer division! In C and C++, dividing an integer by
an integer will always result in an integer, so there's no need to do
all this runaround just to divide two integers.
To find all of these, I used the command
rg --pcre2 '(.+?).+?-.+?(?=\1).+?%.+?([\d]+?).+?\/.+?(?=\2)'
which basically matches expressions of the form 'a - a % b / b', where
'a' and 'b' are identical and there could be any characters in the
spaces.
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).
There's really no reason for this simple multiplication plus division to
be in a lookup table. The compiler will optimize it faster than putting
it in a lookup table will, I'm sure.
This comment was referring to a now-deleted variable named mkdirResult
that was binary-"or"ed with all mkdir() results... except for the saves
directory. That variable was only used for save file migration, which is
now axed, so this comment is referring to nothing now.
I don't really know the answer to Ethan's question, but it doesn't
matter now.
So, it turns out freeing everything in binaryBlob::clear() without
checking for NULL results in an abort() because clear() gets called on
musicWriteBlob after it attempts to write the compiled music. It's just
that no one's using VVV_COMPILEMUSIC, so no one's ran into this.
I'm keeping VVV_COMPILEMUSIC around so in the future people can compile
music directly from the game (and probably half the existing
VVV_COMPILEMUSIC code is going to be thrown out, but oh well).
Since this refers to specific exported file data, let's make sure this
is portable. I'm not sure if we'll ever ship on systems where
sizeof(int) != 4 or sizeof(bool) != 1, but better to be safer and
future-proof than not.
This variable is only used when compiling music. Since it doesn't
actually keep track of the number of headers otherwise, ifdef it behind
VVV_COMPILEMUSIC.
2.3 introduced a regression with destroy(platforms). The problem was
that isplatform wasn't being set to false when the entity got disabled,
so if the platform was moving, it would keep moving until it hit a wall,
instead of stopping immediately.
Previously, loading STDIN used std::istreambuf_iterator and std::vector
and whatnot because... I guess it was less typing? But this isn't 1989;
we have the disk space to spare and we don't need to use fancy stuff
just to save on typing. It's not that hard to implement an array that
regrows to the nearest power of two every time.
All system header includes should come before project-specific includes
(includes specific to this game), while coming after the include
specific to the given file (if any; main.cpp doesn't have any).
These are unused.
Ethan originally added them in case Terry wanted achievement
percentages. But he didn't add them, and I don't think the achievements
are changing anytime soon, so it's safe to remove this dead code.
If the string is hardcoded, then use compile-time string literal
concatenation instead.
I don't know if compilers are smart enough to recognize when you're
passing in hardcoded strings and to concatenate them into the string
literal at compile time instead. I also don't know that if compilers are
smart enough to recognize that, that further they recognize all the
logging functions are just wrappers around printf, and so they can
perform the same optimization at those function call sites, too. So it's
better to just do the string concatenation explicitly instead.
Instead of having three separate copies of the function list, use macro
magic to make it so there is only one list that we use in three
different cases.
SDL just got an API to toggle VSync without having to tear down the
renderer ( libsdl-org/SDL#4157 ). We can remove the workaround and use
that instead. For now, we are putting it behind an ifdef until SDL
2.0.18 officially releases in November.
Fixes#831.
Constants.h will house constants like the screen size and others. But
basically only the screen size for now.
Now we don't have to type that "4 bytes per 40 chars (whole screen)"
comment everywhere...
Since those are all downstream recipients of either static storage or
memory that doesn't move for the duration of the custom level, it's okay
to make these be `const char*`s without having to redo any of the RAII
memory management.
mapclass::currentarea() is included in this as well. I also cleaned up
Tower.cpp's headers to fix some transitive includes because I was
removing UtilityClass.h includes from all other level files too.
The "Untitled room" names no longer show any coordinates, because doing
so would require complicated memory management that's completely
unneeded. No one will ever see them, and if they do they already know
they have a problem anyway. The only time they might be able to see them
is if they corrupted the areamap, but this was only possible in 2.2 and
previous by dying outside the room deaths array in Outside Dimension
VVVVVV, which has since been patched out. Besides, sometimes the
"Untitled room" gets overwritten by something else anyway (especially in
Finalclass.cpp), so it really, really doesn't matter.
There's no reason it needs to be an std::string here.
Although, realistically, we should be using an enum instead of
string-typing, but, eh, that can be fixed later.
Companions would not spawn if you didn't load the current room via a
room transition. This meant that companions wouldn't spawn if you loaded
a save file with a companion, at least not until you moved to a
different room and triggered a screen transition. But most importantly,
it meant that the Intermission 1 supercrewmate would never spawn,
because going to Intermission 1 does a straight gotoroom, and does not
do a room transition.
Turns out the roomchange refactor broke things, because of course it
did. The companion logic was implicitly relying on that bool to be set,
because...? Either way, it doesn't make sense. Using roomchange implied
that the code wanted to be ran only when doing a room transition, which
is clearly not the case here. The best thing to do here is to just move
it to a separate function that gets called at the end of
mapclass::gotoroom().
So, I ended up breaking supercrewmate spawning with that roomchange
refactor. However, upon investigating how to fix it, I was running into
a weird interpolation issue due to scmmoveme, as well as the companion
spawning in the ground in "Very Good". And I was wondering why I or no
one else ended up running into them.
Well, as it turns out, scmmoveme ends up doing absolutely nothing. There
are only two instances where scmmoveme is used. The first is if you
respawn in "Very Good", and somehow have your scmprogress set to that
room. But that's impossible, because whenever you respawn, your
scmprogress is always set to the one after the room you respawn in. Even
if you respawned in the room previous to "Very Good" (which is "Don't
Get Ahead of Yourself!"), it still wouldn't work, since the logic always
kicks in when a gotoroom happens, and not only when a supercrewmate is
actually spawned. Since the scmprogress doesn't match, that case never
gets triggered, and we get to the second time scmmoveme is used, which
is in the catch-all case that always executes.
This second instance... also does nothing, because since we just
respawned, and our scmprogress got set to the room ahead of us, there is
no supercrewmate on screen. Then getscm() returns 0, and the player is
always indice 0, so the only thing we end up doing is setting the
player's x-position to their own x-position. Brilliant.
Anyway, this code results in interpolation issues and the supercrewmate
spawning in the ground on "Very Good" if you die, when my fix is
applied, because my fix moves this logic around to a different frame
order, and that actually ends up making scmmoveme no longer dead code.
So to recap: we have dead code, which looks like it does something, but
doesn't. But if you move it around in a certain way, it ends up having
harmful effects. One of the joys of working on this game...
It's also hilarious that it gets saved to the save file. Why? The only
time this variable is true, it is for literally less than a frame,
because it always gets set to false, because you always respawn using a
gotoroom whenever the supercrewmate dies, because you never respawn in
the same room as a supercrewmate, because Intermission 1 was
deliberately designed that way (else you'd keep continually dying since
the supercrewmate wouldn't move out of the way).
These were bfont_rect, bg_rect, foot_rect, and images_rect.
bg_rect was only used once to draw the ghost buffer in the editor, but
that was only because Ally didn't know you could just pass NULL in, cuz
the ghost buffer is the same size as the backbuffer.
RGBflip() does the exact same thing as getRGB(), now that all the
surface masks have been fixed. This axes RGBflip() and changes all
callers to use getRGB() instead. It is more readable that way.
By doing this, there is less copy-pasting. Additionally, it is now
easier to search for RGBf() - which is an ENTIRELY different function
than RGBflip() - now that the name of RGBf is no longer the first four
characters of some different, unrelated function. Previously I would've
had to do `rg 'RGBf[^\w]'` which was stupid and awful and I hated it.
Turns out, the r, g, and b arguments don't actually do anything!
There was a call to RGBf() in the function. RGBf() is just getRGB() but
first adds 128 and then divides by 3 to each of the color channels
beforehand. Unfortunately, RGBf() does not have any side effects, and
the function threw away the return value. Bravo.
This also reveals that the website images drawn in the credits in the
main menu are only recolored because of a stale `ct` set by the previous
graphics.bigprint(), and not because any color values were passed in to
drawimagecol()... What fun surprises the game has in store for me every
day.
This fixes a regression where entering playtesting while a track was
fading out (by exiting out of playtesting with a track playing and then
immediately entering back in with the level start music set) would
result in no music.
The cause is the game doing fades even though nothing is playing, which
puts it in a confusing state.
This wrapper function is for (a) future-proofing (b) proactive
prevention of future copy-pasting (c) to clarify that we never actually
halt music in the SDL_mixer sense, we only pause it, so to check if the
music is halted we actually check if the music is paused instead. This
is important because Mix_PlayingMusic() does not check if the music is
paused and Mix_PausedMusic() does not check if the music is halted.
When you're on the music changing screen in the editor, it plays the
current track. When you return, it stops playing the track. However, if
you press escape, it doesn't stop playing the track. This is because
pressing escape just returns to the previous menu without stopping
playing the track.
To fix this, I just added some kludge in the return menu function. This
is kinda super bad but it works for now and is just something to clean
up later. Maybe like each menu having exit callbacks or something, I
dunno.
This is kinda a regression, kinda sorta not. In 2.2 and previous,
pressing escape would just close the settings menu entirely, which also
bypassed the music fadeout. 2.3 made it so pressing escape doesn't
entirely close the settings menu, and just returns to the previous menu,
which fails in a different way. But the intended way is definitely to
select the return option and having the music fade out.
This function now properly deletes the Super Gravitron record, the Super
Gravitron rank, and the best game deaths. They were not being properly
reset previously, meaning you would have to go into your save file to
properly clean out your save data.
If the map size was less than 20x20, platv values outside the map would
end up being saved as 67372036.
This happens because SDL_memset() operates on the byte level, and not
the multi-byte level. So it takes only the lower 8 bits of 4 and repeats
it for each byte in each integer, creating 67372036.
This was done in 2.2 and previous probably to fix the fact that there
were multiple conflicting audio controls (the player wants to mute the
audio but the game wants to fade in the audio), but is now actively
harmful since 2.3, because muting the game while finishing the
completion prompt means the music will never come back in, even after
unmuting.
I also notice that when collecting a custom crewmate, the game checks
for the level's start music instead of if there's actually a current
song playing right now. I don't know why this was done, because it
would've been better to copy-paste the trinket collection logic here.
It's entirely possible for the audio to just be muted and never come
back if the level has no start music but plays a song by using a script.
Anyways, leaving it alone because it's quite possible that a level might
be intentionally designed around this, I can't really tell the
intentions of every level creator, and it's easy to work around (either
don't use custom crewmates, which every modern level basically does
nowadays, or just set the start music).
For some reason, when completing a custom level and fading to the menu,
the game attempts to fade the music in and also fade the music out at
the same time. This results in nothing happening at all, and in 2.2 and
previous, results in audio fading out from max volume while the game is
frozen on a black screen after the fadeout.
To avoid any potential badness, just remove these.
In the main game, if you press R during the trinket collection prompt
after collecting a trinket, AND you have never entered Comms Relay, and
you respawn in a different room, the trinket collection gamestate will
be interrupted, but you will still be left with the advance text prompt,
cutscene bars, and muted music.
The previous workaround to fix the music would be to mute and then
unmute the game, but due to the new music changes, this workaround
(which in and of itself is a bug) no longer works. Instead, the music
would have to be restarted by going into another zone on the map.
Having an advance text prompt outside of a cutscene results in the
player being unable to flip, but they can still move around left and
right.
Speedrunners previously used the no-Comms-Relay interrupting behavior to
skip certain trinket collection prompts entirely with a frame-perfect R
press, so I can't patch that out. Having an advance text prompt outside
of a cutscene is (ab)used in custom levels to intentionally prevent the
player from flipping, and furthermore, it's also used in credits warp
runs of the main game to increment the gamestate; so I cannot patch that
out. The ability to press R everywhere even during cutscenes was added
for good reason - to make it less likely that a softlock can happen - so
I don't want to revert it.
But I still think this is worth fixing because previously, the
punishment for missing the frame-perfect window late was simply not
skipping the trinket prompt (since the R-press would be ignored), but
now the punishment is basically having to reset because of the advance
text prompt.
I would usually handle this in gamestate 0, but awful custom levels
might want to intentionally interrupt the gamestate to do, I don't know,
something. No level does that so far, but I'd like to do the least
invasive thing.
So what I've done is made it so the effects of interruption are undone
if you press R and the gamestate is interrupted. This is handled in
mapclass::resetplayer().
Without this, `fixedloop` will loop infinitely until focus is regained.
However, Emscripten won't actually know that focus is regained until
`fixedloop` returns.
getBGR, when used in FillRect, was actually passing colors in RGB order.
But now the masks are fixed, so remove it, and fix up all existing
getBGR colors to use getRGB instead.
Due to the mask inconsistencies, getRGB calls that were passed to
FillRect ended up actually being passed in BGR order. But now that the
masks are fixed, all these BGR colors look wrong. So, fix up all of them
(...that's a _lot_ of copy-pasted code...) to be passed in RGB order.
This fixes the color ordering of every SDL_Surface in the game.
Basically, images need to be loaded in ABGR format (except if they don't
have alpha, then you use RGB? I'm not sure what's going on here), and
then they will be converted to RGB/RGBA afterwards.
Due to the surfaces actually being BGR/BGRA, the game used to use
getRGBA/getRGB to swap the colors back around to BGRA/BGR, but I've
fixed those too.
If it's at all possible to use `const std::string&` when passing
`std::string`s around, then we use it. This is to limit the amount of
memory usage as a result of the frequent use of `std::string`s, so the
game no longer unnecessarily copies strings when it doesn't need to.
I've made a new function, Graphics::do_print(), that does the actual
text printing itself. All the interfaces of the other functions have
been left alone, but now just call do_print() instead.
I also removed PrintOffAlpha() and just calculated the center x-position
in bprintalpha() itself (like bigbprint() does) to make it easier to
de-duplicate code.
Text boxes have `r`, `g`, and `b`, and `tr`, `tg`, and `tb`. `tr`, `tg`,
and `tb` are the real colors of the text box, and `r`, `g`, and `b` are
merely the colors of the text box as the text box's alpha value is
applied to them.
Compare this with, say, activity zones (which are drawn like text boxes
but aren't text boxes): There is `activity_r`, `activity_g`, and
`activity_b`, and when they're drawn they're all multiplied by
`act_alpha`.
So just do the same thing here. Ditch the `tr`, `tg`, and `tb`
variables, and make `r`, `g`, and `b` the new `tr`, `tg`, and `tb`
variables. That way, there's simply less state to have to update
separately. So we can get rid of `textboxclass::setcol()` as well.
