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.
