If a track was restarted after it faded out, then it wouldn't play. This
is because currentsong wasn't set to -1 after fading out, and that is
because the fade out calls pause() instead of haltdasmusik() when it
finishes.
Unlike f196fcd896, this fixes the time
trial music while keeping it to the same behavior as 2.2, and fixes
every single possible case that this music bug could have happened.
This reverts only a part of f196fcd896 -
as the original commit author did not do their changes atomically, they
also squashed in a de-duplication within the same commit. So I'm only
reverting the part of the commit that wasn't the de-duplication, which
is simply the changes to the music.fadeout() calls.
This is being (partially) reverted for several reasons:
1. It's not the correct behavior. What this does instead is persist the
track through after you restart the time trial, instead of fading it
out, then restarting it again. This is in contrast to behavior in
2.2, and I see no reason to not keep the same behavior.
2. It's a single-case patch. The time trials are not the only time in
the game a music track could fade out and then be restarted with the
same track - custom levels could do the same thing too. Instead of
fixing only one case, we should strive to fix EVERY case.
The original commit author (trelbutate) also didn't write anything in
the commit description of f196fcd896. What
you should write in the commit description is things like rationale,
analysis, and other good information that would be useful to anyone
looking at your commit to understand why you did what you did. Having no
commit description leaves readers in the dark as to why you did what you
did.
Thus, I don't know why trelbutate went with this solution, or if they
knew that it was only a single-case patching, or if they knew that it
wasn't 2.2 behavior.
By not writing the commit description, they miss a chance for
reflection; speaking from personal experience, I myself have gone back
and improved my commits countless times because I wrote commit
descriptions for every single one of them, and sometimes whenever I
write them, I think to myself "hang on a minute, that doesn't sound
quite right" and end up finding improvements.
If trelbutate wrote a commit description, they might have realized that
it wasn't 2.2 behavior, and gone back and fixed up their commit to be
correct. As it stands, though, they didn't have to think about it in the
first place because they never bothered to write a commit description.
edteleportent is a global variable that gets assigned whenever the
player collides with a warp token, and gets read from later down the
line in gamelogic(). While I don't know of any way to cause anything bad
with this (and I did try), storing a temporary indexing variable like
this is only bound to be a liability in the future - so we might as well
prevent badness now by adding a bounds check here.
This fixes a bug where quitting to the menu from command-line
playtesting with -playassets specified would always use those assets
when loading back in to any custom level. This also fixes loading in to
a custom level quicksave always using the command-line playtesting
arguments instead of using the actual quicksave.
In a vertically-warping room, the 'height' of the room becomes 232
pixels, regardless of if you have a room name or not. So the remaining 8
rows of pixels at the bottom of the screen corresponds with the first 8
rows of pixels at the top of the screen, and entities in the bottom 8
rows of pixels get teleported to the top of the screen.
The screen wrapping drawing code doesn't draw entities in the top 8 rows
of pixels at the bottom, leading to a discontinuous effect where it
looks like vertically-warping entities don't neatly change from the
bottom to the top or vice versa - this is especially noticeable with
enemies. To fix this, just increase the threshold for drawing top
entities at the bottom of the screen by 8 pixels.
When an entity vertically warps, it teleports upwards or downwards by
232 pixels. However, the graphics code draws them with an offset of 230
pixels. This is off by 2 pixels, but it's enough to make a
downwards-moving enemy look like it suddenly collides with the bottom of
the screen (in a room without a room name) before it warps, especially
if you go frame-by-frame.
It seems like for whatever reason that the frames portion of save files
is never read from, and always zeroed. Well, technically they get parsed
but the result is immediately discarded afterwards.
I see no reason to do this, so I'm removing these zeroes.
This fixes being able to make music fully fade in (or out) by unfocusing
the game, or making the fade bars fully fade in (or out) by unfocusing
the game, or racking up the timer while the game is unfocused.
In 2.2 and previous, the game would call resetgameclock() every frame
for the last 30 frames of the time trial countdown in order to make sure
it gets reset. This was in a render function, and didn't get brought out
in 2.3, so 2.3 resets the game clock *while rendering*, which is kinda
bad and is an oversight on my part for not noticing.
Instead of doing that, just add a conditional to the timer so that it
won't tick during the time trial countdown. This fixes#699 even further
by making it so the time trial par can't even be lost during the
countdown, because the timer won't tick up - so you can never get a sad
squeak to play by pausing the game or unfocus-pausing it during the
countdown.
For some reason, resetgameclock() is only ever used in gamerender(), and
everywhere else just zeroes the clock manually. This is weird to me, so
I've made it so everywhere that zeroes the clock uses the
resetgameclock() function to do so.
Otherwise, if the timer ticked up past the par (via using the unfocus
pause or pause menu), it would result in the sad squeak being played
every frame because the game would constantly be setting
timetrialparlost, then moving to the code block below, assuming that
since timetrialparlost that we haven't lost the par already, and playing
the squeak.
timetrialparlost gets reset in hardreset() and startgamemode() anyways,
so there's no need to be constantly resetting this variable.
Fixes#699.
It turns out this entire chunk of code is simply unneeded (and is
actively harmful) since when we're done with the time trial,
quittomenu() gets called, and that removes the previous stack frame
anyway.
I'm guessing that I added this code, then added quittomenu(), then
didn't consider how this code and quittomenu() would mix. But anyways,
this bug is fixed.
Fixes#714.
This seems to be a comment left by Ethan that he never got around to. So
I did it for him.
What I've done is made it so FileSystemUtils.cpp knows what a binary
blob is, and moved the binary blob loading code directly to
FileSystemUtils.cpp. To do this, I removed the private access modifier
from binaryBlob - I don't think we'll need it, and anyways when we move
to C we can't use it.
Along the way, I also cleaned up the style of the function a bit - the
null termination offset is no longer hardcoded, and the function no
longer mixes code and declarations together in the same block.
I also noticed that when printing all the filenames at the end, a single
invalid header would stop the whole loop instead of just being skipped
over... this seems to be a bug to me, so I've made it so invalid headers
just get skipped over instead of stopping the whole loop.
In FileSystemUtils.h, I used a forward declaration. In hindsight,
incomplete forward declarations should basically always be done in
header files if possible, otherwise this introduces the possibility of
transitive includes - if a file includes this header and it does a full
include, the file is silently able to use the full header, whereas if
it's a forward declaration, then the moment the file tries to use the
full header it fails, and then it's forced to include the full header
for itself. But uh, that's a code cleanup for later.
While fixing all the other music bugs, I discovered that starting
playtesting in the editor wouldn't play the level music.
The problem is that the editor playtesting start code calls
music.fadeout() before calling music.play(). This queues up the track
from the music.play() call. After that, what should happen is that
processmusic() processes the fade, the fade is then finished, and then
after that it sees that the music is halted so it can play the queued
track.
Instead what happens is that the function first attempts to play the
music before the fade is processed and finished, so play() will re-queue
the music again, but the queue gets cleared right after that (this is a
subtle bit of behavior - it means if the game fails to play a queued
track due to it fading, it's not going to re-queue it again and end up
in some sort of infinite loop).
This is a frame ordering issue - the function is tripping over itself
when it shouldn't be. To fix it, just put the queue processing code
after the fade processing code.
This fixes the 2.2-and-below music blocking workaround not working in
2.3.
The issue was that when the music got halted by the script, the fade
volume would still be processing, silently being decremented in the
background. So the script playing the track afterwards would make the
game queue it (as it was called during the fade), but then the music is
halted so the game would attempt to play it, but the fade is STILL
happening so it wouldn't actually play it and would attempt to queue the
track again.
However, that queue gets discarded immediately afterwards because the
music.play() call happened inside the code responsible for playing the
queued music, and that code unconditionally clears the queue variables
immediately after calling play(). So that's good to know - if the game
queues a song, but fails to play it because of a fade, it's not going to
immediately re-queue it and potentially get stuck in a loop of
infinitely queueing the same song over and over again each frame.
Anyways, the source of the problem is not resetting the fade booleans
when halting music, so I've reset them.
Fixes#701.
The problem here is that even though we start playing the music when the
volume is set to zero, mixer's state doesn't have volume zero, so
whatever it plays next will be the very first quanta of the track but at
the previous volume (in this case, the maximum volume). To fix this,
just update mixer when we update the volume here - it's okay to not
account for user volume because it ends up being zero anyway.
Fixes#710.
This fixes a bug where fading music in but not going through the
music.play() path wouldn't start the fade volume from zero. If this
happened, then the previous volume would persist, and if the previous
volume was the max volume, then that essentially canceled out the
fade-in and prevented it from happening at all. But now all paths to
fadeMusicVolumeIn() set the volume to zero first, instead of only the
caller of music.play().
When you pick up a trinket in the wild, the music gets silenced, so it
silently plays in the background until you advance the trinket text.
However, foundtrinket (used when Victoria or Vitellary give you a
trinket) is inconsistent with this, and halts the music instead of
silencing it.
This was probably due to the musicfadein script command not being
implemented, so Terry or Simon had to simply make do and halt the music
instead. But musicfadein is implemented and is being used in the trinket
cutscenes, so this is another inconsistency that I will fix.
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.
This is 2.2 behavior, which I forgot to keep. Otherwise, if music has
halted and you try to play the same track, it simply won't work, because
the current song is the same as the song you're trying to play. This is
what happened with the trinket scripts - the game halted music, then
tried to play the same track.
Fixes#712.
It's not really used because CreateDirectory doesn't support setting
chmod values, but it does clarify intent of the argument.
Co-authored-by: Ethan Lee <flibitijibibo@gmail.com>
In #52 I fixed VVVVVV not being able to handle filepaths with non-ASCII
characters on Windows. 2f0a0bce4c and
aa5c2d9dc2 reintroduce this problem,
however, by reverting the definition of mkdir to how it was before the
fix and using the non-Unicode version of CreateDirectory. And I can
confirm that VVVVVV indeed doesn't make its folder anymore with a
Windows username of "тест". This commit fixes that issue.
This adds music and volume sliders to the audio options. To use the
sliders, you navigate to the given option, then press ACTION, and your
selection will be transferred to the slider. Pressing left or right will
move the slider accordingly. Then you can press ACTION to confirm the
volume is what you want and deselect it, or you can press Esc to cancel
the volume change, and it will revert to the previous volume; both
actions will write your settings to disk.
Most of this commit is just adding infrastructure to support having
sliders in menus (without copy-pasting code), which is a totally
completely new user interface that has never been used before in this
game. If we're going to be adding something new, I want to make sure
that it at least is done the RIGHT way.
Closes#706.
This adds <musicvolume> and <soundvolume> tags to unlock.vvv and
settings.vvv, so users' volume preferences will be persistent across
game sessions. This does not add the user interface to change them from
in-game; the next commit will do that.
This function is simple - it takes a given buffer and its size, fills it
with a certain character, and null-terminates it. It's meant to be used
with freshly-created buffers, so we don't copy-paste code.
Pressing return in gameplay options would send you back to the pause
menu instead of the general options menu, and pressing return in graphic
options would send you back to the pause menu instead of the general
options menu, too. Additionally, pressing Esc in graphic options would
also send you back to the pause menu instead of the general options
menu.
Like I said before, the menu system is still a bit hardcoded in some
places, and these happened because Terry forgot to update them when he
changed the menus around.
Fixes#711.
The in-game menu code is better than it was in 2.2 but still pretty
hardcoded, so to fix this just change each individual case around. This
bug happened because the "options" button was in the place where "quit
to menu" was previously, but Terry forgot to update it when changing all
the options around.
PhysFS requires a write dir to create a directory, so the first PHYSFS_mkdir
never could have worked. Because of that we need to go back to the old mkdir,
and since we're bringing that back we can reuse it for saves/levels, because we
know it works and we don't have to worry about middlewares ruining anything.
When a text box in the script system (not the gamestate system) is
displayed onscreen and "- Press ACTION to advance text -" is up, the
game sets pausescript to true, so the script system won't blare past the
text box and keep executing. Then it also sets advancetext to true.
Crucially, these two variables are different, so if you have pausescript
true but advancetext false, then what happens?
Well, you get softlocked. There's no way to continue the script.
How is this possible? Well, you can teleport to the (0,0) teleporter
(the teleporter in the very top-left of the map) and regain control
during the teleporter animation. To do that, in 2.2 and below, you have
to press R at the same time you press Enter on the teleporter, or in 2.3
you can simply press R during the cutscene. Then once you teleport to
the room, it's really precise and a bit difficult (especially if
Viridian is invisible), but you can quickly walk over to the terminal in
that room and press Enter on it.
Then what will happen is the terminal script will run, but the
teleporter gamestate sequence will finish and turn advancetext off in
the middle of it. And then you're softlocked.
To fix this, just add a check so if we're in gamestate 0 and there's a
script running, but we have pausescript on and advancetext off, just
turn pausescript off so the game automatically advances the script.
This softlock was reported by Tzann on the VVVVVV speedrunning Discord.
If you manage to get softlocked by being stuck in completestop, the next
thing you'll notice is that quitting to the menu or loading back in will
not reset this.
So you can actually softlock yourself in 2.3 by doing the trinket
cutscene, then quitting to the menu (because 2.3 lets you open the pause
menu during completestop). This is a bug, and should be fixed.
You can skip the "You have found a shiny trinket!" cutscene. The
conditions are that this can only be done in the main game, in the main
dimension (no Polar Dimension), the checkpoint that you last touched
must not be in the same room as the trinket, and you have to have
skipped the Comms Relay cutscene. To do the skip, you press R on the
exact frame (or previous frame, if input delay is enabled) that Viridian
touches the trinket. Then, the gamestate will be immediately set to 0
(because of the gotoroom) and the cutscene will be skipped.
Speedrunners of the main game, well, run the main game already, the
only trinket in the Polar Dimension is not one you want to do a death
warp at, and they have a habit of automatically skipping over the Comms
Relay cutscene because they press R at the beginning of the run when
Viridian teleports to Welcome Aboard, to warp back to the Ship and so
they can leave rescuing Violet for later.
So someone reported softlocking themselves by doing the trinket text
skip in 2.3. The softlock is because they're stuck in a state where
completestop is true but can't advance to a state that turns it off. How
does this happen? It's because they pressed R too late and interrupted
the gamestate sequence. In 2.2 and previous, if you're in the gamestate
sequence then you can't press R at all, but 2.3 removes this restriction
(on account of aiming to prevent softlocks). So only on the very first
frame can you death warp and interrupt the gamestate sequence before it
happens at all.
Anyways to fix this, just turn completestop off automatically if we're
in gamestate 0 and there's no script running.
This softlock was reported by Euni on the VVVVVV speedrunning Discord.
So some people reported the levels list crashing when they loaded it.
But this wasn't reproducible every time. They didn't provide any
debugging information, so I had to use my backup plan: doing a full
audit of the code path taken for loading the levels list.
And then I found this. It turns out this was because I used a
LOAD_ARRAY_RENAME() macro on an std::vector. You can't do that because
you need to use push_back() to resize a vector, so the macro will end up
indexing into nothing, causing a segfault. However, this code path would
only be taken if you have an old levelstats.vvv, from 2.2 and previous -
which explains why it wasn't 100% reproducible. But now that I know you
need an old levelstats.vvv, this bug happens 100% of the time.
Anyways, to fix this, just ditch the macro and expand it manually, while
replacing the indexing with a proper usage of push_back().
While the game does support playing levels with filenames that don't
have the .vvvvvv extension, it doesn't do it well.
Namely, those files can't be loaded or saved into the editor (because a
.vvvvvv always gets tacked on to your input when saving or loading). In
2.3, this gets worse because you can't load a level without a .vvvvvv
extension from command-line playtesting (because a .vvvvvv automatically
gets added) and you can't load per-level custom assets.
The only place where extensionless level files are supported is when
loading level metadata. But this makes it so they no longer work. This
is technically an API break, but it's easily fixed (just add the
.vvvvvv), plus there's nothing to be gained from not having an
extension, plus basically no one ever actually did this in the first
place (as far as I know, I am the only person to have ever done this,
and no one else ever has).
