You're intended to rescue Violet first, and not second, third, or
fourth, and especially not last.
If you rescue her second, third, or fourth, your crewmate progress will
be reset, but you won't be able to re-rescue them again. This is because
Vitellary, Verdigris, Victoria, and Vermilion will be temporarily marked
as rescued during the `bigopenworld` cutscene, so duplicate versions of
them don't spawn during the cutscene, and then will be marked as missing
later to undo it.
This first issue can be trivially fixed by simply toggling flags to
prevent duplicates of them from spawning during the cutscene instead of
fiddling with their rescue statuses.
However, there is still another issue. If you rescue Violet last, then
you won't be warped to the Final Level, meaning you can't properly
complete the game. This can be fixed by adding a `crewrescued() == 6`
check to the Space Station 1 Level Complete cutscene. There is
additionally a temporary unrescuing of Violet so she doesn't get
duplicated during the `bigopenworld` cutscene, and I've had to move that
to the start of the `bigopenworld` and `bigopenworldskip` scripts,
otherwise the `crewrescued() == 6` check won't work properly.
I haven't added hooks for Intermission 1 or 2 because you're not really
meant to play the intermissions with Violet (but you probably could
anyway, there'd just be no dialogue).
Oh, and the pre-Final Level cutscene expects the player to already be
hidden before it starts playing, but if you rescue Violet last the
player is still visible, so I've fixed that. But there still ends up
being two Violets, so I'll probably replace it with a special cutscene
later that's not so nonsensical.
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
For some reason it called obj.getplayer() and did nothing with the
result. Weird. But it does say "Test script for space station" above.
Removing this fixes an 'unused variable' warning.
With the scope of these variables reduced, it makes analyzing this
function easier, as you can now clearly see all temporary variables are
actually initialized before they're used.
The game has four different functions for the same XML schema:
Game::loadtele(), Game::savetele(), Game::loadquick(), and
Game::savequick(). This essentially means one XML schema has been
copy-pasted three different times.
I can at least trim that number down to being copy-pasted only one time
by de-duplicating the reading and writing part. So both Game::loadtele()
and Game::loadquick() now use Game::readmaingamesave(), and
Game::savetele() and Game::savequick() now use
Game::writemaingamesave().
This will make it take less work to add XML forwards compatibility
(#373).
This is a regression from 25779606b4
(PR #74).
On the one hand, I should've thought this through carefully when
implementing the fix for the lower-score overwrite bug. On the other
hand, flibit didn't seem to notice either. And on the third hand, no one
else seems to have noticed, when they have had over 8 months to do so.
Not even the person who originally reported the lower-score overwrite
bug and also reported other bugs I hadn't noticed (e.g. the "You have
rescued a crewmate!" in Flip Mode) noticed this bug, which I believe was
weee50. But to be fair, he does seem to be less active nowadays.
On the fourth hand, I only realized the cause of the duplicate bug after
stepping through it in GDB, instead of just looking at it and going "hey
wait a minute" earlier. I'm surprised it didn't take me longer to
realize the problem.
I'm not sure what all these hands mean anymore, or where I'm getting
extra hands from. Whatever. This regression is fixed now.
When I added the over-30-FPS mode, I kept running into this problem
where the special images of text boxes would render during the
deltaframes of fade-in/fade-out animations, even though they shouldn't
be. So I simply added a flag to the text box that enables drawing these
special images.
However, this doesn't solve the problem fully, and there's still a small
chance that a special-image text box could draw another special image
during its deltaframes. It's really rare and you have to have your
deltaframe luck juuuuuust right (or you could use libTAS, probably), but
it helps to be in 40% slowmode and have a high refresh rate (which, if
it isn't a multiple of 30, you should disable VSync, too, in order to
not have a low framerate).
So instead, special images will only be drawn if the text box has fully
faded in completely. That solves the issue completely.
This commit fixes a bug that's existed since MMMMMM was added (so, 2.2),
where if you quicksaved in a custom level while no music was playing,
then quit and loaded that quicksave, and you were using PPPPPP while
having MMMMMM available, it would play MMMMMM track 15, even though the
game intends for the music to simply be silent.
This is due to the same bug that lets you play MMMMMM tracks if you're
on PPPPPP - musicclass::play() does a modulo, but C++ modulo is not
guaranteed to be positive given negative inputs, so the 16-track offset
is added to a negative number, resulting in targeting the MMMMMM
soundtrack instead of PPPPPP.
That exploit doesn't harm anyone and shouldn't be fixed, EXCEPT it
causes a problem in this specific case. But this bug can be fixed
without removing that exploit.
Note that I made the check do not-equal to -1 instead of greater-than
-1, so levels that intend on using track -2, -3, -4, etc. upon loading a
quicksave will still work as their creator intended. It's just that
specifically -1 is patched out, just to fix this issue.
What this simply does is make it so that in the event that
game.hascontrol is somehow set to false when there isn't a cutscene
running (i.e. when game.state is 0 and script.running is false), it gets
set back to true again.
There's many ways to interrupt a gamestate and/or a running script, most
notably telejumping and doing a screen transition in the middle of the
animation, interrupting it.
This implements the first part of my idea in this comment:
https://github.com/TerryCavanagh/VVVVVV/issues/391#issuecomment-659757071
Achievements could be unlocked in custom levels/make and play, so this adds the wrapper function `game.unlockAchievement` which calls `NETWORK_unlockAchievement` if `map.custommode` is false.
Also, this function and `Game::unlocknum` have both been `ifdef`ed to be empty if MAKEANDPLAY is defined.
This fixes the bug where Viridian would appear to "zip" when they would
be teleported to the position of the teleporter before being flung out
of it.
As discussed in #393, I've also set the oldxp/oldyp when Viridian is
temporarily positioned in the center of the room, even though at this
point they should already be invisible. This is just to be safe.
Fixes#393.
Okay, so basically here's the include layout that this game now
consistently uses:
[The "main" header file, if any (e.g. Graphics.h for Graphics.cpp)]
[blank line]
[All system includes, such as tinyxml2/physfs/utfcpp/SDL]
[blank line]
[All project includes, such as Game.h/Entity.h/etc.]
And if applicable, another blank line, and then some special-case
include screwy stuff (take a look at editor.cpp or FileSystemUtils.cpp,
for example, they have ifdefs and defines with their includes).
Including a header file inside another header file means a bunch of
files are going to be unnecessarily recompiled whenever that inner
header file is changed. So I minimized the amount of header files
included in a header file, and only included the ones that were
necessary (system includes don't count, I'm only talking about includes
from within this project). Then the includes are only in the .cpp files
themselves.
This also minimizes problems such as a NO_CUSTOM_LEVELS build failing
because some file depended on an include that got included in editor.h,
which is another benefit of removing unnecessary includes from header
files.
Now if your cursor was at the bottom of the screen when you entered
playtesting but then was moved to the top during playtesting, when you
exit, the roomname won't instantly disappear and then sheepishly rise
back up again.
And if your cursor was at the bottom of the screen when you entered
playtesting, and exited still being that way, the roomname will rise
back down again and won't instantly disappear.
Both of these behaviors make the roomname movement much more continuous
than it was previously, and it feels smoother and less janky.
Also, use inspecial() instead of writing out each part of the
conditional separately. This just basically adds the insecretlab
conditional to the if-statement, which shouldn't be a big deal.
graphics.textbox.clear() should be used instead of
graphics.textboxremove() or graphics.textboxremovefast(), because even
with graphics.textboxremovefast(), you'll still have to process at least
one frame of GAMEMODE logic before the text boxes are actually properly
removed, and this caused a 1-frame glitch when exiting playtesting with
text boxes on-screen and then re-entering playtesting.
Technically I could've only fixed it in Game::returntoeditor(), but I
wanted to be safe, so I also fixed it in scriptclass::hardreset(), too.
This fixes a bug where the settings menu would immediately be brought up
if you used Esc to exit playtesting, unless you were an incredible ninja
and only pressed it for exactly one frame.
This fixes an annoying bug where if you use Up or Down to press ACTION
on the "All crewmates rescued!" dialogue whenever you rescue the last
crewmate during playtesting, it'll move you to the room above or below
you. This is because ed.keydelay isn't set to 6 (which is the standard
value that it gets set to whenever you press most keys in the editor),
but now it is.
The shouldreturntoeditor variable is supposed to be used because it
fixes the warp background not being reset if you exit into a
horizontally/vertically warping room with a different background. It
also properly resets other variables, which is good, too.
There's a bug where playtesting from Ved doesn't properly play the music
of the level, due to no fault with Ved.
This was because the music was being faded out by
scriptclass::startgamemode() case 23 after main() called music.play().
To fix this, just call music.play() when all the other variables are
being set in Game::customloadquick().
So you get a trophy and achievement for completing the game in Flip
Mode. Which begs the question, how does the game know that you've played
through the game in Flip Mode the entire way, and haven't switched it
off at any point? It looks like if you play normally all the way up
until the checkpoint in V, and then turn on Flip Mode, the game won't
give you the trophy. What gives?
Well, actually, what happens is that every time you press Enter on a
teleporter, the game will set flag 73 to true if you're NOT in Flip
Mode. Then when Game Complete runs, the game will check if flag 73 is
off, and then give you the achievement and trophy accordingly.
However, what this means is that you could just save your game before
pressing Enter on a teleporter, then quit and go into options, turn on
Flip Mode, use the teleporter, then save your game (it's automatically
saved since you just used a teleporter), quit and go into options, and
turn it off. Then you'd get the Flip Mode trophy even though you haven't
actually played the entire game in Flip Mode.
Furthermore, in 2.3 you can bring up the pause menu to toggle Flip Mode,
so you don't even have to quit to circumvent this detection.
To fix both of these exploits, I moved the turning on of flag 73 to
starting a new game, loading a quicksave, and loading a telesave (cases
0, 1, and 2 respectively in scriptclass::startgamemode()). I also added
a Flip Mode check to the routine that runs whenever you exit an options
menu back to the pause menu, so you can't circumvent the detection that
way, either.
Gamestates 300..336 are used to start scripts in custom levels. However,
it looks like instead of having the cases have common code, each
individual case was copy-pasted numerous times, which is pretty
wasteful.
std::string is one of those special types that has a constructor that
just initializes itself to a blank state automatically. This means all
`std::string`s are by default already `""`, so there's no need to set
them. And in fact, cppcheck throws out warnings about performance due to
initializing `std::string`s this way.
Instead, the string in MenuOption is just a buffer of 161 chars, which
is 40 chars (160 bytes if each were the largest possible UTF-8 character
size) plus a null terminator. This is because the maximum length of a
menu option that can be fit on the screen without going past is 40
chars.
There's no need to use a template here. Just manually call SDL_tolower()
or SDL_toupper() as needed.
Oh yeah, and use SDL_tolower() and SDL_toupper() instead of libc
tolower() and toupper().
So, originally, I wanted to keep them on Game, but it turns out that if
I initialize it in Game.cpp, the compiler will complain that other files
won't know what's actually inside the array. To do that, I'd have to
initialize it in Game.h. But I don't want to initialize it in Game.h
because that'd mean recompiling a lot of unnecessary files whenever
someone gets added to the credits.
So, I moved all the patrons, superpatrons, and GitHub contributors to a
new file, Credits.h, which only contains the list (and the credits max
position calculation). That way, whenever someone gets added, only the
minimal amount of files need to be recompiled.
They're always fixed-size anyways, there's no need for them to be
vectors.
Also used the new INBOUNDS_ARR() macro for the map.explored bounds
checks in Script.cpp, and made map.explored a proper bool array instead
of an int array.
Since they're always fixed-size, they don't need to be dynamically-sized
vectors.
entityclass::customcrewmoods is now a proper bool instead of an int now,
and I replaced the hardcoded constant 6 with a static const int Game
attribute to make it easier to change.
These are the besttimes, besttrinkets, bestlives, and bestrank
attributes of Game. bestframes was already a plain array.
As these are always fixed-sized, there's no reason for them to be
vectors. Also, I put their size in a static const int so it's easy to
change how many of them there are.
