When you pick up a trinket in the wild, the music will fade back in
afterwards. However, the special trinket cutscenes (where Victoria or
Vitellary will directly give you a trinket) are inconsistent with this,
and restart the music instead of fading it back in.
Looking at the scripts themselves, it immediately becomes obvious the
reason for this inconsistency - 2.2 and previous didn't implement the
musicfadein command, so it couldn't be used, and Terry or Simon simply
had to make do with simply restarting the music. However, 2.3 implements
musicfadein, so we can simply swap it out and remove the
trinketscriptmusic command.
Instead of copying to a temporary string, just use SDL_strncmp(). Also,
I checked the blame, and apparently I committed the line that used
strcmp() instead of SDL_strcmp(), for whatever reason. But that's fixed
now.
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.
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).
There's a bug in the cutscene that plays if your companion is Vitellary
in the room "Now Stay Close To Me...". The relevant gamestate is
gamestate 43, which for Vitellary calls the script `int1yellow_4`.
When Vitellary says the text box "That big... C thing! I wonder what it
does?", Terry intended for Vitellary to change his facing direction to
the left, as you can see with the command `changedir(yellow,0)` in the
original scripting. `changedir()` just changes the `dir` attribute of an
entity, and a `dir` of 0 means face left and a `dir` of 1 means face
right.
Then when Vitellary says "Maybe we should take it back to the ship to
study it?", Terry intended for him to face rightwards once again, as
indicated by the `changedir(yellow,1)` command.
Unfortunately, what happens instead is that when Vitellary says the
first text box ("That big... C thing! I wonder what it does?"), he turns
left for precisely one frame, and then afterwards goes back to facing
right. Then the second text box comes around, but he's already facing
right. How come?
Well, the problem here is that Vitellary's AI for "follow Viridian" is
overriding his `dir` attribute. Vitellary's AI says "get close to
Viridian", but Vitellary is already close enough to them that he stays
put. However, he still turns to face them as part of that AI.
To fix that, we need to put him in the AI mode that specifically says to
face left, with the command `changeai(yellow,faceleft)`. That way, he no
longer has the AI mode of following Viridian, and he will actually look
left for the intended duration instead of only looking left for one
frame.
But then we have another problem. When the cutscene ends, Vitellary no
longer follows Viridian. I mean it makes sense - we just placed him in
"only face left" mode, not "follow Viridian" mode! And this is not
merely a visual problem, because Vitellary is a supercrewmate and the
game won't let the player walk off the screen if Vitellary isn't
offscreen yet.
To fix THAT issue, we'll need to put Vitellary back in "follow Viridian"
mode. It turns out that the `changeai()` command was more intended for
scripting crewmates (entity type 12), NOT supercrewmates (entity type
14). As such, the command assumes that you'll want state numbers that
apply to entity type 12, such as 10, 11, 12, 13, and 14, even though the
only one that applies to entity type 14 is state 0, and every other
state number just makes it so that the entity doesn't move an inch. And
specifying faceleft/faceright is just state number 17.
Luckily, we can still pass the raw state number to `changeai()`, we
don't have to use its intended names. So I do a `changeai(yellow,0)` to
set Vitellary's state number back to 0 when it comes time to make him
face right again.
As a bonus, I added comments to the changed lines. This is a semi-obtuse
method of scripting, so it's always good to clarify.
For some reason, there were just exact duplicates of the talkgreen_2
script and alarmon/alarmoff commands. I have no idea why, but cppcheck
identified them.
The previous way manually concatenated the first 7 characters of the
string together (and had an std::min() calculation). The new way instead
does std::string::substr(), which is much more snappy.
Although it's not an issue, this should minimize the stack footprint of
calling scriptclass::load(), especially if it goes down to calling
scriptclass::loadcustom() or scriptclass::loadother().
Instead each line is now held in a const char* array, like it should be.
This results in less work for the compiler, especially with
optimization, since every time the compiler encounters a constant
argument in a function, it has to go off and locate a place to put it.
But if we're upfront and say, hey, here's all the strings we ever need
conveniently packaged into one place, it'll be much more cooperative.
I have the feeling that none of the devs understood what extern did, and
they kind of just sprinkled it everywhere until things started working.
But like all other classes, it should just be one line in the class's
respective header file, and shouldn't be so messy.
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.
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.
