The main game used a set of copy-pasted code to set the music of each
area. There WAS some redundancy built-in, but only three rooms in each
direction from the entrance of a zone.
Given this, it's completely possible for players to mismatch the music
of the area and level. In fact, it's easy to do it even on accident,
especially since 2.3 now lets you quicksave and quit during cutscenes.
Just play a cutscene that has Pause music, then quicksave, quit, and
reload. Also some other accidental ways that I've forgotten about.
To fix this, I've done what mapclass has and made an areamap. Except for
music. This map is the map of the track number of every single room,
except for three special cases: -1 for do nothing and don't change music
(usually because multiple different tracks can be played in this room),
-2 for Tower music (needs to be track 2 or 9 depending on Flip Mode),
and -3 for the start of Space Station 2 (track 1 in time trials, track 4
otherwise).
I've thoroughly tested this areamap by playing through the game and
entering every single room. Additionally I've also thoroughly tested all
special cases (entering the Ship through the teleporter or main
entrance, using the Ship's jukebox, the Tower in Flip Mode and regular
mode, and the start of Space Station 2 in time trial and in regular
mode).
Closes#449.
2.3 has a regression where if you move back and forth between a zone,
you can get the wrong music playing in a zone. An example is the
Overworld and Lab. Just walk in to the Lab and immediately walk back
out, and you'll get Potential for Anything playing in the Overworld.
This regression was caused by facb079b35.
That commit removed assigning -1 to currentsong when a fadeout was
called.
Basically, the previous behavior was: currentsong is 4, we enter Lab and
nicechange gets queued to 3 but currentsong gets set to -1, then going
back nicechange gets queued to 4 again.
However, if we don't assign -1, then going back will keep nicechange at
3. Why? Because niceplay() checks for currentsong before assigning
nicechange. If currentsong is still the same then it doesn't assign
nicechange.
To fix this, just always unconditionally assign nicechange.
If spawned as a custom enemy (createentity entry 56), or spawned outside
of the rooms they spawn in in the main game, they will repeatedly clone
themselves every frame, which profusely leaks memory. In fact it quickly
causes a crash in 2.2 and previous, but 2.3 fixes that crash, so it just
keeps spawning enemies endlessly, which eventually lags the game, and
eventually can out-of-memory your system (bad!).
The problem is those movement types rely on entclass::setenemyroom() to
change their `behave` to be 11 or 13. Else, the new entity created will
still have `behave` 10 or 12, which will create ANOTHER entity in the
same way, and so on, and so forth.
So to fix this, just make it so if an enemy is still `behave` 10 or 12
by the end, then, just set it to -1. That way it'll stay still and won't
cause any harm.
I considered setting the `behave` to 11 or 13 respectively, but, that's
probably going farther than just fixing a memory leak, and anyways, it's
not that much useful for me as a custom level maker, and the entities
spawned aren't really controllable.
In order to let callers provide their OWN callback functions through the
callback function WE provide to PhysFS, we casted the function pointer
to a void pointer.
Unfortunately, this is apparently undefined behavior... if your compiler
doesn't have an extension for it. And most compilers on most
architectures do. (In fact compilers on POSIX systems most certainly
have it due to dlsym() returning a void* which could actually be a
pointer to a function sometimes.)
But imo, it's better to be safe than sorry in this regard. Especially
when given GCC's approach to optimizing int + 100 > int (spoilers: they
remove it entirely! It's faster, but also broken!).
I've decided to wrap it in a struct. And as a nice side effect, if we
ever need more data to be passed through... well we already have this
struct.
Technically, it's also standards-compliant to cast a _pointer to_ a
function pointer to a void pointer. But that extra layer of pointer
indirection would get real confusing to conceptualize real fast (or at
least is more confusing than just putting it in a struct).
Since you've been able to resume music stopped by stopmusic() with
resumemusic(), if a track was stopped by stopmusic(), the unfocus pause
itself would end up resuming the track when regaining focus.
The solution is to simply check for if music.currentsong is -1 or not.
So, platv is a room property that controls the speed of custom entity
platforms in the room (unless, of course, they're created with
createentity). Problem is, this is how 2.2-and-previous coding standards
were:
ed.level[game.roomx-100+((game.roomy-100)*ed.maxwidth)]
Overly long, verbose, not entirely clear unless you already know what it
means? Copy-pasted over and over due to all of the above? Surely a
recipe for not making any coding errors!
Ironically enough, copy-pasting is basically the best approach here
(short of refactoring the whole thing, like I did in
945d5f244a), since if you don't ACTUALLY
copy-paste and just re-type it on your own, you'll end up making more
mistakes. Like what happened here:
ed.level[game.roomx-100+((game.roomy-100)*ed.mapwidth)].platv
Do you see the mistake...? Yeah, mapwidth (with a P) instead of maxwidth
(with an X). You'd have to look closely to find it.
So what does this mean for platv? Well, it means that it multiples the
y-coordinate of the room by the map width instead of the max width (20),
like every other room property. So that means if your map width is less
than 20, like say, map width 10, the platv value for (2,2) will be
stored in (2,1)'s room properties instead of (2,2)'s. Because if you go
off of map width, the room index for (2,2) is 2 + 2 * 10 = 22, but if
you go off of max width, the room index for (2,1) is 2 + 1 * 20 = 22.
Now this wouldn't be bad, except for another 2.2-and-previous
standard... kind of just not exposing things directly to the end user.
Whether that's simply not documenting something (as in the case of
ifwarp and warpdir, which by all measures were completely intended to be
used in custom levels but just simply were never known properly until I
discovered how to use them in 2019), or in this case, not giving any way
for the user to fiddle with platv from the in-game editor. Because if
there was a way to do that, and someone decided to test to see if platv
worked okay, they would discover something was up.
So... since I refactored room properties in
945d5f244a, I kind of broke platv by
fixing it. Now levels that relied on platv being the broken way don't
work.
How do I fix it, and thus break it again? Well, I'll do what I did for
scripts - handle the scrambling when reading and writing the level, and
keep things sane at least internally.
Thus: editorclass::load() will unscramble platv data in the right way,
and editorclass::save() will re-scramble platv in the right way too.
To match the option to nuke all main game save data, there is also now
an option to nuke all custom level save data separately (which is just
all custom level quicksaves, along with stars for level completion). It
has its own confirmation menu too. It does not delete any levels from
the levels folder.
Custom level quicksaves are NOT affected by the clear data menu, so the
player should be able to delete quicksaves this way. The quicksave
confirmation menu now has an extra option to delete the save (and that
option also has its own confirmation menu before deleting).
This error case can happen, but if it does, non-console users get an
ERROR page with no further information. So use setLevelDirError if this
failure mode happens. And Menu::errorloadinglevel needs to be changed to
accomodate that.
Not sure why the original implementation decided to do things this way
instead of snprintf'ing a path to the .zip itself. Otherwise, if the
level is from data.zip, PHYSFS_getRealDir() will return the path of
data.zip, which then fails to mount for separate reasons.
Since createentity() started accepting p1/p2/p3/p4 arguments, it now
unconditionally passes in whatever arguments were present there
previously, when there weren't any before.
This can lead to unexpected behavior when selectively using and then
omitting p1/p2/p3/p4 arguments.
Also, plenty of existing levels already only use the 5-argument version
of createentity(). And createcrewman() can take up to 6 arguments at
once. It's not far-fetched that an existing level could createentity()
right after doing a 6-argument createcrewman(), which would lead to a
different behavior than in 2.2 and previous.
So instead, instead of checking if `words[index]` is an empty string (it
only sets the string to be empty if there are enough argument separators
on the line), ACTUALLY check if it's empty. I've added a static array
(no need for it to be exported) that keeps track of this. createentity()
now checks for that instead of `words`.
It's possible to get one page of levels by removing all the built-ins,
either by removing them directly from data.zip or by putting files with
the same filenames as them in your level folder that don't contain
nothing.
And hey, there's already a check for if no levels exist at all, so why
not check for this too?
Previously, you would only get the trinket completion star if you got
the exact same amount of trinkets as there are custom entity trinkets in
the level file. But if you got more (say, if the level spawned extra
"bonus trinkets"), you wouldn't be able to get the star.
This is true of the custom crewmate case as well, but I've decided to
not change that case, because there are still downsides to the resulting
behavior and it's better to just leave it alone because it's rare for it
to happen anyways.
Since custom levels have gained the functionality to show trinkets on
the minimap, it's nice to just save the showtrinkets variable directly
to the save file, without having to make level makers handle it
themselves.
If you have unfocus pause off, and unfocus audio pause off, then this command will go into effect.
When it's set to on, the audio will pause when you unfocus the game. When it's set to off, the
audio will not. This is different from the setting, and gets saved to the save file.
If a zip file is improperly structured, a message will be displayed when
the player loads the level list.
This will only display the last-displayed improper zip, because there
only needs to be one displayed at a time. Also because doing anything
more would most likely require heap allocation, and I don't want to do
that.
This will wrap text on-the-fly, since I will be introducing text that
needs to be wrapped whose length we can't know in advance. (Or we can,
but, that'd be stupid.)
I took the algorithm from Dav999's localization branch, but it's not
like it's a complicated algorithm in the first place. Plus I think it
actually handles words that get too long to fit on a single line better
than his localization branch. The only difference is that I removed all
the STL, and made it more memory efficient (unlike his localization
branch, it does not copy the entire string to make a version with
newline separator characters).
This macro needs to be used because Clang is stupid and doesn't let you
use /* fallthrough */ comments like GCC does. However, if GCC is too old
(as is the case on CentOS 7), then it won't recognize __has_attribute
either.
Some people prefer the 2.2 behavior where unfocusing pauses the game,
but the music still plays. One such person is Trinket9 on the VVVVVV
Discord server, who wanted it that way.
The reason audio pausing was added in the first place was to prevent
desyncing music in levels with cutscenes that synced to music. Rather
than reverting it, let's add this option instead.
Similar to disabling the elephant flashiness, at least one
photosensitive person has told me the flashy color animation makes their
eyes kind of hurt a little bit. Also it screws up the compression really
badly when they record (especially the green noisy tiles!).
The colors will still cycle, but the individual animations within each
color will be completely static.
It's quite rude to close the game. Especially if the user does not use
the console. They won't know why the game closed.
Instead, just return -1. All usages of font_idx() should be and are
bounds checked anyways. This will result in missing characters, but,
it's not like the characters had a font image in the first place,
otherwise we wouldn't be here. And if the user sees a bunch of
characters missing in their font, they'll probably work out what the
problem is even without having a console. And it's still far better than
abruptly closing the game.
And use WHINE_ONCE to prevent spamming the console.
Let's say you have a zip named LEVELNAME.zip, but the only .vvvvvv file
it contains is NOTLEVELNAME.vvvvvv. This zip would end up printing both
the 'LEVELNAME.vvvvvv is missing' and 'It has .vvvvvv file(s) other than
LEVELNAME.vvvvvv' messages, even though we already know there's
something wrong with the zip, and the 'other level files' message is
redundant, since in this case the problem here is simply just the
.vvvvvv file being named the wrong way.
The 'other level files' message is only intended to be printed when
LEVELNAME.vvvvvv *does* exist, but there's additional .vvvvvv files in
the zip on top of that, so don't print this message if LEVELNAME.vvvvvv
exists.
Since colors going into FillRect() need to be in BGR format, we need to
use getBGR instead. (Well, actually, it gets passed in RGB, but then at
some point the order gets switched around, and, really, this game's
masks are all over the place, I'm going to fix that in 2.4.)
This can happen if you select an option in a menu that (A) returns to
the previous menu and (B) saves settings. If the settings save fails,
this will create another menu on the same frame that cycles the tower BG
after it's already been cycled for that frame. Examples are the slowdown
and glitchrunner menus.
I could fix this by creating a new function that copy-pastes all of
Game::savestatsandsettings_menu() except for the map.nexttowercolour()
at the end. But that's copy-pasting code.
Instead what I've done is added a variable to signal if the color has
already been cycled this frame, so we don't cycle it again. This also
covers cases of possible double-cycling in the future as well.
This is because the fade delay did not last long enough.
I was under the mistaken impression that the fade animation lasts for 15
frames. However, this does not account for the fact that the offset of
each fade bar is dependent on RNG, and the worst case scenario is that
they have an offset of 96 pixels (in the opposite direction of the
fade).
The actual fade animation timer accounts for the worst case scenario, so
the fade animation actually lasts for (320 pixels plus 96 pixels is 416
pixels, 416 pixels divided by 24 pixels per frame equals 17.333...
frames, but since the actual timer keeps adding/subtracting 24 pixels
per frame until it passes the 416-pixel threshold, that gets rounded up
to...) 18 frames.
And an extra frame to make it so deltaframe interpolation doesn't
suddenly stop on the last deltaframes before the screen is completely
black.
I also need to draw the screen black on the map screen when glitchrunner
mode is off, if there's a fadeout going on. Else that would introduce
yet another frame flicker.
