In the last commit, I removed having the flip mode conditional directly
inside the sprite-drawing code for each size type, which would reduce
the indentation one level. However, I opted to hold off un-indenting
until this commit, otherwise it would've produced too much noise.
The game uses flipsprites.png instead of sprites.png when in flip mode,
mostly to add exceptions for sprites that SHOULDN'T be flipped in flip
mode.
Looks like to achieve this, the routines for sprite drawing got
copy-and-pasted every single time flipsprites.png needed to be
conditionally used, resulting in large amounts of copy-pasted code. And
this copy-pasted code resulted in copy-paste errors, with relating to
VVVVVV-Man, because apparently due to two copy-pasting errors, the
combined giant crewmate in the epilogue uses flipsprites.png even if you
aren't in flip mode, and it also uses the width instead of the height of
sprites_rect when in flip mode (although, this doesn't end up mattering,
but still).
The solution here is to simply change the referenced sprites vector to a
pointer that can conditionally change based on the game being in flip
mode or not.
Looks like I forgot to test that my music silencing patch didn't break
the music being silent during the "You have found a shiny trinket" and
"You have found a shiny crewmate" text boxes. So I've added a check for
game.completestop in the music handling in main.cpp.
Found this bug while I was testing my towerlogic/gamelogic merge patch.
The problem here is that we're directly using the C stdio library,
instead of using PHYSFS's stuff. So I've added a function
FILESYSTEM_delete() that does exactly that.
This commit fixes a slightly frustrating thing where if you start a new
game, and then exit before saving, "start game" will always take you to
a new game, even though you have unlocked things like the Secret Lab or
Time Trials.
Now, if you select "new game" (only possible if you have something
unlocked), then quit before saving, "start game" will still take you to
the play menu, but "continue" is replaced with "start" and "new game" is
gone.
This will be a useful shorthand to ask "do we have the Secret Lab, or
any Time Trial, or Intermission replays, or No Death Mode, or Flip Mode
unlocked?"
This fixes being able to rack up a large amount of stack frames by
pressing Esc repeatedly in the editor, which would be a problem if you
were to then return to the main menu afterwards.
Instead, if Menu::ed_settings is already in the stack, the game will
simply return to that menu instead of creating it. Else, it will just
create the menu.
Also, as extra attention to detail, I made sure that the menu create or
return only happens if Esc opens the settings menu, and not when Esc is
closes it.
This stabilizes the code that handles the menu that you land on if you
press Esc and quit to the menu.
Instead of using Game::returnmenu(), we now use the new function
Game::returntomenu() to clearly express intent that we want to return to
a specific menu. So I've added another kludge variable
Game::wasinintermission for the was-in-intermission case.
Also, I made it so that if you didn't have a main game telesave or
quicksave, you just get brought back to the main menu. Because you
shouldn't be able to go to the play menu without a quicksave or
telesave.
When exiting from a game-gamestate which may have been entered through a
varying amount of menus, the solution is to not use Game::returnmenu(),
and to instead have a way to go back to a certain given menu.
This commit fixes a bug where the second, third, and fourth tiles of
moving platforms would render offset from the first tile if the moving
platform hit the top or left edge of the screen.
This is due to the fact that SDL_BlitSurface() will end up changing the
coordinates of the rectangle we pass to it to be 0 if they're negative,
but only after it's already been drawn. Previously, we kept re-using the
same rectangle each time we drew each segment of the moving platform,
but since it only changes the draw rectangle after it's already been
drawn, the first tile shows up fine, but not the rest of the tiles,
hence resulting in an offset.
To fix this, we do the same thing as we did for drawing the "really big
sprite" (size-type 9): just reset the rectangle we use every time we
draw a segment of the moving platform.
They're literally the exact same thing, except one is 4 and the other
has an 8. No need to have all that copy-pasted code.
Actually, the only other difference was that size-type 8 set the
drawRect to sprites_rect instead of tiles_rect for some reason? Even
though it doesn't matter, anyway, because SDL_BlitSurface() only cares
about the X and Y you pass it, not the width and height, which is the
only difference between tiles_rect and sprites_rect.
To make sure that people wouldn't wonder where size-type 8 went if they
saw a blank space between size-type 7 and size-type 9, I kept the
size-type 8 conditional, but inside it is just a comment telling you to
go to size-type 2.
Instead of copy-pasting all the BlitSurfaceStandard()s all over again,
just make the referenced vector a pointer that changes depending on
map.custommode.
There's a noticeable seam in the horizontal and warp backgrounds
whenever you enter a new room. Entering a new room triggers the game to
re-draw the entire warp background instead of simply scrolling what it
already has. This seam is the result of the initial background draw
being misaligned with the rest of the scrolling.
If you get out your measuring tools, you'll see that it's misaligned by
exactly 3 pixels (this applies to both horizontal and vertical warping).
If you look at the part of the code where the game draws fresh warping
textures after scrolling the existing ones offscreen, you'll see it
starts with an offset of 317, which is exactly 320 minus 3. And for
vertical, it uses 237, which is exactly 240 minus 3.
This is where the misalignment comes from. Since the incoming textures
are drawn 3 pixels to the left, but the initial draw isn't, this results
in a misalignment and causes the seam.
To fix this, draw the initial draw of the horizontal and vertical warp
backgrounds 3 pixels to the left.
It looks like one bracket got out-of-place for whatever reason. This
doesn't affect the case-switch at all, due to how case-switches work,
but it's still weird to look at.
Indentation has been updated accordingly.
Some custom levels have their own custom music and sync that music to
scripted cutscenes, which is actually pretty impressive. However,
they've always run into a small thorn, which is that you can easily
desync the music by unfocusing the game, because the audio will keep
playing when the game is unfocused.
This should remove that thorn by pausing the audio on unfocus, and
resuming when focused, so that the music can no longer desync, but you
can still pause the game by unfocusing it.
This is yet another feature in VCE that hasn't been upstreamed until
now.
It looks like this variable was originally intended to keep track of th
volume of the game, but then it was used as a boolean in main.cpp to
make sure the game didn't call Mix_Volume() and Mix_VolumeMusic() every
frame.
However, it is now a problem, because I put the music mute handling code
in the very branch that game.globalsound protects against, but since
game.globalsound is here, if I mute the music, then mute the whole game,
then unmute the music, and then unmute the whole game, sound effects
will no longer be muted but the music will still be muted, until I mute
and unmute the whole game again. This is annoying and inconsistent, so
I'm removing this check from the 'if (!game.muted)' branch.
Plus, given that the Mix_VolumeMusic() and Mix_Volume() calls happen
every frame if the game is muted anyways, it doesn't seem to be a
problem to call these every frame.
These do basically nothing. The only time they're used is
getGlobalSound() in an if-statement in main.cpp, but all that
if-conditional does is call setGlobalSound() anyway, which is something
that doesn't really have any side effects. So I'm removing these vars to
simplify the code.
This is for people who want to use their own soundtrack while playing
the game, but who don't want to mute the sound effects as well.
This feature was added to VCE, but it was added in the strangest way. It
was made an option in "game options" instead of being a keybind, and I
don't know why.
The environment variable SteamTenfoot corresponds with the game running
in Steam Big Picture mode or SteamOS if it is defined. There's a
certification process for both full controller support and Big Picture
mode, and being able to launch a file window in Big Picture mode is an
instant cert failure.
Have to add some includes and put these behind some ifdefs, of course.
I'm pretty sure FreeBSD and OpenBSD and Haiku are POSIX enough that the
"open" command will work on them, too.
I would've loved to make FILESYSTEM_openDirectoryEnabled a simple bool
instead of a function, but I ran into issues with putting it in the
FileSystemUtils header file, so I'll just make it a function and call it
a day.
This fixes a bug where levels in the levels list duplicate if there's an
invalid file (such as a folder) in the levels directory.
It looks like it happens because we don't free the memory if
PHYSFS_readBytes() encounters an error, even though we should. Then we
get into Undefined Behavior territory and end up reusing memory, and
here it just happens that previously, parsing the entire XML document
for each level file was enough to make the loaded file pointer point to
garbage that would fail the metadata check, but if we optimize it so we
don't parse the entire XML document, it starts reusing memory instead.
To find each individual tag quickly, to optimize levels list loading.
I opted to not read the tags <Created>, <Modified>, and <Modifiers> as
they're actually pretty useless.
Also I've added a tag finder for <MetaData> but it's not meant to be
used directly, it's only used to check that the tag exists.
This turns the implicit-fallthrough warning into a full compile-time
error.
Implicit fallthrough is when you forget a break statement in a
case-switch, thus letting one case fall through into the next case and
causing debugging headaches.
This is different from the good type of fallthrough that you use to have
one case with multiple different names, like so:
case 0:
case 1:
case 2:
In that case, it's obvious that you want to have fallthrough there.
The problem was, if you were in a time trial and quit, it wouldn't go
back to selecting your current time trial. But also if you were in a
custom level and quit, you would still be on the playerworlds menu.
The problem was twofold: first, I simply wasn't doing the custommode
check. But secondly, I couldn't use map.custommode directly, because
whenever you quit the game aggressively hardreset()s everything
immediately when you press ACTION.
There's probably a good reason for that aggressive hardreset(), so I
won't touch that hardreset() in any way. Instead, I had to introduce two
kludge variables wasintimetrial and wasincustommode to Game, and use
those to do the check proper.
This makes it more convenient if you have a large levels directory, as
some people in the VVVVVV custom levels community do.
On the first page, this option will change to be "last page" instead.
Since the addition of another menu option pushes up the list of levels
too close to the selected level data itself, I've had to move the list
of levels down by 4 pixels (but "next page"/"previous page"/"return to
menu" are still in their same position).
This feature was already added to VCE but hasn't been upstreamed until
now.
This also replaces some createmenu()s with returnmenu()s as needed even
when said createmenu()s already didn't go to the main menu.
Now when you exit the level editor, you'll be selecting the "level
editor" option in "play levels", and if you exit from a level you'll
still be selecting that level in the levels list.
Furthermore, regardless of what you're exiting, your cursor position
will be remembered.
This is to not reset your cursor position every time you return on
something. It's also to automatically keep track of which menu was the
previous menu instead of manually hardcoding said previous menu.
You were able to mismatch the color of the quicksave/telesave summary
and the text/background by pressing Esc when in the "continue" menu,
then pressing ACTION on "no, return".
This commit fixes that bug by putting the map.settowercolour(3) inside
the Menu::continuemenu creation code itself. However, since the
Menu::youwannaquit code does map.nexttowercolour() right after it does
the game.createmenu(), we also need to put the map.nexttowercolour()
before the game.createmenu() beforehand so it doesn't mess up the cyan
color that Menu::continuemenu sets.
Additionally, I removed the map.settowercolour() from the input handling
of Menu::play, as it's superfluous.
This marks pressing ACTION on "next page" in the levels list, credits,
pressing ACTION on "continue" in "You have unlocked" menus, and pressing
ACTION on an unlock option in the unlock menu and time trial unlock menu
as being the same menu.
This is to prevent creating unnecessary stack frames when using said
menu options in those menus.
These aren't necessary, the menu will update regardless. There isn't
even such a call for the mouse cursor toggle option, that's how
unnecessary it is.
Unless it's the main menu, or unless it's not the same menu. Whether or
not the menu is the same is left up to the caller, because some menus
could be the same but use different names, so we can't simply
automatically check that the names are different and assume that they
aren't the same menu.
This temp variable isn't used anywhere else, and even if it was it's set
to something every time it's used, so there's no risk of this commit
breaking any backwards compatibility.
I assume it was so a dev could mark the spot where they needed to put in
the analogue toggle, and they found a unique yet easy to remember
sequence of characters to Ctrl+F as a marker.
Looks like it was a remnant from the Flash days, and the "delete your
saves if you want to use slowdown" was a bit too mean so it stopped
being a thing in the C++ version.
Much more stylistic, you don't need to repeat "game.currentmenuname" for
each case, and you don't need to deal with the dangling first "if" that
doesn't have an "else".
I presume it was meant to have the text of the currently-selected menu
option inside it, before the code switched over to using the indice of
the currently-selected menu instead? Would've been more error-prone to
use the text name directly.
Stringly-typed things are bad, because if you make a typo when typing
out a string, it's not caught at compile-time. And in the case of this
menu system, you'd have to do an excessive amount of testing to uncover
any bugs caused by a typo. Why do that when you can just use an enum and
catch compile-time errors instead?
Also, you can't use switch-case statements on stringly-typed variables.
So every menu name is now in the enum Menu::MenuName, but you can simply
refer to a menu name by just prefixing it with Menu::.
Unfortunately, I've had to change the "continue" menu name to be
"continuemenu", because "continue" is a keyword in C and C++. Also, it
looks like "timetrialcomplete4" is an unused menu name, even though it
was referenced in Render.cpp.
I think it's about time that this number be updated, yeah? This isn't to
say that 2.3 is finished or almost finished or anything, this is just to
clearly differentiate that this isn't 2.2.
If you have invincibility mode or slowdown enabled, the game will not
let you select the Secret Lab, Time Trials, or No Death Mode. To make
this clearer, this commit grays out said options if they are disabled
for that reason.
The game won't let you select the Secret Lab if you're in invincibility
mode, probably so you can't set illegitimate Super Gravitron records
just by standing there and doing nothing.
However, for some reason, it'll still let you select the Secret Lab even
if you've slowed down the game. For consistency, let's prevent selecting
the Secret Lab if the game isn't running at fullspeed, too.
I've converted every "else if"-chain in menu render/input code to be a
case-switch, except for the levels list, the "game options" menu
(because it has the MMMMMM menu option which isn't a compile-time
constant), and the "play" menu (because it has the Secret Lab menu
option which also isn't a compile-time option).
I also did NOT convert some case-switches relating to unlocks in
Input.cpp, mostly because they use a system where the "if we have this
unlocked" conditional is a part of the "if this is the current menu
option" conditional, and they use the 'else' branch to play a sad sound
if that "if we have this unlocked" conditional fails.
I've also converted the game.gameframerate and game.crewrescued() "else
if"-chains to be case-switches instead.
There was one level that was indented with 2 spaces instead of 4, which
made everything else look weird. Then some lines were randomly indented
further for no reason.
Doing this before the next commit is done so as to not make the next
commit noisier.
This removes duplicate code that came about as a result of various
possible permutations of menu options, depending on being M&P, having no
custom level support, having no editor support, and having MMMMMM.
The menus with such permutations are the following:
- main menu
- "start game" is gone in MAKEANDPLAY
- "player levels" is gone in NO_CUSTOM_LEVELS
- "view credits" is gone in MAKEANDPLAY
- "game options"
- "unlock play data" is gone in MAKEANDPLAY
- "soundtrack" is gone if you don't have an mmmmmm.vvv file
- "player levels"
- "level editor" is gone in NO_EDITOR
I achieve this de-duplication by clever use of calculating offsets,
which I feel is the best way to de-duplicate the code with the least
amount of work, if a little brittle.
The other options are to (1) put function pointers on each MenuOption
object, which is pretty verbose and would inflate Game::createmenu() by
a lot, (2) switch all game.currentmenuoption checks to instead check for
the text of the currently-selected menu option, which is very
error-prone because if you make a typo it won't be caught at
compile-time, (3) add a unique ID to each MenuOption object that
represents a text but will error at compile-time if you make a typo,
however this just duplicates all the menu option text, which is more
code than was duplicated previously.
So I just went with this one.
I don't know why you would have to have both defined simultaneously, but
now you can.
The compile fail was caused by the fact that if both were defined, then
there would be an expression like this in Map.cpp:
switch (t)
{
case 0:
}
which is an invalid expression.
The solution is to put 'case 0:' into the MAKEANDPLAY ifdef-guard as
well.
Just like I moved the menu ACTION press handler, I'm doing this as well.
It only removes one level of indentation, but it makes titlerender()
easier to understand.
Just like before, I have to put the separate function first, else
titlerender() won't know what it is.
Previously, the code looked something like:
else { if (...) {...} else { if (...) {...} else { etc. } }
And kept indenting every time there was an else-if.
This commit puts all else-ifs on the same indentation level, so it
doesn't slowly push the code to the right.
This takes out 3 indentation levels from the ACTION press handling,
making titleinput() easier to read as a whole.
Unfortunately, we have to put menuactionpress() first, even though I'd
want it the other way around, otherwise titleinput() won't know what it
is.
If we need it (which I don't think we will be anytime soon) we can
always just get it back through source control. Otherwise, it simply
gets in the way.
Firstly, menu options are no longer ad-hoc objects, and are added by
using Game::option() (this is the biggest change). This removes the
vector Game::menuoptionsactive, and Game::menuoptions is now a vector of
MenuOption instead of std::string.
Secondly, the manual tracker variable of the amount of menu options,
Game::nummenuoptions, has been removed, in favor of using vectors
properly and using Game::menuoptions::size().
As a result, a lot of copy-pasted code has been removed from
Game::createmenu(), mostly due to having to have different versions of
menus depending on whether or not we have certain defines, or having an
mmmmmm.vvv file inside the VVVVVV directory. In the old days, you
couldn't just add or remove a menu option conveniently, you had to
shuffle around the position of every other menu option too, which
resulted in lots of copy-pasted code. But now this copy-pasted code has
been de-duplicated, at least in Game::createmenu().
This removes map.numshinytrinkets in favor of using
map.shinytrinkets.size(). Having automatic length tracking is much less
error-prone and less tedious.
It's treated like a bool anyway, so might as well make it one.
This also necessitates updating every single instance where it or an
element inside it is used, too.
Looks like it was here from that arg passing stuff from earlier, as a
workaround to not pass args around. Well, there's no need to create an
extra UtilityClass now either, just use the one in the global namespace.
Previously, it existed solely to count the number of trinkets and
crewmates when loading a level, because we were keeping track of the
amount of them manually, incrementing and decrementing every time a
trinket or crewmate was added or removed, but loading a new level
represented a case that could potentially not be an increment or
decrement.
However, since the amount tracking is now handled automatically, this
function now does nothing, and can be safely removed.
Same principle as removing the separate variable to track number of
collected trinkets. This means it's less error-prone as we're no longer
tracking number of trinkets separately.
In the function that counts the number of trinkets, I would've liked to
have used std::count_if(). However, the most optimal way would require
using a lambda, and lambdas are too new for the C++ standard we're
using. So I just bit the bullet and counted them manually.
game.trinkets is supposed to be correlated with obj.collect, however why
not just count obj.collect directly?
This turns game.trinkets into a function, game.trinkets(), which will
directly count the number of collected trinkets and return it. This will
fix a few corner cases where the number of trinkets can desync with the
actual collection statuses of trinkets.
In order to keep save compatibility with previous versions of VVVVVV,
the game will still write the <trinkets> variable. However, it will not
read the <trinkets> variable from a save file.
It's a bit rude to put the user back at the main menu after toggling
something. Maybe they also wanted to do something else in the menu while
they're toggling MMMMMM, there's no reason to immediately put them back
there.
Whenever you collect a trinket, game.stat_trinkets gets updated (if
applicable) to signify the greatest amount of trinkets you have ever
collected in the game. However, custom levels shouldn't be able to
affect this, as their trinkets are not the same trinkets as the main
game.
It turns out that when the game warps moving platforms, it won't remove
the block from the position before they warped. Eventually, these blocks
will pile up and will never be removed, causing a memory leak.
I noticed that the code for going to the adjacent room when offscreen
and for warping instead if the room is warping was a bit
copy-and-pasted. To clean up the code a bit, there's now 5 separate
checks in gamelogic():
if (map.warpx)
if (map.warpy)
if (map.warpy && !map.warpx)
if (!map.warpy)
if (!map.warpx)
I made sure to preserve the previous weird horizontal warping behavior
that happens with vertical warping (thus the middle one), and to
preserve the frame ordering just in case there's something dependent on
the frame ordering.
The frame ordering is that first it will warp you horizontally, if
applicable, then it will warp you vertically, if applicable. Then if you
have vertical warping only, that weird horizontal warp. Then it will
screen transition you vertically, if applicable. Then it will screen
transition you horizontally, if applicable.
To explain the weird horizontal warp with the vertical warp: apparently
if an entity is far offscreen enough, and if that entity is not the
player, it will be warped horizontally during a vertical warp. The
points at which it will warp is 30 pixels farther out than normal
horizontal warping.
I think someone ran into this before, but my memory is fuzzy. The best I
can recall is that they were probably createentity()ing a high-speed
horizontally-moving enemy in a vertically warping room, only to discover
that said enemy kept warping horizontally.
As a result of the previous commit, textboxclass::clear() is now unused.
textboxclass::firstcreate() was already useless. So remove both those
functions and initialize the values in the textboxclass constructor.
This removes the variables graphics.ntextbox, as well as removing
'active' from each text box object. Thus, all text boxes are really
real, and you don't have to check its 'active' variable.
Something that's slightly annoying is that in order to make the vector
of text boxes be properly used, the text box cannot remove itself.
Because the text box does not know it's in a vector. So move the removal
of the text box to drawgui() instead.
Just like earlier, these are of the form
if (cond1) { if (cond2) { if (cond3) { thing; } } }
and are really annoying to read.
Also this removes the remnants of the 'active' system that have been
replaced with 'if (true)' conditionals in order to not add noise to the
diff.
Previously, it was used in order to clear a block and deactivate it, and
the constructor function simply called clear() in order to not duplicate
code. However, clear() is no longer necessary (just remove the block
from the blocks vector), and so we can put initialization right back in
the constructor function.
It turns out the game engaged in pseudo-UB when removing activity zones,
which got turned into actual UB due to the previous commit.
There were three places where this could happen:
- Pressing ENTER on an activity zone in normal gameplay
- Pressing ENTER on an activity zone in in-editor playtesting (because
the code is duped here)
- Pressing ESC and quitting to menu while standing inside an activity
zone
In all cases, game.activeactivity would still be pointing to a
non-existent activity zone. This activity zone in the previous system
would simply be a block with a false 'active', and in the system where
C++ vectors are used properly, would index past the blocks array.
In fact, it is a bug that when you press ENTER on an activity zone, the
activity zone prompt suddenly turns to black, then immediately
disappears. It was pointing to a block that had its clear() method
called, which is why it was all black, and it was an inactive block!
This commit makes it so pressing ENTER on an activity zone smoothly
fades out the activity zone prompt instead of being sudden black.
This removes the variables obj.nblocks, as well as removing the 'active'
attribute from the block object. Now every block is guaranteed to be
real without having to check the 'active' variable.
Removing a block while iterating now uses the removeblock_iter() macro.
Previously there was an entclass::clear(), and initialization of an
entclass was done by calling clear() in order to not duplicate code. But
now there's no need for an entclass::clear(), and it is in fact unused
(just call entityclass::removeentity() instead), so I'm removing this
function.
I guess these were here earlier when there were 'active' conditionals,
but then I removed those, so now they look weird next to the 'i != j'
conditionals, so I'm removing them.
These would be of the form
if (cond1) { if (cond2) { if (cond3) { thing; } } }
which is really annoying to read and could've been written as
if (cond1 && cond2 && cond3) { thing; }
so that's what I'm fixing here.
There will be another commit later that fixes this but in places related
to blocks.
Not sure why this function is here. It makes sense if you know that the
game will only do certain moving platform things if you already have a
moving platform in the room, however apparently this function has
absolutely nothing to do with it.
This function's sole purpose was to make sure obj.nentity was in sync,
and that obj.nentity-1 pointed to the last 'active' entity in
obj.entities. But now that obj.nentity is removed and we use
obj.entities.size() instead, it is no longer necessary.
In the previous commit, if an if-statement consisted solely of checking
the active attribute of an entity, I temporarily changed it to 'true'
and put a comment to remove it later, because it would add too much
noise to unindent everything in the same commit.
This removes the variables obj.nentity and obj.nlinecrosskludge, as well
as removing the 'active' attribute from the entity class object. Now
every entity you access is guaranteed to be real and you don't have to
check the 'active' variable.
The biggest part of this is changing createentity() to modify a
newly-created entity object and push it back instead of already
modifying an indice in obj.entities.
As well, removing an entity now uses the new obj.removeentity() function
and removeentity_iter() macro.
Also when we switch everything to not use 'active', we'll need this
macro to remove entities while iterating forwards through the vector one
at a time.
Ok, once we switch everything over to not use the 'active' system, it's
easier to read removeentity(t) than it is to read
entities.erase(entities.begin() + t).
This moves the setenemyroom() function onto the entity object itself, so
I can more easily change all 'entities[k].' to 'entity.' in
entityclass::createentity() later.
Additionally, I've had to move the rn() macro from Entity.h to Ent.h, or
else entclass::setenemyroom() won't know what it is.
This moves the setenemy() function onto the entity object itself,
instead of having to give the indice of the entity in obj.entities. This
makes the code more object-oriented so later I can simply change all
'entities[k]' to 'entity.' in entityclass::createentity().
It is an exact duplicate of musicclass::haltdasmusik(), so use that
function instead and update callers. Looks like
musicclass::haltdasmusik() came first, anyway (musicclass::stopmusic()
was only used in editor.cpp).
Looks like musicfade is an unused variable, anyway. I might remove it,
but I have some plans in the future that involve repairing what it was
intended for, so I'll hold off on removing it (and some other unused
variables in Music.cpp) for now.
As discussed earlier, some custom levels have taken advantage of the
fact that songs 0 and 7 loop and also fade in when using PPPPPP while
having an mmmmmm.vvv file present.
The problem is that it would index out-of-bounds if you did this, but
this UB hasn't caused an exception until my change to refactor
script-related vectors by removing their separate length-trackers.
Most of the code was already commented out, and those comments were
removed in earlier commits, but this removes all recording variables
from Game and simplifies the game-gamestate handling in main.cpp a
little bit.
Just a miscellaneous code cleanup.
There's no glitches that take advantage of the previous situation,
namely that 'temp' was a global variable in Logic.cpp and editor.cpp.
Even if there were, it seems like it would easily lead to some undefined
behavior. So it's good to clean this up.
This is the "Behavioral logic", "Basic Physics", and "Collisions with
walls" trio.
They were originally aligned but then I removed global args, thus
misaligning them. So now I'm re-aligning them back again.
Surprisingly not that many. It looks like at one point in development,
damage blocks were created for every single spike in the Tower, before
it was too laggy so they switched to directly checking collision with
the tile instead.
This removes a bunch of commented-out code that was clearly kept from
the Flash version, even though the Flash graphics API is much different
than SDL's. Also removes a bunch of TODOs that either say nothing, or
say something whose meaning has been totally lost to time due to being
completely vague, or something that's already been done and someone
forgot to remove the TODO.
Most of this is telecookie/quickcookie stuff, which was used in the
Flash version, but there's no longer any such thing as a save cookie.
Also one TODO that says to make a function that's now been made.
Unlike, say, the scriptclass i/j/k stuff, these tempstrings are only
purely visual, and there's no known glitches to manipulate them anyway.
Thus I think it's safe to make this cleanup.
It looks like this may have been used earlier in development, judging
from the name, obviously, but right now it seems like it's used as an
error message if a main game level is asked for an invalid room (well,
only two of them - the Lab and Warp Zone). It should probably be
formalized into an error system, if we want to keep teststring, and also
people would never see it anyway because I don't think there's a
reliable and consistent way to trigger loading a non-existent room.
I have seen someone manage to load a non-existent Warp Zone room only
one time, but even then this teststring didn't pop up. So this
teststring doesn't even trigger in the right circumstances.
Also, when it does pop up, as far as I can tell it will stay onscreen,
which is kinda annoying. So I'm just removing this ancient relic from
the code.
To be fair, it was more on the level of entire functions using different
indentation than the surrounding code, but it's not consistent enough
for me to justify leaving it alone.
Looks like this function was created because editorclass::load() takes
in a string by reference, not by value, and thus mutates it afterwards,
so if you passed a string in when you didn't want it to be mutated, bad
things would happen.
However, a better workaround for the above issue would simply to
duplicate the string and pass that string instead, thus the original
string wouldn't be affected.
To reiterate, I just want to remove the mixed indentation that just
randomly pops up in the middle of a file, because it gets annoying.
Thus, the indentation of a particular piece of code should simply match
the surrounding code. And I consider it completely fine that this file
switches from indenting 4-wide spaces to tabs starting from
Graphics::setcol() onwards. I don't think it's worth it to untabify
Graphics::setcol() and below.
This removes all indentation that suddenly switches in the middle of a
function. Most particularly egregious offenses are the ones made by the
person who has 2-wide tabs, but keeps tabbing up to make each
indentation level match up with the 4-wide spaces, so to them (and only
them) it will look just fine, but since by default tabstop is 8-wide,
their lines are pushed off all the way to the right.
This changes something like UtilityClass::String to help.String,
basically. It takes less typing this way, and is a neat effect of having
global args actually be global variables.
Now that it does nothing due to some earlier changes, it's a useless
function that does nothing. (Well, it was already a useless function,
but those earlier changes made it clearer just exactly how useless it
is.) So remove that function and remove all its callsites.
'swfStage' gets set to 'stage' in updategraphicsmode() but... that does
absolutely nothing, because they both contain exactly the same thing.
And these variables aren't referenced anywhere else. So I'm removing
both of these variables.
Although it keeps getting set to true and false in various places, it
never once gets checked, essentially deeming it a variable that's used
but does nothing.
The 'r', 'g', and 'b' arguments do absolutely nothing. Except unlike
Graphics::drawtile(), there's only one version of Graphics::drawtile2(),
so just remove those args and update callers.
The 'r', 'g', and 'b' arguments do absolutely nothing, even though
they're used in the version of Graphics::drawtile() that's more used. So
delete the other version without those extra arguments, and then remove
the extra arguments from the remaining version. And then update callers.
This removes all global args in all functions in titlerender.cpp.
Additionally, all 'dwgfx' has been renamed to 'graphics' in that file
(there are a lot of them, as you might guess).
This commit removes the passing around of global args in the logic
functions. Additionally, all 'dwgfx' has been replaced with 'graphics'
in Logic.cpp.
This removes global arg passing from all functions on editorclass.
Callers have been updated correspondingly. Additionally, all 'dwgfx' has
been replaced with 'graphics' in editor.cpp.
This commit removes all global args from the parameters of each function
on the scriptclass object, and updates all places they are called
accordingly. It also changes all instances of 'dwgfx' to 'graphics' in
Script.cpp.
Interestingly enough, it looks like editor.h depended on Script.h's
class define of the musicclass. I've temporarily placed the class define
in editor.h, but by the end of this patchset it'll be gone.
This removes global args from all functions on the Graphics class.
Callers of those functions in other files have been updated accordingly.
Of course, since Graphics.cpp is already in the Graphics namespace,
I do not need to change all 'dwgfx' to 'graphics' in Graphics.cpp.
This commit removes the global args being passed around from the
function args on the mapclass object, as well as updating all callers in
other files to not have those args. Furthermore, 'dwgfx' has been
renamed to 'graphics' in Map.cpp.
This commit removes all global args from functions on the entityclass
object, and updates the callers of those functions in other files
accordingly (most significantly, the game level files Finalclass.cpp,
Labclass.cpp, Otherlevel.cpp, Spacestation2.cpp, WarpClass.cpp, due to
them using createentity()), as well as renaming all instances of 'dwgfx'
in Entity.cpp to 'graphics'.
I've decided to call dwgfx/game/map/obj/key/help/music the "global args".
Because they're essentially global variables that are being passed
around in args.
This commit removes global args from all functions on the Game class,
and deals with updating the callsites of said functions accordingly. It
also renames all usages of 'dwgfx' in Game.cpp to 'graphics', since the
global variable is called 'graphics' now.
Interesting to note, I was removing the class defines from Game.h, but
it turns out that Graphics.h depends on the mapclass and entityclass
defines from Game.h. And also Graphics.h spelled mapclass wrong (it
forgot the "class") so I just decided to use that existing line instead.
This is only temporary and after all is said and done, at the end of
this pull request those class defines will be gone.
This is a refactor that turns the script-related arrays `ed.sb`, and
`ed.hooklist` into C++ vectors (`script.commands` was already a vector, it was
just misused). The code handling these vectors now looks more like idiomatic
C++ than sloppily-pasted pseudo-ActionScript. This removes the variables
`script.scriptlength`, `ed.sblength`, and `ed.numhooks`, too.
This reduces the amount of code needed to e.g. simply remove something from
any of these vectors. Previously the code had to manually shift the rest of
the elements down one-by-one, and doing it manually is definitely error-prone
and tedious.
But now we can just use fancy functions like `std::vector::erase()` and
`std::remove()` to do it all in one line!
Don't worry, I checked and `std::remove()` is in the C++ standard since at least
1998.
This patch makes it so the `commands` vector gets cleared when
`scriptclass::load()` is ran. Previously, the `commands` vector never actually
properly got cleared, so there could potentially be glitches that rely on the
game indexing past the bounds set by `scriptlength` but still in-bounds in the
eyes of C++, and people could potentially rely on such an exploit...
However, I checked, and I'm pretty sure that no such glitch previously existed
at all, because the only times the vector gets indexed are when `scriptlength`
is either being incremented after starting from 0 (`add()`) or when it's
underneath a `position < scriptlength` conditional.
Furthermore, I'm unaware of anyone who has actually found or used such an
exploit, and I've been in the custom level community for 6 years.
So I think it's fine.
Someone being mean could've overwritten the telesaves of unsuspecting
players, or unlocked a bunch of stuff which they shouldn't have for
those players, using things like the telesave() command and gamestates.
To prevent this, return early in Game::savequick(), Game::savetele(),
and Game::unlocknum() if we are in custommode.
Instead of passing the error codes out of the function, just handle the
errors directly as they happen, and fail gracefully if something goes
wrong instead of continuing.
Whenever you would press Alt+Enter, or Alt+F, or on macOS Command+Enter,
or on macOS Command+F, or F11, the game would print this useless error
message to console, every single time: "Error: failed: " and it would
concatenate SDL_GetError() after it, but most of the time SDL_GetError()
is blank, so it would print just that.
Instead, what the fullscreen shortcut will now do is check the result of
the relevant SDL functions, BEFORE it decides to print an error message.
And when it DOES print an error message, it will be less vague and will
say instead "Error: toggling fullscreen failed: <output of
SDL_GetError()>".
This means Screen::ResizeScreen() and Screen::toggleFullScreen() are now
int-returning functions. Ideally, every function interfacing with SDL
would return an error code, but that's too much for this simple patch.
Additionally, I took the opportunity to clean up the surrounding
formatting of the code a bit, most notably dedenting the
keypress-clearing stuff by one tab level, converting the
shortcut-handling code to spaces, and removing commented-out code.
Each check has been put in its own variable, so the final conditional is
more readable, and the ifdef is no longer right smack in the middle of
an if-statement.
Also the control flow has been changed (the "else" has been removed from
the shortcut-checking conditional) so I could put the variables closer
to the actual conditional itself. I don't think it affects anything,
though.
This patch allows the use of pressing F11 to toggle fullscreen, as well as
allowing the use of Right Command when using the Command+Enter/F shortcut on
macOS. Apparently Alt+Enter isn't the only shortcut to toggle fullscreen,
Alt+F also works, which I didn't know before.
I'm adding F11 as a shortcut because it's a far more natural shortcut to
toggle fullscreen than this Alt+Enter or Alt+F business, which seems to be a
relic mimicking some other games and some Microsoft stuff?
I'm also adding RCommand+Enter/F because I see no reason not to. If you can
use RAlt on non-macOS, why can't you use RCommand on macOS, too?
I also cleaned up the formatting relating to the shortcut code, and made sure
the closing parenthesis was outside the ifdef so my text editor wouldn't
highlight the parenthesis inside the non-macOS ifdef-branch as a dangling
closing parenthesis, because it assumes the one in the branch above is the
actual closing parenthesis and doesn't parse macros.
If you Alt+Tabbed while in fullscreen mode, the game would stay in
fullscreen instead of switching to windowed, but there was a chance it
would EITHER use the same internal resolution which would mismatch the
window resolution (don't know when exactly this happens, but still) and
stay being in an actual windowed mode, OR switch between
fullscreen/windowed every other time you re-focused the window, which is
annoying.
Now, whenever you Alt+Tab in fullscreen, the game will be in windowed
mode, and then when you re-focus it will go back to fullscreen.
Consistently.
Previously, the only way to guarantee that the game actually saved your
unlock.vvv after changing an option, was to ensure you pressed ACTION on
the "quit game" menu option.
This makes Alt+F4 graceful in that pressing it will also save
unlock.vvv, as it should. Now truly UN-graceful ways of exiting the
game, such as SIGSEGV, SIGABRT, or pkill -9 or pkill -15 will not save
unlock.vvv, as should be the case.
Text outline is not drawn on roomtext when you're actually playing the
game, so don't draw the outline in the editor, either.
FIQ mistakenly added text outline to roomtext in
ca9f577fc4.
There's an if-else chain that first deals with figuring out if there's
an entity where your left-click happened, and to do this it uses
edentat(), which returns a sentinel value of -1 if there is NOT an
entity where your cursor is.
It's very important to check that the value returned ISN'T -1 before you
start indexing the 'edentity' vector, since if you DO index it with that
-1, it'll result in Undefined Behavior because you're doing an
out-of-bounds array access.
Now, here's what the if-else chain looked like before:
if(tmp==-1 && ed.free(ed.tilex,ed.tiley)==0)
{
...
}
else if(edentity[tmp].t==1)
The bug here is very subtle but it was an easy oversight. Basically, if
'ed.free' ended up not being zero, control flow would jump to the next
"else if" over, which then ends up asking for the -1th index of
'edentity', which is Undefined Behavior.
This undefined behavior has now resulted in a crash on my system after
TerryCavanagh/VVVVVV#172, due it shuffling things around juuuuust enough
such that this UB would end up resulting in a segfault instead of
chugging along and working fine. For me and my system, this meant that
if my first left-click in the editor upon opening the game was me
placing down a tile and not placing down an entity, the game would
crash. But, it would be fine if I first placed down an entity and then
afterwards placed down tiles, because it's UB.
And I'm almost certain this was the cause of the very strange bug where
you couldn't hold down left-click for the foreground-placing tool (but
you COULD for the background-placing tool) that seemed to occur most
often on Windows (TerryCavanagh/VVVVVV#25).
The solution to this is to stick in another conditional in the tree
before any indexing occurs, such that there's no way any other
conditionals with the indexing in the conditional tree could end up
being hit. In summary, the if-else chain looks like this now:
if(tmp==-1 && ed.free(ed.tilex,ed.tiley)==0)
{
...
}
else if(tmp == -1)
{
//Important! Do nothing, or else Undefined Behavior will happen
}
else if(edentity[tmp].t==1)
This turns the array 'edentity' into a proper vector, and removes the need to
use a separate length-tracking variable and manually keep track of the actual
amount of edentities in the level by using the long-winded
'EditorData::GetInstance().numedentities'. This manual tracking was more
error-prone and much less maintainable.
editorclass::naddedentity() has been removed due to now functionally being the
same as editorclass::addedentity() (there's no more
'EditorData::GetInstance().numedentities' to not increment) and for also being
unused in the first place.
editorclass::copyedentity() has been removed because it was only used to shift
the rest of the edentities up manually, but now that we let C++ do all the
hard work it's no longer necessary.
This refactors the roomtext code to (1) not use ad-hoc objects and (2)
not use a separate length-tracking variable to keep track of the actual
amount of roomtext in a room.
What I mean by ad-hoc object is, instead of formally creating a
fully-fledged struct or class and storing one vector containing that
object, this game instead hacks together an object by storing each
attribute of an object in different vectors.
In the case of roomtext, instead of making a Roomtext object that has
attributes 'x', 'y', and 'text', the 'text' attribute of each is stored
in the vector 'roomtext', the 'x' attribute of each is stored in the
vector 'roomtextx', and the 'y' attribute of each is stored in the
vector 'roomtexty'. It's only an object in the sense that you can grab
the attributes of each roomtext by using the same index across all three
vectors.
This makes it somewhat annoying to maintain and deal with, like when I
wanted add sub-tile positions to roomtext in VVVVVV: Community Edition.
Instead of being able to add attributes to an already-existing
formalized Roomtext object, I would instead have to add two more
vectors, which is inelegant. Or I could refactor the whole system, which
is what I decided to do instead.
Furthermore, this removes the separate length-tracking variable
'roomtextnumlines', which makes the code much more easy to maintain and
deal with, as the amount of roomtext is naturally tracked by C++ instead
of us having to keep track of the actual amount of roomtext manually.
This fixes a source of undefined behavior, where the int returned by
ss_toi() would be random garbage memory if the string passed into it
would be empty. That's because if the string is empty, there are no
characters to parse, so nothing simply gets put into x.
The easiest way to pass an empty string in to ss_toi() would be to use
script commands with empty arguments.
The problem was that the code seemed to be wrongly copy-pasted from the
code for generating the trinket-found text boxes (to the point where
even the comment for the crewmate-found text boxes didn't get changed
from "//Found a trinket!").
For the trinket-found text boxes, they use y-positions 85 and 135 if not
in flip mode, and y-positions 105 and 65 if the game IS in flip mode.
These text boxes are positioned correctly in flip mode.
However, for the crewmate-found text boxes, they use y-positions 85 and
135 if not in flip mode, as usual, but they use y-positions 105 and 135
if the game IS in flip mode. Looks like someone forgot to change the
second y-position when copy-pasting code around.
Which is actually a bit funny, because I can conclude from this that it
seems like the code to position these text boxes in flip mode was
bolted-on AFTER the initial code of these text boxes was written.
I can also conclude (hot take incoming) that basically no one actually
ever tested this game in flip mode (but that was already evident, given
TerryCavanagh/VVVVVV#140, less strongly TerryCavanagh/VVVVVV#141, and
TerryCavanagh/VVVVVV#142 is another flip-mode-related bug which I guess
sorta kinda doesn't really count since text outline wasn't enabled until
2.3?).
So I fixed the second y-position to be 65, just like the y-position the
trinket text boxes use. I even took the opportunity to fix the comment
to say "//Found a crewmate!" instead of "//Found a trinket!".
Previously, when MMMMMM is installed but the user is using PPPPPP, niceplay would still restart the song even if it's the same. That has been fixed. In addition, Plenary and Path Complete no longer loop when MMMMMM is installed but PPPPPP is in use.
This fixes another way you could end up typing on a non-existent line in
the script editor.
In a script with only 1 line, which is empty, the game would let you
press backspace on it, removing the line. This results in you typing on
a non-existent line.
You will keep typing on it until you either close the script or press
Up. If you press Up, you will be unable to get back to the non-existent
line, for it doesn't exist - but the text you typed on the non-existent
line will still be there, until you close the script and re-open it.
This makes the "[Press ENTER to return to editor]" fade out after a few frames, allowing screenshots of custom levels to be cleaner and to make sure nothing is obscured while the user is editing their level.
This commit also adds alpha support in BlitSurfaceColoured, where it takes into account the alpha of the pixel *and* the alpha of the color.
`graphics::getRGBA(r,g,b,a)` was added to help with this.
Following discussion on TerryCavanagh/VVVVVV#153, I suggested that
instead of reverting my M&P guards from TerryCavanagh/VVVVVV#124 (which
would only revert it for The Final Level, The Lab, Overworld, and The
Tower, leaving Space Station 1 & 2 and The Warp Zone alone which could
potentially cause the same problem that motivated
TerryCavanagh/VVVVVV#153), we should initialize the map data with 0s
instead.
It turns out that the line `tstring=tstring[tstring.size()-1];` also appears once in Scripts.cpp.
This causes the game to segfault after activating a terminal with an empty line at the end of it.
I added a quick `if` around this line, and set `tstring` to an empty string when needed.
In `editor.cpp`, there's a few sections of code that try and index stuff using `string.length()-1`.
This causes issues where if the string is empty, the result is -1, causing undefined behavior.
Flibit fixed a few of these cases, like on line `375` of editor.cpp:
`if((int) tstring.length() - 1 >= 0) // FIXME: This is sketchy. -flibit`
It turns out that one of these weren't caught, over at line `471`.
`tstring=tstring[tstring.length()-1];`
This causes builds compiled on Windows to segfault if you load more than one level in the editor.
I added a quick `if` around it, setting `tstring` to an empty string, which seems to fix the problem.
It's really obvious that screen shaking is not processed in towers if
you bring up the pause menu then quickly quicksave and bring it back
down. The screen won't shake, but it will suddenly start shaking if you
exit the tower, finishing off the stalled screenshake timer.
If you died in (11,7) and a moving platform was to the left of the line
x=152, even if it was moving vertically it would get snapped to x=152,
in custom levels.
Surprised nobody has ran into this before (although people have ran into
the other kludge, which is placing tile 59 at [18,9] if you're in a room
on either the line x=11 or y=7).
Previously, in tower mode, being inside walls would just kill you, unlike
being inside walls outside tower mode, which was somewhat confusing.
Also, spikes behaved differently with regards to invincibility, being
unsolid in towers but solid outside them.
This does not change the behaviour of the "edge" spikes in towers.
The only places where you can die in a room without a room name is the
Overworld, and I feel that calling it Dimension VVVVVV is appropriate.
You can't naturally die in The Ship nor the Secret Lab, and you can only
do it by pressing R, so I didn't feel it appropriate to add checks to
make the hardest room be "The Ship" or "The Secret Lab" if you managed
to get your hardest room to be a room in either of those areas.
If you looked at the "- Press ENTER to Teleport -" text in flip mode,
you might have noticed that the outline looked pretty strange and
glitched-out for some reason. It's just the outline rendering
upside-down. To fix this, make PrintOff() use flipbfont instead of
bfont.
The problem with flipme() was that it WAS properly reflecting
("reflecting" as in mirroring over a given line) the text box over the
line y=120, BUT it forgot to account for the height of the text box.
Thus, the text box position would be off by the length of its own
height. And when the text box got taller, this offset would worsen and
worsen.
In flip mode, warping entities would appear to change color for a brief
moment. This is actually them defaulting to the actual color used in
flipsprites.png, instead of first being whited-out and then re-colored
using graphics.setcol().
To fix this, use BlitSurfaceColoured() and pass `ct` along instead of
using BlitSurfaceStandard().
This is useful for distributions, which may not want to put data.zip in
the same directory as the binary. This can't be distribution-specific
due to the license ("Altered source/binary versions must be plainly
marked as such, and must not be misrepresented as being the original
software.").
If you died in Prize for the Reckless, which is at (11,7), and respawned
in the same room, tile 59 (a solid invisible tile) would be placed at
[18,9] to prevent the moving platform from going back through the
quicksand.
Unfortunately, the way that this kludge was added is poor.
First, the conditional makes it so that it doesn't happen in ONLY
(11,7). Instead of being behind a positive conditional, the tile is
placed in the else-branch of an if-conditional that checks for the
normal case, i.e. if the current room is NOT (11,7), thus being a
negative conditional.
In other words, the positive conditional is "game.roomx == 111 &&
game.roomy == 107". To negate it, all you would have to do is
"!(game.roomx == 111 && game.roomy == 107)".
However, whoever wrote this decided to go one step further, and actually
DISTRIBUTE the negative into both statements. This would be fine, except
if they actually got it right. You see, according to De Morgan's laws,
when you distribute a negative across multiple statements you not only
have to negate the statements themselves, but you have to negate all the
CONJUNCTIONS, too. In other words, you have to change all "and"s into
"or"s and all "or"s into "and"s.
Instead of making the conditional "game.roomx != 111 || game.roomy !=
107", the person who wrote this forgot to replace the "and" with an
"or". Thus, it is "game.roomx != 111 && game.roomy != 107" instead. As a
result, if we re-negate this and take a look at the positive
conditional, i.e. the conditional that results in the else-branch
executing, it turns out to be "game.roomx == 111 || game.roomy == 107".
This ends up forming a cross-shape of rooms where this kludge happens.
As long as your room is either on the line x=11 or on the line y=7, this
kludge will execute.
You can see this if you go to Boldly To Go, since it is (11,13), which
is on the line x=11. Checkpoint in that room, then touch a disappearing
platform, wait for it to fully disappear, then die. Then an invisible
tile will be placed to the left of the spikes on the ceiling.
Anyway, to fix this, it's simple. Just change the "and" in the negative
conditional to an "or".
The second problem was that this kludge was happening in custom levels.
So I've added a map.custommode check to it. I made sure not to make the
same mistake originally made, i.e. I made sure to use an "or" instead of
an "and". Thus, when you re-negate the negative conditional and turn it
into the positive conditional, it reads: "game.roomx == 111 &&
game.roomy == 107 && !map.custommode".
(19,8) is hardcoded to warp on all-sides no matter what. This is fine,
except for the fact that it was doing this in custom levels, too, even
despite the fact that the warp background and color would be overridden
anyway. The only workaround was to add a warp line to the room in custom
levels. I've added a check for custommode so that this won't happen.
The game uses magic values x=-500 and y=-500 to indicate when a text box
should be centered horizontally or vertically. It does this for x=-1
too, but it's buggy because it only looks at the first line of the text
box to center it. In this commit I fix it so that it will look at all of
the lines of the text box to center it instead.
This uses utfcpp combined with a custom font, in the form of a PNG and text file. By default, the game acts exactly as it did before; custom fonts can be provided by third parties.
This fixes a bug where warp lines created through createentity wouldn't
work without there already being a warp line present in the room as an
edentity.
This fixes a bug where if warpdir() was used during in-editor
playtesting, the changed warp direction would persist even when leaving
playtesting.
This would be very annoying to correct back every time you playtested
and warpdir() was used, so I've added some kludge to store the actual
warp direction of each room when entering playtesting, and then set the
warp directions back when leaving playtesting.
This commit makes `help`, `graphics`, `music`, `game`, `key`, `map`, and
`obj` essentially static global objects that can be used everywhere.
This is useful in case we ever need to add a new function in the future,
so we don't have to bother with passing a new argument in which means we
have to pass a new argument in to the function that calls that function
which means having to pass a new argument into the function that calls
THAT function, etc. which is a real headache when working on fan mods of
the source code.
Note that this changes NONE of the existing function signatures, it
merely just makes those variables accessible everywhere in the same way
`script` and `ed` are.
Also note that some classes had to be initialized after the filesystem
was initialized, but C++ would keep initializing them before the
filesystem got initialized, because I *had* to put them at the top of
`main.cpp`, or else they wouldn't be global variables.
The only way to work around this was to use entityclass's initialization
style (which I'm pretty sure entityclass of all things doesn't need to
be initialized this way), where you actually initialize the class in an
`init()` function, and so then you do `graphics.init()` after the
filesystem initialization, AFTER doing `Graphics graphics` up at the
top.
I've had to do this for `graphics` (but only because its child
GraphicsResources `grphx` needs to be initialized this way), `music`,
and `game`. I don't think this will affect anything. Other than that,
`help`, `key`, and `map` are still using the C++-intended method of
having ClassName::ClassName() functions.
This has two benefits:
(1) The game uses less resources when it is asked to gotoroom to the
same room because it is no longer redrawing the warp background
every single frame, which is very wasteful.
(2) The warp background no longer freezes or flickers if the player is
standing inside a gotoroom script box (which calls gotoroom every
frame or every other frame, because every time the gotoroom happens
the script box gets reloaded).
First, two bug fixes. Room text input mode wasn't properly unset
upon pressing Esc, making the prompt get stuck, requiring you to
add roomtext again and finish it to make it go away. Secondly,
escaping script text input would remove the wrong entity.
I also tweaked the handling slightly so that instead of deleting
the entity if it already existed if escaping from text input,
it merely reverts the change in script name/roomtext to what it
was previously.
I considered refactoring the editor text input handler entirely,
but figured such a change would be a bit too extensive for the
purpose of this repository.
Ingame entities are drawn backwards, probably to draw the player on top,
being entity 0 (usually, at least). Make the level editor draw entities
in the same order.
This is to make sure that the room data of those areas are not
distributed with the M&P binary, even if they are already inaccessible
because of other M&P ifdefs (I tested).
When the game enter towermode, it adjusts player amd camera x/y depending
on what screen the player entered it from (The Tower) or the loadlevel
mode ("minitowers"; Panic Room and The Final Challenge). This code didn't
account for respawning to checkpoints. This is unlikely to matter in most
circumstances, but can cause problems in some corner cases, or with R abuse.
This could cause a player to die, respawn outside camera edges and then
immediately die again due to the edge spikes, repositioning the camera
properly. Invincibility would cause further issues, but that's Invincibility
Mode for you -- if this was the only problem I wouldn't bother.
I added a check that repositions the tower camera appropriately if a player
enter a tower as part of the respawn process.
FillRect() is similar enough to memset when blending isn't used, so the
game will take a fast path drawing the roomname background when the
background is opaque.
This is the variable dwgfx.translucentroomname and <translucentroomname>
in unlock.vvv.
This lets you see through the black background of the roomname at the
bottom of the screen, i.e. it makes the roomname background translucent.
So you can see if someone decides to hide pesky spikes there.
The roomname background used to just be a simple SDL_Rect that was drawn
using SDL_FillRect with a color of 0. Unfortunately, it seems that you
cannot use transparent colors with SDL_FillRect, it just defaults to
being fully opaque. However, you CAN draw surfaces with translucency,
which seems like the easiest thing to do. But the first step is to
convert the roomname background to an SDL_Surface.
This replaces the FillRect()s with SDL_BlitSurface() in the three places
roomnames are drawn: in towerrender, in gamerender, and in editorrender.
For some reason, there are two lines that have been copy-pasted the
exact same way and in the exact same place, namely being at the end of
each branch of the if-else conditional, which makes them be executed no
matter what. If they're going to be executed no matter what, we might as
well make it clearer and take those two lines out of each branch.
I ran the game through Valgrind to catch various issues.
One thing caught my attention -- map.resumedelay. This is
used by The Tower/etc to add a delay before the game is
resumed (probably for camera controls delaying it) for
spawning the player. This variable was uninitialized. Notably,
if I explicitly set it to 592851 or similar, it reproduces the
bug, even if I cannot reproduce it with my typical compilation
flags. So fixing this by initializing it to 0, alongside some
other Valgrind warnings, should fix the problem entirely.
* Add a null terminator to loaded TinyXML files
The TinyXML parse() function expect a C-like string, including terminator.
When xml loading was changed, it loaded the file, but included no such thing.
Thus, we load the file, then reallocate the memory so that we can insert a
null terminator to it, before passing it to parse().
* Tweak TinyXML file loading
Instead of first loading the file content into memory, then reallocate it
to add a null pointer, add an argument to the file load function for whether
to append a null terminator or not, defaulting to false. It still returns the
length without the null pointer in case a length ptr is passed.
An earlier change caused TinyXml to prettyprint specifically the
contents of entities (script names and roomtext) a bit more than
before in level files, and added an unusually high amount of whitespace
(particularly, it added an empty line to every entity). This happens
because, for some reason, an empty string is explicitly being added
when creating an entity XML element. The line is so mysterious it feels
like it probably somehow solved a nasty bug long ago and shouldn't be
touched, but it was probably just a mistake, and with all that
whitespace it doesn't look good.
Previously, if it was called in a custom level, it would say "out of
Twenty" no matter what, even if the level had zero trinkets. This commit
fixes that.
Previously, it was a hardcoded list going up to fifty.
This time, it's less hardcoded. Most of the time, it will take a number,
find out its tens-place word, find out its ones-place word, and combine
the two together. There are special cases for the teens, and the numbers
zero through nine, and one hundred.
Also, now if it is given a negative value, it will just return "???"
instead.
If there are more than one hundred it will still say "Lots".
2.2 will handle this just fine, because there are 100 slots allocated
for trinkets and crewmates, and it will save and load all 100 slots just
fine. Except, it only resets the first 20 slots when starting a level
from the beginning, but that's minor and already fixed in 2.3 anyway.
This commit makes it so you can now place up to one hundred trinkets and
crewmates in the editor.
When editorclass::reset() was resetting the contents of the level
previously, it was mixing up the X and Y bounds. The Y bound was
supposed to be 30*maxheight, and the X bound was supposed to be
40*maxwidth. Instead, it took 30*maxwidth as its Y bound and
40*maxheight as its X bound.
Then, when it actually indexes the contents vector to set each tile to
0, it used 30*maxwidth instead of 40*maxwidth.
The difference between width and height is a bit hard to spot, but one
thing you can do to remember the difference is to remember the fact that
X corresponds with width, and Y corresponds with height. Also, rooms are
40 by 30 tiles, and so X (and therefore width) should correspond with
40, and Y (and therefore height) should correspond with 30.
As a result of mixing up the variables, whenever you played a 20x20 map,
quit the level and then started making a new 20x20 map, the tiles of the
last four rows of the previous map would persist, from y=16 (1-indexed)
all the way to y=20 (1-indexed).
I don't recall anyone ever running into this bug before, which is a bit
strange. But if no one truly has ever ran into this bug before, then I'm
genuinely surprised.
While working on the patch to fix the enemy type room property of each
room not getting reset, and testing the fix, I noticed that for some
reason some contents of the previous level I played in order to test the
enemy type property persisting was ALSO persisting alongside the enemy
type property.
Then I read the code and when I realized that the X and Y bounds were
getting mixed up I groaned. Very loudly.
This fixes a bug where if you loaded a level, then started making a new
level in the editor, the enemy types from the previous level would
persist.
While working on VVVVVV: Community Edition and adding a new room
property for enemy speed, I noticed that enemy type was not getting
reset at all. After some testing, I confirmed that this was the case. So
this bug is fixed now.
This fixes a bug where you could get mismatched text and background
colors on the menu screen by being in the Tower and exiting at a certain
time. The background when mismatched will always be red, which seems to
have something to do with the fact that when you enter the Tower the
game always sets the background to be red. I could get a quick repro
setup going by starting in the checkpoint in Teleporter Divot, then
quickly entering the Tower, exiting, then re-entering, then pressing Esc
and quitting - all done very quickly.
The important thing about this mismatch is that it's only temporary, and
will be corrected when you press ACTION and the color of the text and
background in the menu both change at the same time, which is what
map.nexttowercolour() does. So all I do is simply call that function
when exiting to the menu, and it will fix the color mismatch.
Earlier in 53950e14de65a54d9369c5183a16337782d3dc4e, I made playing a
song while a song was already fading quickly fade out the current song,
but then added an exception for if the fade came from the musicfadeout()
command. This commit adds another exception for if the fade came from
niceplay(), since otherwise the music transitions between areas in the
game would go too quick.
This fixes an issue where you could softlock the game if you exited
playtesting mode by pressing Esc while a "- Press ACTION to advance text
-" prompt was onscreen, then re-entered playtesting and then started any
script.
The "- Press ACTION to advance text -" prompt is most directly
controlled by game.advancetext, but when it appears game.pausescript is
usually set as well. You need to have both variables on at the same time
in order to press ACTION. If only advancetext is on, then you won't be
able to flip but can still move around, but if only pausescript is on,
then the game softlocks because the only way you can turn pausescript
off is by pressing ACTION, but the game will only let you press ACTION
if *both* advancetext and pausescript are on.
This fixes a bug where bringing up the pause screen while in a tower
would draw the room you entered BEFORE the tower during the time it took
for the pause screen to be brought up or down.
This fixes a bug in the editor where if you had cutscene bars active
while exiting playtesting, when you re-entered playtesting, the bars
would do their closing animation even though they should be already
gone.
Out-of-bounds array access is Undefined Behavior, which means Bad
Things.
In this particular case, it was indexing an array by using the
`testeditor` variable. Which is fine, except it was indexing that array
*in a conditional that only happens if `testeditor` is -1*. So it was
indexing an array at position -1, which is Out of Bounds and is Not
Good.
For a long time, VVVVVV has had an issue where if you used custom
graphics with translucent pixels (i.e. pixels whose opacity was neither
0% nor 100%, but somewhere between 0% and 100%), those pixels would end
up looking very ugly. They seemed to be overlaid on top of some weird
blue color, instead of actually being translucent.
This bug only occurred when in-game and not in towermode. So, most of
the time, basically. The translucent pixels only displayed properly
inside the Tower, both minitowers, and the editor (when not
playtesting).
I traced this down to the use of the magic number 0xDEADBEEF in
Graphics::drawmap() and Graphics::drawfinalmap(). Looks like someone had
fun finding a color. Was it you, simoroth? Anyway, I've changed it so it
simply uses 0 instead. Note that this magic number needs to be changed
for BOTH FillRect() and OverlaySurfaceKeyed(), or else the game's
background will be some random solid color instead of actually being
drawn properly.
There's a long-standing issue where the Direct Mode status of the loaded
custom map in memory never resets properly. So if you load a level with
a certain layout of Direct Mode rooms, that same layout will be
preserved if you start making a new level in the editor. This commit
fixes that issue.
Similar to 2ebccbc3e9, there's also a
long-standing bug where if you backspace an empty line (and this time,
the line IS actually already empty, not merely
emptied-earlier-in-the-frame), the game will quickly delete more blank
lines if there are any above the blank line you deleted. Again, this is
annoying too, if you so happen to need to use lots of blank lines.
To fix this, it's simple - just set ed.keydelay to 6 when the game
backspaces an empty line. Then it won't be so trigger-happy in deleting
blank lines.
There is a long-standing bug with the script editor where if you delete
the last character of a line, it IMMEDIATELY deletes the line you're on,
and then moves your cursor back to the previous line. This is annoying,
to say the least.
The reason for this is that, in the sequence of events that happens in
one frame (known as frame ordering), the code that backspaces one
character from the line when you press Backspace is ran BEFORE the code
to remove an empty line if you backspace it is ran. The former is
located in key.Poll(), and the latter is located in editorinput().
Thus, when you press Backspace, the game first runs key.Poll(), sees
that you've pressed Backspace, and dutifully removes the last character
from a line. The line is now empty. Then, when the game gets around to
the "Are you pressing Backspace on an empty line?" check in
editorinput(), it thinks that you're pressing Backspace on an empty
line, and then does the usual line-removing stuff.
And actually, when it does the check in editorinput(), it ACTUALLY asks
"Are you pressing Backspace on THIS frame and was the line empty LAST
frame?" because it's checking against its own copy of the input buffer,
before copying the input buffer to its own local copy. So the problem
only happens if you press and hold Backspace for more than 1 frame.
It's a small consolation prize for this annoyance, getting to
tap-tap-tap Backspace in the hopes that you only press it for 1 frame,
while in the middle of something more important to do like, oh I don't
know, writing a script.
So there are two potential solutions here:
(1) Just change the frame ordering around.
This is risky to say the least, because I'm not sure what behavior
depends on exactly which frame order. It's not like it's key.Poll()
and then IMMEDIATELY afterwards editorinput() is run, it's more
like key.Poll(), some things that obviously depend on key.Poll()
running before them, and THEN editorinput(). Also, editorinput() is
only one possible thing that could be ran afterwards, on the next
frame we could be running something else entirely instead.
(2) Add a kludge variable to signal when the line is ALREADY empty so
the game doesn't re-check the already-empty line and conclude that
you're already immediately backspacing an empty line.
I went with (2) for this commit, and I've added the kludge variable
key.linealreadyemptykludge.
However, that by itself isn't enough to fix it. It only adds about a
frame or so of delay before the game goes right back to saying "Oh,
you're ALREADY somehow pressing backspace again? I'll just delete this
line real quick" and the behavior is basically the same as before,
except now you have to hit Backspace for TWO frames or less instead of
one in order to not have it happen.
What we need is to have a delay set as well, when the game deletes the
last line of a char. So I set ed.keydelay to 6 as well if editorinput()
sses that key.linealreadyemptykludge is on.
For a long time, the script editor has had a bug where it would let you
put the cursor on a nonexistent script line, which would APPEAR to be
the last line of the script... but in reality, it WASN'T the last line
of the script, and in fact, the ACTUAL last line was the line ABOVE the
script.
So, if you typed anything on this nonexistent line, it would appear to
get erased when exiting the script. Thus, people have (erroneously)
misdiagnosed this as the script editor somehow being trigger-happy and
erasing lines when it shouldn't be, when in reality it should've have
let you gone onto that line in the first place!
Hiding the mouse cursor does makes sense in a lot of situations I agree, but it's very much a preference thing, and I don't see a good reason to change the original behavour. Some people (i.e. me) don't like cursors disappearing in windowed mode when you move the cursor over the screen, but most importantly, it makes the editor much less pleasant to use (since you're relying on the 30fps editor cursor box).
I'd be happy to support, say, a setting that allowed you to disable the mouse cursor, or even a 15 second time-out which makes the cursor disappear if not moved (and reappear once moved), but just having it off by default feels wrong to me.
Yikes. Somebody brought this to my attention, I didn't even remember that I'd written it. "Spa" or "Spastic" is kind of a south park esque slang term that used to be pretty common in Ireland, which I used without even thinking about it. It's definitely not something I would say anymore, 10 years on, and it's something I shouldn't have said at the time either. I'm sorry :(
(somebody on twitter was asking me about how much cleaning up of the source code I did before launching this. I think this commit kinda answers that)
* Add entity type attribute checks to getcrewman()
This means that the game is no longer able to target other non-crewmate
entities when it looks for a crewmate.
This has actually happened in practice. A command like
position(cyan,above) could position the text box above a horizontal warp
line instead of the intended crewmate.
This is because getcrewman() previously only checked the rule attributes
of entities, and if their rule happened to be 6 or 7. Usually this
corresponds with crewmates being unflipped or flipped, respectively.
But warp lines' rules overlap with rules 6 and 7. If a warp line is
vertical, its rule is 5, and if it is horizontal, its rule is 7.
Now, usually, this wouldn't be an issue, but getcrewman() does a color
check as well. And for cyan, that is color 0. However, if an entity
doesn't use a color, it just defaults to color 0. So, getcrewman() found
an entity with a rule of 7 and a color of 0. This must mean it's a cyan
crewmate! But, well, it's actually just a horizontal warp line.
This commit prevents the above from happening.
If you're in the room being targeted when a `warpdir()` command is run,
and it sets the warp direction to none (warp dir 0), and if you're in a
Lab or Warp Zone room, the background will be set to the wrong one,
because it will always set the background to the stars-going-left
background, instead of setting it to the Lab background or the
stars-going-up background.
However, this is only a visual glitch, and is a temporary one, because
if you manage to trigger a `gotoroom()` on the room, it will get set to
its proper background.
This commit makes it so if you're in a Lab room, the background gets set
to the Lab background, and if you're in a Warp Zone room, the background
gets set to the stars-going-up background.
There are three map-related vectors that need to be reset in hardreset:
`map.roomdeaths`, `map.roomdeathsfinal`, and `map.explored`. All three
of these vectors use the concatenated rows system, whereby each room is
given a room number, calculated by doing X + Y*20, and this becomes
their index in each vector.
There's a double-nested for-loop to handle resetting all of these, where
the outer for-loop iterates over the Y-axis and the inner for-loop
iterates over the X-axis, but only `map.explored` is properly reset.
That's because it's the only vector that properly indexes using X +
Y*20. The other vectors reset using X, so previously, only the first row
of `map.roomdeaths` and `map.roomdeathsfinal` got reset, corresponding
with only the first row of rooms in both Dimension VVVVVV and Outside
Dimension VVVVVV getting reset.
This commit makes sure that both get reset properly.
There are two main problems with flipgravity():
1. It doesn't work for an already-flipped crewmate.
2. It doesn't work on the player.
This commit addresses both of those issues.
For reference, here is the list of scores for a custom level:
0 - not played
1 - finished
2 - all trinkets
3 - finished, all trinkets
Previously, you could've played a level and finished it and set a score
of 3. Then you could re-play the entire level, but manage to only get a
score of 1, i.e. only finished but not with all trinkets. Then your
score for that level would get set back to 1, which basically nullifies
the time you spent playing that level and getting all of the trinkets.
This commit makes it so your new score will get set only if it is higher
than the previous one.
This disables being able to press M to mute while editing a script in
the script editor. It also disables it in the script list, too, but I'm
hoping no one thinks that's an issue, because I personally don't think
it is.
This fixes a bug where if you died after activating a teleporter prompt
and then respawned in the same room as the teleporter, the teleporter
prompt would go away. Which would be very annoying if you wanted to, you
know, teleport.
You don't even have to R-press to make this happen. "Level Complete!"
and Energize are two teleporter rooms in the main game that have hazards
in them.
I see no reason to set game.activetele to false when dying. For one, it
gets set to false during a gotorom anyway. For another, I couldn't get
the game to activate the teleport prompt in a room without a teleporter
in my testing, mostly because when you touch a teleporter, your
checkpoint is set to that teleporter, so you can't R-press to go to
another room and carry over the teleport prompt.
This uncomments the code for the font outline. Now, you will see the
font outline on text that had it in the Flash version, most notably "-
Press ENTER to Teleport -" and the time trial overlay text.
This also fixes a problem with the commented code, where un-centered
text didn't outline properly.
The game makes sure that the player entity is never destroyed, but in
doing so, it doesn't destroy any duplicate player entities that might
have been created via strange means e.g. a custom level doing a
createentity with t=0.
Duplicate player entities are, in a sense, not the "real" player entity.
For one, they can take damage and die, but when they do they'll still be
stuck inside the hazard, which can result in a softlock. For another,
their position isn't updated when going between rooms. It's better to
just destroy them when we can.
This is similar to what was reverted in 3011911 but this time should use
syntax available even before C++11, tested on Clang with
`-std=c++98 -Wall -pedantic`.
I've never given out my real name online, so I'll sort my username as a
last name. My personal preference would have been to sort the list by
first name instead of last name, since that'd be clearer, plus you
often recognize people by their first name rather than their last name.
The TinyXml functions to load and save files don't properly support
unicode file paths on Windows, so in order to support that properly, I
saw no other option than to do the actual loading and saving via PHYSFS
(or to use the Windows API on Windows and retain doc.LoadFile and
doc.SaveFile on other OSes, but that'd be more complicated and
unnecessary, we already have PHYSFS, right?).
There are two new functions in FileSystemUtils:
bool FILESYSTEM_saveTiXmlDocument(const char *name, TiXmlDocument *doc)
bool FILESYSTEM_loadTiXmlDocument(const char *name, TiXmlDocument *doc)
Any instances of doc.SaveFile(<FULL_PATH>) have been replaced by
FILESYSTEM_saveTiXmlDocument(<VVVVVV_FOLDER_PATH>, &doc), where
<FULL_PATH> included the full path to the saves or levels directory,
and <VVVVVV_FOLDER_PATH> only includes the path relative to the VVVVVV
directory.
When loading a document, a TiXmlDocument used to be created with a full
path in its constructor and doc.LoadFile() would then be called, now a
TiXmlDocument is constructed with no path name and
FILESYSTEM_loadTiXmlDocument(<VVVVVV_FOLDER_PATH>, &doc) is called.
There's now a thin layer of UTF-16 around the WinAPI functions to get
the path to the Documents folder and to create a new directory, so that
account usernames with non-ASCII characters do not result in no VVVVVV
folder being created or used.
Resizing the vector does the same thing that the loops did, it changes
the size for the vector and initializes it with default-constructed
elements (or 0 or its equivalent for POD types).
Where a specific value is needed, it is set with the second
parameter of resize().
The next official VVVVVV build removes 32-bit Linux (like all my other games),
and I need to get rid of the shell script on macOS at some point, so this
basically brings it up to what my other games are doing. Plus, this probably
fixes a bug where someone tries to run their executable away from the root...
Doing this is not necessary as CMake already looks up the default one
correctly and in fact breaks whenever you have multiple Xcode versions
installed so Xcode is not called Xcode.app.
(And it does not respect the default Xcode set using xcode-select)
The behavior now is to respect CMAKE_OSX_SYSROOT if set by the
user on the command line. If it's not set, try to use the hardcoded
path to the 10.9 SDK if it's present. If not, warn about the fact
that a different SDK is used.
- Return in Unlock Play Modes goes to Game Options instead of the main menu
- changing some Graphics Options settings no longer plays two menu select sounds, which was really loud :(
- Return in Game Pad Options plays the menu select sound
- turning off Screen Effects plays the menu select sound
When building on macOS targeting an older version than the version of
the SDK currently used, this prevents accidentally using APIs that are
too new (introduced in macOS versions newer than the deployment target).
Follow-up to #19 which did the change for macOS.
It appears to work as expected on Linux too.
Tested on a distro where XDG_DATA_HOME is undefined by default,
and `PHYSFS_getPrefDir` also resolves to `~/.local/share/VVVVVV/`.
The first organization parameter is unused on Linux and macOS.
Instead use PHYSFS_getPrefDir which does the same than the manual
concatenation done before. The organization name argument is required
but is not used on macOS.
