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.
This is a variable that's only used in one method, and it's always
initialized beforehand. No need to carry it around, taking up memory,
and making code analysis more complicated.
All parameters are now made const, to aid in the reader in knowing that
they aren't ever changed.
Useless comments have been removed and been replaced with helpful
comments.
Useless parentheses have been removed.
Spacing has been made consistent.
Declarations and code are no longer mixed.
I'm honestly not too sure why drawcustompixeltextbox ever existed? All
it seemed to do was draw even more horizontal/vertical tiles to finish
any gaps in the tiling... which was all completely unnecessary and
wasteful, because even the previous drawpixeltextbox implementation
covered all gaps in all custom level map sizes that I tried.
Anyway, that at least gets rid of one copy-pasted function.
This draws the remaining horizontal/vertical tile just beside the final
corner if the width/height is not a multiple of 8. (It'd be wasteful to
draw it if the width/height was a perfect multiple of 8, and result in
double-drawing translucent pixels if there were any.)
This has an advantage over the previous system of shifting the
horizontal/vertical tiling, in that custom corner textures don't look
weird due to overlapping like this. Now, custom horizontal/vertical
tiles _can_ look weird if they don't completely tile correctly (or if
they have translucent pixels), but that's better than mucking up the
corners.
`w` and `h` are provided alongside `w2` and `h2`. `w2` and `h2` are in
blocks of 8, while `w` and `h` are in pixels. Therefore, `w2` and `h2`
can just be figured out by diving `w` and `h` by 8.
Also, `xo` and `yo` were used to slide the horizontal/vertical tiling of
the text box a bit into one set of corners, so the horizontal/vertical
tiling wouldn't visibly overlap with the other corners, if using default
textures. This requires hardcoding it for each width/height of text box,
which isn't something that's generalizable. Also, it results in corners
that look weird if the corners have custom textures that don't adhere to
the same shape as default textures.
In the next commit I'll fix the non-multiple-of-8 text box dimensions
differently. Can't do it in this commit or the diff looks weird (at
least with my diff algorithm).
This fixes a bug where the player could bring up the map on the very
first frame of a gamemode(game) animation. This is because the menu
animation checked graphics.menuoffset, but graphics.menuoffset wouldn't
have changed at that point because it only set graphics.resumegamemode.
Instead, just check for graphics.resumegamemode directly. We also need
to assign it to false whenever the map is closed so the player won't be
prevented from using the map screen again.
This fixes all the headaches about map.extrarow having to be the correct
value and which way it should be and whatnot. The latest headache was
the detection that prevent user-initiated menu animations while an
animation was already happening being tripped because
graphics.menuoffset would be 230 (due to closing the menu while being in
a room without a room name), but then going to a room with a room name
would check for 240 instead, and 230 is less than 240. (The numbers are
the wrong way round because I got the ternaries the wrong way round, but
even if the numbers are the correct way round, the bug would still
happen, but it would just be reversed.)
So instead, I've just made it 240 for both. This doesn't change the
duration of the menu animation (because the animation moves in
increments of 25, and 230 / 25 == 240 / 25 under integer division). It
might change the animation slightly, but it was already inconsistent
anyway because map.extrarow was always set to be 1 in custom levels, and
I legitimately would not be able to tell the difference without
recording the animations and nitpicking it frame-by-frame.
Fixes#841.
The player gets kicked out of the Super Gravitron if they have
invincibility or slowdown enabled. However, this can be confusing if no
message pops up
( https://steamcommunity.com/app/70300/discussions/0/3039355280230178910/ )
. So I've made it so that a text box will pop up when they get kicked
out.
This makes it so gamemode(teleporter) will always do an animation, even
if the game is already in TELEPORTERMODE.
I used this script to test:
gamemode(teleporter)
delay(5)
gamemode(teleporter)
delay(5)
gamemode(teleporter)
In 2.2, this script starts the map menu bringing-up animation three
times.
In previous 2.3, this script starts the map menu bringing-up animation
once, but then the next gamemode(teleporter) immediately finishes the
animation, and the third gamemode(teleporter) does nothing.
This commit restores it to 2.2 behavior.
This makes it so it's not even possible to stay on the TELEPORTERMODE
screen by opening the map while it's being brought down. It also makes
it so the map animation is able to be canceled when being brought up
just by opening the map and closing it.
Fixes#833.
This restores it to 2.2 behavior, where the cutscene bars timer also
ticked in TELEPORTERMODE. It was a 2.3 regression that the cutscene bars
timer didn't tick there.
This makes it so if you manage to get stuck in TELEPORTERMODE when a
cutscene ends, the cutscene won't be stuck on untilbars() waiting for
the cutscene bars to go away, since the cutscene bars timer now ticks.
Also, add a sync parameter to avoid calling syncfs too often.
Calling syncfs twice in a row is both inefficient and leads to errors
displaying twice. This allows us to bypass it when saving unlock.vvv as
part of savestatsandsettings.
This object basically had no reason to exist... it was just more verbose
to use, which really reminded me of Java. Anyway, this is the last thing
named after the editor for no reason when it should be a part of the
customlevelclass, so I moved its attributes to customlevelclass.
This fixes the fact that the name of the singular type is plural, but
the name of the plural array is singular. Which has always annoyed me,
too. Also this makes it more clear that custom entities don't have much
to do with the editor.
That's what it is - it's an entity in a custom level. Not something to
do with the editor, necessarily. Like before, the name of the XML
element will remain the same.
That's what edlevelclass is... so that's what it should be named. (Also
removes that "ed", too, making this less coupled to the in-game editor.)
Unfortunately, for compatibility reasons, the name of the XML element
will still remain the same.
CustomLevels.h now uses 4-space indents - like all other space-indented
files - instead of 2-space indents. This has bugged me for a while and I
decided to just fix it now.
This is a pretty hefty commit! But essentially, I made a new editorclass
object, and moved all functions and variables that only get used in the
in-game level editor to that class. This cleanly demarcates which things
are in the editor and which things are just general custom level stuff.
Then I fixed up all the callers. I also fixed up some NO_CUSTOM_LEVELS
and NO_EDITOR ifdefs, too, in several places.
As far as I can tell, this function has never been implemented, and only
existed in this header file. FILESYSTEM_getLevelDirFileNames() already
exists (well, used to exist; it's been changed and renamed to
FILESYSTEM_enumerateLevelDirFileNames()), so I'm removing this now.
This accompanies the editor.cpp -> CustomLevels.cpp change; I'll be
splitting out the editor functions in the next commit. The name of the
include guard has been changed as well, but not anything else.
This moves editorrenderfixed(), editorrender(), editorinput(),
editorlogic(), and their associated functions to a new file named
Editor.cpp - which is exactly what it says on the tin; it stores all the
functions related to the actual in-game editor loop. Also, the existing
editor.cpp has been renamed to CustomLevels.cpp.
All XML functions now check the return value of
tinyxml2::XMLDocument::Error() after each document gets loaded in to
TinyXML-2. If there's an error, then all functions return. This isn't
strictly necessary, but printing the error message that TinyXML-2 is the
bare minimum we could do to be useful.
Additionally, I've standardized the error messages of missing or
corrupted XML files.
Also, the way the game went about making the XML handles was... a bit
roundabout. There were two XML handles, one for the document and one for
the root element - although only one XML handle suffices. So I've
cleaned that up too.
I could've gone further and added error checking for a whole bunch of
things (e.g. missing elements, missing attributes), but this is good
enough.
Also, if unlock.vvv or settings.vvv don't exist yet, the game is
guaranteed to no-op instead of continuing with the function. Nothing bad
seems to happen if the function continues, but the return statements
should be there anyway to clearly indicate intent.
If settings.vvv doesn't exist, loadsettings() calls savesettings(), but
savesettings() already prints a message if settings.vvv doesn't exist.
So then the output would look like
No settings.vvv found. Creating new file
No settings.vvv found
Which is clearly redundant.
The same thing happens with unlock.vvv, but in that case the following
prints instead
No unlock.vvv found. Creating new file
No Stats found. Assuming a new player
I will need to be able to return from this function if there's an XML
error, otherwise writing out the control flow manually gets really
nasty. And while I'm at it, it's some a nice de-duplication as well.
To do this, we create a temporary struct that bundles up all the
information we want for the summary, and pass it in to the intermediate
load function.
Furthermore, we can get rid of reading map.finalstretch - it affects
nothing. map.finalmode is still needed, however, because of the usage of
map.area().
Previously, Flip Mode rendering had to be complicated and allocate
another buffer to call FlipSurfaceVerticle, and it was just a mess.
Instead, why not just do SDL_RenderCopyEx, and let SDL flip the screen
for us? This ends up pretty massively simplifying the rendering code.
`-forcecolor` will force color to be on. `-nocolor` will force color to
be off.
And just because I'm a nice person, I've also added British versions of
those flags. As a treat.
This includes the bold as well.
INFO is just default, WARN is yellow, ERROR is red.
We try to automatically detect if the output is a TTY (and thus supports
colors), and don't emit colors if so. Windows 10 supports ANSI color
codes starting with a specific build, but we don't care to emit whatever
garbage Microsoft invented for builds older than that.
This is because the y-position of the graphics.onscreen() check was a
little too high. Then their name (under Beta Testing) would suddenly
disappear too early. You'd have to look real close to spot it, but it
does happen. It's cuz the credits are all kinda hardcoded, which is
probably bad, but fixing that would have to come later...
I talked with Ethan earlier about this. For 2.3, he wanted me in GitHub
contributors (well, still separate from the rest), to really highlight
the source-code-release community-driven nature of 2.3, but he said it'd
be fine to put me in C++ credits in 2.4.
The RWops stuff isn't a part of any standard PhysFS package (and given
that it explicitly wraps around SDL I'm not sure how you _would_ package
it). So we need to get the physfsrwops.h include in if
BUNDLE_DEPENDENCIES is off, otherwise this results in a compile-time
include-not-found failure.
Additionally, I've placed the PhysFS RWops stuff in their own extras/
folder, so none of the other PhysFS stuff gets included in a
-DBUNDLE_DEPENDENCIES=OFF build.
The game will freeze the player immediately if they release a
directional button within 3 frames of pressing it. Similar to flipping,
this involves global state, and will only apply to the first player
entity.
Closes#484
Flipping only applies momentum to the player entity currently being
processed. This normally wouldn't be a problem. However, flipping
involves global state, and only one flip can occur per frame. This means
that additional player entities don't get this boost of momentum, which
feels somewhat unnatural during gameplay.
This commit fixes this by splitting flip logic out of the loop over
player entities, and applying the flip momentum to all player entities.
We need to check for graphics.setflipmode, not graphics.flipmode,
because graphics.flipmode only gets assigned at the end of the frame
(due to the deferred callback). Otherwise, returning from the options
menu would always turn flag 73 on, which would make you ineligible to
get the Flip Mode trophy, even if you're in Flip Mode.
Originally this started as a "deduplicate a bunch of duplicated code in script commands" PR,
but as I was working on that, I discovered there's a lot more that needs to be done than
just deduplication.
Anything which needs a crewmate entity now calls `getcrewmanfromname(name)`, and anything which
just needs the crewmate's color calls `getcolorfromname(name)`. This was done to make sure that
everything works consistently and no copy/pasting is required. Next is the fallback; instead of
giving up and doing various things when it can't find a specific color, it now attempts to treat
the color name as an ID, and if it can't then it returns -1, where each individual command handles
that return value. This means we can keep around AEM -- a bug used in custom levels -- by not
doing anything with the return value if it's -1.
Also, for some reason, there were two `crewcolour` functions, so I stripped out the one in
entityclass and left (and modified) the one in the graphics class, since the graphics class also
has the `crewcolourreal` function.
If `setactivitytext` was the last line in a script,
the command would index the vector out of bounds.
I also modified the formatting to keep consistent
with the rest of the codebase.
These commands will change the colour and text of the next
activity zone that gets spawned. `setactivitycolour` takes all
textbox colors, and `setactivitytext` will take the text on
the next line. These commands were designed this way
to avoid breaking forwards compatibility.
When an activity zone is spawned through the
use of `createactivityzone`, and `i` is 35,
then it'll change the activity zone text to
"Press ENTER to interact".
On Emscripten, SDL_Delay is implemented as a busy loop. In addition,
everything happens on a single thread. This effectively means that
you have to let Emscripten manage the main loop, since if you do it
yourself the browser will just be frozen.
Otherwise, the new arguments to destroy(), which are 'moving' and
'disappear', would be thrown away by the simplified parser. Let's create
less work for ourselves to do and simply not have a hardcoded list of
allowed arguments for destroy() in the parser.
destroy(platforms) has been bugged since 2.0. The problem with it is
that it removes the platform entity, but doesn't remove its block. This
results in essentially turning the platorm invisible and stopping it
from moving.
This error should be fixed, but some levels (including my own) rely on
the invisible platform trick. So instead, the fixed version will be
implemented under a different name, destroy(moving).
There's also another problem with destroy(platforms), which is that the
name is misleading and it doesn't additionally destroy disappearing
platforms. I would also fix this, but in order to not run the risk of
breakage, it will have to be implemented under a different name, too. So
this will be destroy(disappear). As an added benefit, it's also more
granular to have platform-destroying functions under different names
than it is to consolidate them under the same name.
When I added the two-frame delay fix, I didn't realize that Game had a
roomchange variable that was being used as a temporary variable here.
Now that it's fully spelled out and obvious (just look at the top of
gamelogic()), I realize that the variable exists and is being used, and
other readers will realize it's being used too - so now that I know it
exists, I can axe the screen_transition variable I added in favor of
using roomchange instead.
The purpose of this variable was to keep track of if gamelogic() called
map.gotoroom() at any point during its execution. So map.gotoroom()
always unconditionally set it to true, and then gamelogic() would check
it later.
Well, there's no need to put that in a global variable and do it like
that! It makes it less clear when you do that.
So what I've done instead is made a temporary macro wrapper around
map.gotoroom() that also sets roomchange to true. I've also made it so
any attempt to use map.gotoroom() directly results in failure (and since
then using map.gotoroom() in the wrapper macro would also fail, I've had
to make a gotoroom wrapper function around map.gotoroom() so the wrapper
macro itself doesn't fail).
This is a temporary vector that only gets used in mapclass::gotoroom().
It's always guaranteed to be cleared, so it's safe to move it off.
I'm fine with using references here because, like, it's a C++ STL vector
anyway - when we switch away from the STL (which is a precondition for
moving to C), we'll be passing around raw pointers here instead, and
won't be using references here anyway.
This is a temporary variable that doesn't need to be on Game. It is
guaranteed to be initialized every time mapclass::gotoroom() gets
called, so it's safe to move it off.
Enemy/platform bounds are intended to not be drawn if they cover the
whole screen, since that's what their default bounds are.
However, the code inadvertently made it so if ANY of the bounds touched
a screen edge, the bounds wouldn't be drawn. This is because the
conditionals used "and"s instead of "or"s. The proper way to write the
positive conditional is "x1 is 0 and y1 is 0 and x2 is 320 and y2 is
240", and when you invert that conditional, you need to also invert all
"and"s to be "or"s. This is not the first time that the game developers
failed to properly negate conjunctional statements...
This is to make it so RNG is deterministic when played back with the
same inputs in a libTAS movie even if screen effects or backgrounds are
disabled.
That way, Gravitron RNG is on its own system (seeded in hardreset()),
separate from the constant fRandom() calls that go to visual systems and
don't do anything of actual consequence.
The seed is based off of SDL_GetTicks(), so RTA runners don't get the
same Gravitron RNG every time. This also paves the way for a future
in-built input-based recording system, which now only has to save the
seed for a given recording in order for it to play back
deterministically.
Otherwise, levels could leave stale arguments in the array, and then the
behavior of another level loaded right after might end up being
different because of that.
This is done for consistency with Terry's patrons, which are sorted by
first name and not last.
Also some people go with their usernames and so don't have a last name
to speak of, which ended up being pretty weird.
Kai is my last name. Elizabeth is my middle name. I went with my middle
name as last name for a while before figuring out what I wanted my last
name to be.
Third time's the charm.
The fundamental problem with the previous attempts was that they ended
up saying arguments existed due to stale `words` anyway. So to actually
know if an argument exists or not, we need to assign to `argexists` _as_
we parse the line.
And make sure to take care of that last argument too.
Also I thoroughly tested this this time around. I'm done pulling my hair
out over this.
Ever since tilesheets got expanded, custom levels could use as many
tiles as they wanted, as long as it fit under the 32-bit signed integer
limit.
Until 6c85fae339 happened and they were
reduced to 32,767 tiles.
So I'm being generous again and changing the type of the contents array
(in mapclass and editorclass) back to int. This won't affect the
existing tilemaps of the main game, they'll still stay short arrays. But
it means level makers can use 2 billion tiles once again.
This lets users place down tiles above 1199 in Direct Mode, if their
tilesheet has more than 1200 tiles.
I don't like the copy-pasted code here but it'll have to make do.
If you use Lab tilecol 6, you get the rainbow background. However, this
is unintended, because the associated autotiling is... not very good.
To combat that, Ved disallows using the Lab rainbow background outside
of Direct Mode. We will follow Ved here and only allow switching to the
rainbow background if you're in Direct Mode. Also make sure if someone
is disabling Direct Mode with the rainbow background that it gets reset
properly.
The main game used a set of copy-pasted code to set the music of each
area. There WAS some redundancy built-in, but only three rooms in each
direction from the entrance of a zone.
Given this, it's completely possible for players to mismatch the music
of the area and level. In fact, it's easy to do it even on accident,
especially since 2.3 now lets you quicksave and quit during cutscenes.
Just play a cutscene that has Pause music, then quicksave, quit, and
reload. Also some other accidental ways that I've forgotten about.
To fix this, I've done what mapclass has and made an areamap. Except for
music. This map is the map of the track number of every single room,
except for three special cases: -1 for do nothing and don't change music
(usually because multiple different tracks can be played in this room),
-2 for Tower music (needs to be track 2 or 9 depending on Flip Mode),
and -3 for the start of Space Station 2 (track 1 in time trials, track 4
otherwise).
I've thoroughly tested this areamap by playing through the game and
entering every single room. Additionally I've also thoroughly tested all
special cases (entering the Ship through the teleporter or main
entrance, using the Ship's jukebox, the Tower in Flip Mode and regular
mode, and the start of Space Station 2 in time trial and in regular
mode).
Closes#449.
2.3 has a regression where if you move back and forth between a zone,
you can get the wrong music playing in a zone. An example is the
Overworld and Lab. Just walk in to the Lab and immediately walk back
out, and you'll get Potential for Anything playing in the Overworld.
This regression was caused by facb079b35.
That commit removed assigning -1 to currentsong when a fadeout was
called.
Basically, the previous behavior was: currentsong is 4, we enter Lab and
nicechange gets queued to 3 but currentsong gets set to -1, then going
back nicechange gets queued to 4 again.
However, if we don't assign -1, then going back will keep nicechange at
3. Why? Because niceplay() checks for currentsong before assigning
nicechange. If currentsong is still the same then it doesn't assign
nicechange.
To fix this, just always unconditionally assign nicechange.
If spawned as a custom enemy (createentity entry 56), or spawned outside
of the rooms they spawn in in the main game, they will repeatedly clone
themselves every frame, which profusely leaks memory. In fact it quickly
causes a crash in 2.2 and previous, but 2.3 fixes that crash, so it just
keeps spawning enemies endlessly, which eventually lags the game, and
eventually can out-of-memory your system (bad!).
The problem is those movement types rely on entclass::setenemyroom() to
change their `behave` to be 11 or 13. Else, the new entity created will
still have `behave` 10 or 12, which will create ANOTHER entity in the
same way, and so on, and so forth.
So to fix this, just make it so if an enemy is still `behave` 10 or 12
by the end, then, just set it to -1. That way it'll stay still and won't
cause any harm.
I considered setting the `behave` to 11 or 13 respectively, but, that's
probably going farther than just fixing a memory leak, and anyways, it's
not that much useful for me as a custom level maker, and the entities
spawned aren't really controllable.
In order to let callers provide their OWN callback functions through the
callback function WE provide to PhysFS, we casted the function pointer
to a void pointer.
Unfortunately, this is apparently undefined behavior... if your compiler
doesn't have an extension for it. And most compilers on most
architectures do. (In fact compilers on POSIX systems most certainly
have it due to dlsym() returning a void* which could actually be a
pointer to a function sometimes.)
But imo, it's better to be safe than sorry in this regard. Especially
when given GCC's approach to optimizing int + 100 > int (spoilers: they
remove it entirely! It's faster, but also broken!).
I've decided to wrap it in a struct. And as a nice side effect, if we
ever need more data to be passed through... well we already have this
struct.
Technically, it's also standards-compliant to cast a _pointer to_ a
function pointer to a void pointer. But that extra layer of pointer
indirection would get real confusing to conceptualize real fast (or at
least is more confusing than just putting it in a struct).
Since you've been able to resume music stopped by stopmusic() with
resumemusic(), if a track was stopped by stopmusic(), the unfocus pause
itself would end up resuming the track when regaining focus.
The solution is to simply check for if music.currentsong is -1 or not.
So, platv is a room property that controls the speed of custom entity
platforms in the room (unless, of course, they're created with
createentity). Problem is, this is how 2.2-and-previous coding standards
were:
ed.level[game.roomx-100+((game.roomy-100)*ed.maxwidth)]
Overly long, verbose, not entirely clear unless you already know what it
means? Copy-pasted over and over due to all of the above? Surely a
recipe for not making any coding errors!
Ironically enough, copy-pasting is basically the best approach here
(short of refactoring the whole thing, like I did in
945d5f244a), since if you don't ACTUALLY
copy-paste and just re-type it on your own, you'll end up making more
mistakes. Like what happened here:
ed.level[game.roomx-100+((game.roomy-100)*ed.mapwidth)].platv
Do you see the mistake...? Yeah, mapwidth (with a P) instead of maxwidth
(with an X). You'd have to look closely to find it.
So what does this mean for platv? Well, it means that it multiples the
y-coordinate of the room by the map width instead of the max width (20),
like every other room property. So that means if your map width is less
than 20, like say, map width 10, the platv value for (2,2) will be
stored in (2,1)'s room properties instead of (2,2)'s. Because if you go
off of map width, the room index for (2,2) is 2 + 2 * 10 = 22, but if
you go off of max width, the room index for (2,1) is 2 + 1 * 20 = 22.
Now this wouldn't be bad, except for another 2.2-and-previous
standard... kind of just not exposing things directly to the end user.
Whether that's simply not documenting something (as in the case of
ifwarp and warpdir, which by all measures were completely intended to be
used in custom levels but just simply were never known properly until I
discovered how to use them in 2019), or in this case, not giving any way
for the user to fiddle with platv from the in-game editor. Because if
there was a way to do that, and someone decided to test to see if platv
worked okay, they would discover something was up.
So... since I refactored room properties in
945d5f244a, I kind of broke platv by
fixing it. Now levels that relied on platv being the broken way don't
work.
How do I fix it, and thus break it again? Well, I'll do what I did for
scripts - handle the scrambling when reading and writing the level, and
keep things sane at least internally.
Thus: editorclass::load() will unscramble platv data in the right way,
and editorclass::save() will re-scramble platv in the right way too.
To match the option to nuke all main game save data, there is also now
an option to nuke all custom level save data separately (which is just
all custom level quicksaves, along with stars for level completion). It
has its own confirmation menu too. It does not delete any levels from
the levels folder.
