1
0
Fork 0
mirror of https://github.com/TerryCavanagh/VVVVVV.git synced 2024-12-23 10:09:43 +01:00
Commit graph

1757 commits

Author SHA1 Message Date
Misa
f6919981e5 Silence music instead of halting in foundtrinket
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.
2021-04-12 16:17:31 -04:00
Misa
d965a11988 Don't restart music in trinket cutscenes
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.
2021-04-12 16:17:31 -04:00
Misa
711b36c9c8 Set currentsong to -1 when halting music
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.
2021-04-12 16:17:31 -04:00
Dav999-v
dbd9a114b6 Change mode in Windows-only mkdir to 0777
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>
2021-04-12 13:25:27 -04:00
Dav999-v
274ae98915 Re-fix handling filepaths with non-ASCII characters on Windows
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.
2021-04-12 13:25:27 -04:00
Misa
e8316c7e9a Implement music and sound volume sliders
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.
2021-04-11 20:56:16 -04:00
Misa
27874e1dc6 Add music and sound volume config options
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.
2021-04-11 20:56:16 -04:00
Misa
2a3f17f1f7 Add audio category to options menu
The audio category contains the MMMMMM soundtrack option, as well as
stubs for the soon-to-be-implemented volume slider options.
2021-04-11 20:56:16 -04:00
Misa
5133d58777 Add VVV_fillstring()
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.
2021-04-11 20:56:16 -04:00
Misa
eb9d3582d8 Fix incorrect return code from game options and graphic options
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.
2021-04-11 16:58:10 -04:00
Misa
d89ce76577 Fix returning from confirm quit menu putting cursor on options
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.
2021-04-11 16:58:10 -04:00
Ethan Lee
aa5c2d9dc2 Move first CreateDirectory to getOSDirectory, avoids redundancy on non-Win32 2021-04-11 11:55:35 -04:00
Ethan Lee
2e630ce06c Forgot a 0... 2021-04-11 11:44:56 -04:00
Ethan Lee
2f0a0bce4c Revert to my old userdata folder creation code.
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.
2021-04-11 11:32:29 -04:00
Ethan Lee
aa97a5d6a1 Revert "Only migrate savedata if "saves" was created", need this for levels too
This reverts commit fcb09f85cb.
2021-04-11 11:26:11 -04:00
Ethan Lee
fcb09f85cb
Only migrate savedata if "saves" was created 2021-04-11 11:19:49 -04:00
Misa
bc0b9e1fa0 Fix softlocks from turning off advancetext at wrong time
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.
2021-04-10 21:47:23 -04:00
Misa
fe8df3001e Reset completestop in hardreset
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.
2021-04-10 21:47:23 -04:00
Misa
dd4100f752 Fix softlocks from mistimed trinket text skip
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.
2021-04-10 21:47:23 -04:00
Vittorio Romeo
082a0c2205 Fix '-Wpedantic' warnings under gcc 10.x 2021-04-10 20:53:01 -04:00
viri
6e1054e362 CONTRIBUTORS.txt: Update contact info 2021-04-09 22:39:33 -04:00
Misa
f96f5703ff Fix level list segfault when upgrading old levelstats.vvv
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().
2021-04-09 22:22:05 -04:00
Misa
0f0e218cf6 Use macros to calculate accessibility offset
This means the offset is calculated at compile-time, and we can use a
proper case-switch here instead of an else-if chain.
2021-04-09 17:42:58 -04:00
Misa
b5f1cbb2d1 Clean up unused vars/funcs from eee98b0e07
eee98b0e07 made some functions and
variables unused, which generate compiler warnings. So I've removed
them.
2021-04-09 17:42:58 -04:00
Misa
8e4b904f57 Fix whitespace from eee98b0e07
eee98b0e07 introduced mixed indentation
and one trailing whitespace, which I've cleaned up.
2021-04-09 17:42:58 -04:00
Misa
9e560e31cf Blacklist non-.vvvvvv files in level metadata
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).
2021-04-09 11:26:59 -04:00
TerryCavanagh
8b1efb4335 Default to 60 FPS mode (fixes #702) 2021-04-09 20:40:22 +10:30
TerryCavanagh
eee98b0e07 Clean up of options menus for v2.3 (fixes #696) 2021-04-09 20:39:12 +10:30
TerryCavanagh
d9d5cbbab2 Changed the wording on "Invincibility" and "Game Speed" accessibility options
I've wanted to do this one for a very long time
2021-04-09 17:53:15 +10:30
Misa
300f1b7919 Switch assets mounting to dedicated directory
This fixes an issue where you would be able to mount things other than
custom assets in per-level custom asset directories and zips.

To be fair, the effects of this issue were fairly limited - about the
only thing I could do with it was to override a user-made quicksave of a
custom level with one of my own. However, since the quicksave check
happens before assets are mounted, if the user didn't have an existing
quicksave then they wouldn't be able load my quicksave. Furthermore,
mounting things like settings.vvv simply doesn't work because assets
only get mounted when the level gets loaded, but the game only reads
from settings.vvv on startup.

Still, this is an issue, and just because it only has one effect doesn't
mean we should single-case patch that one effect only. So what can we
do?

I was thinking that we should (1) mount custom assets in a dedicated
directory, and then from there (2) mount each specific asset directly -
namely, mount the graphics/ and sounds/ folders, and mount the
vvvvvvmusic.vvv and mmmmmm.vvv files. For (1), assets are now mounted at
a (non-existent) location named .vvv-mnt/assets/. However, (2) doesn't
fully work due to how PhysFS works.

What DOES work is being able to mount the graphics/ and sounds/ folders,
but only if the custom assets directory is a directory. And, you
actually have to use the real directory where those graphics/ and
sounds/ folders are located, and not the mounted directory, because
PHYSFS_mount() only accepts real directories. (In which case why bother
mounting the directory in the first place if we have to use real
directories anyway?) So already this seems like having different
directory and zip mounting paths, which I don't want...

I tried to unify the directory and zip paths and get around the real
directory limitation. So for mounting each individual asset (i.e.
graphics/, sounds/, but especially vvvvvvmusic.vvv and mmmmmm.vvv), I
tried doing PHYSFS_openRead() followed by PHYSFS_mountHandle() with that
PHYSFS_File, but this simply doesn't work, because PHYSFS_mountHandle()
will always create a PHYSFS_Io object, and pass it to a PhysFS internal
helper function named openDirectory() which will only attempt to treat
it as a directory if the PHYSFS_Io* passed is NULL. Since
PHYSFS_mountHandle() always passes a non-NULL PHYSFS_Io*,
openDirectory() will always treat it like a zip file and never as a
directory - in contrast, PHYSFS_mount() will always pass a NULL
PHYSFS_Io* to openDirectory(), so PHYSFS_mount() is the only function
that works for mounting directories.

(And even if this did work, having to keep the file open (because of the
PHYSFS_openRead()) results in the user being unable to touch the file on
Windows until it gets closed, which I also don't want.)

As for zip files, PHYSFS_mount() works just fine on them, but then we
run into the issue of accessing the individual assets inside it. As
covered above, PHYSFS_mount() only accepts real directories, so we can't
use it to access the assets inside, but then if we do the
PHYSFS_openRead() and PHYSFS_mountHandle() approach,
PHYSFS_mountHandle() will treat the assets inside as zip files instead
of just mounting them normally!

So in short, PhysFS only seems to be able to mount directories and zip
files, and not any loose individual files (like vvvvvvmusic.vvv and
mmmmmm.vvv). Furthermore, directories inside directories works, but
directories inside zip files doesn't (only zip files inside zip files
work).

It seems like our asset paths don't really work well with PhysFS's
design. Currently, graphics/, sounds/, vvvvvvmusic.vvv, and mmmmmm.vvv
all live at the root directory of the VVVVVV folder. But what would work
better is if all of those items were organized into a subfolder, for
example, a folder named assets/. So the previous assets mounting system
before this patch would just have mounted assets/ and be done with it,
and there would be no risk of mounting extraneous files that could do
bad things. However, due to our unorganized asset paths, the previous
system has to mount assets at the root of the VVVVVV folder, which
invites the possibility of those extraneous bad files being mounted.

Well, we can't change the asset paths now, that would be a pretty big
API break (maybe it should be a 2.4 thing). So what can we do?

What I've done is, after mounting the assets at .vvv-mnt/assets/, when
the game loads an asset, it checks if there's an override available
inside .vvv-mnt/assets/, and if so, the game will load that asset
instead of the regular one. This is basically reimplementing what PhysFS
SHOULD be able to do for us, but can't. This fixes the issue of being
able to mount a quicksave for a custom level inside its asset directory.

I should also note, the unorganized asset paths issue also means that
for .zip files (which contain the level file), the level file itself is
also technically mounted at .vvv-mnt/assets/. This is harmless (because
when we load a level file, we never load it as an asset) but it's still
a bit ugly. Changing the asset paths now seems more and more like a good
thing to do...
2021-04-05 16:39:37 -04:00
Misa
798bf9e490 Add filename to mountAssetsFrom() error message
This will clarify which directory, exactly, failed to mount. I know it
gets printed earlier in the mounting process, but it can't hurt to print
it twice, just to be sure. Also this is for consistency.
2021-04-05 16:39:37 -04:00
Misa
ff3cba9cee Replace asset load calls with loadAssetToMemory()
All assets now use FILESYSTEM_loadAssetToMemory() instead of
FILESYSTEM_loadFileToMemory().
2021-04-05 16:39:37 -04:00
Misa
72ae6921ea Add FILESYSTEM_loadAssetToMemory()
This is currently just a wrapper around FILESYSTEM_loadFileToMemory(),
but assets are supposed to use this function instead of the regular one.
2021-04-05 16:39:37 -04:00
Misa
aea5611e5b Remove default argument from loadFileToMemory()
Default function arguments are the devil, and it's better to be more
explicit about what you're passing into the function. Also because we
might become C-only in the future and to help faciliate that, we should
get rid of C++-isms like default function arguments now.
2021-04-05 16:39:37 -04:00
Misa
a11920e1a9 Remove redundant getDirSeparator() calls in init()
PHYSFS_getDirSeparator() already gets called and stored in pathSep at
the top of FILESYSTEM_init(). So clearly, two people worked on this
function and forgot that both pieces of code existed at the same time
(or it was one person independently forgetting both).
2021-04-05 16:39:37 -04:00
Misa
34ec943b5c Remove getDirSeparator() usage from mountAssetsFrom()
PhysFS uses platform-independent notation, so we really don't need to
care about getting the correct dir separator here. Especially since we
don't ever do so anywhere else (e.g. load/saveTiXml2Document()), either.
2021-04-05 16:39:37 -04:00
Misa
d95ba3a8b3 Rename FILESYSTEM_mount() to FILESYSTEM_mountAssetsFrom()
This is to make it clear that this is not a general-purpose mounting
function; it is a helper function for FILESYSTEM_mountAssets()
specifically for treating a directory or file as an assets directory,
and mounting assets from there.
2021-04-05 16:39:37 -04:00
Misa
43692388c0 Use FILESYSTEM_mount() when mounting zips
There's no reason to handle mounting .zip files differently than
mounting a directory... we already mount .data.zip files using
FILESYSTEM_mount(), so why go through the trouble of opening a .zip
manually (which means on Windows the .zip can't be touched for the
duration of playing the custom level), making up a place to mount it at,
and then mount that made-up name, instead of just using
FILESYSTEM_mount()?

Whoever cobbled this asset mounting thing together really didn't fully
understand what they were doing.
2021-04-05 16:39:37 -04:00
Misa
a8a09a207f Properly camel-case FILESYSTEM_[un]mountassets()
They are now camel-cased to be consistent with the rest of the
filesystem functions.
2021-04-05 16:39:37 -04:00
Misa
9c8ecdb0f4 Return early if FILESYSTEM_mountassets() fails
This way, we avoid the unnecessary graphics.reloadresources() call - if
we can't mount assets, why bother reloading resources?

The return type of FILESYSTEM_mount() has been changed from void to bool
to indicate success, accomodating its callers accordingly.
2021-04-05 16:39:37 -04:00
Misa
1e375f9ecf Un-export FILESYSTEM_mount()
This function is never used outside of FileSystemUtils.cpp; there is no
reason to export it.
2021-04-05 16:39:37 -04:00
Ethan Lee
5060b4dfe3 Only do focus fullscreen toggling on X11.
I haven't been able to reproduce this old thing on any setup I have. The patch
from 2013 was originally for X11, and Wayland's fullscreen doesn't allow for
this sort of thing, so let's start scoping this down for eventual removal when
X11 is finally out of our minds forever.
2021-04-05 11:07:32 -04:00
Misa
510ec07021 Re-fix resumemusic/musicfadein once again
So it looks like facb079b35 (PR #316) had
a few issues.

The SDL performance counter doesn't really work that well. Testing
reveals that unfocusing and focusing the game again results in
the resumemusic() script command resuming the track at the wrong time.
Even when not unfocusing the game at all, stopping a track and resuming
it resumes it at the wrong time. (Only disabling the unfocus pause fixes
this.)

Furthermore, there's also the fact that the SDL performance counter
keeps incrementing when the game is paused under GDB. So... yeah.

Instead of dealing with the SDL performance counter, I'm just going to
pause and resume the music directly (so the stopmusic() script command
just pauses the music instead). As a result, we no longer can keep
constantly calling Mix_PauseMusic() or Mix_ResumeMusic() when focused or
unfocused, so I've moved those calls to happen directly when the
relevant SDL events are received (the constant calls were originally in
VCE, and whoever added them (I'm pretty sure it was Leo) was not the
sharpest tool in the shed...).

And we are going to switch over to using our own fade system instead of
the SDL mixer fade system. In fact, we were already using our own fade
system for fadeins after collecting a trinket or a custom level
crewmate, but we were still using the mixer system for the rest. This is
an inconsistency that I am glad to correct, so we're also doing our own
fadeouts now.

There is, however, an issue with the fade system where the length it
goes for is inaccurate, because it's based on a volume-per-frame second
calculation that gets truncated. But that's an issue to fix later - at
least what I'm doing right now makes resumemusic() and musicfadein()
work better than before.
2021-04-02 16:13:54 -04:00
Misa
6d3a73c540 Add pause(), pauseef(), and resumeef() to musicclass
musicclass already had a resume() function for music.

These are just wrappers around the appropriate SDL_mixer functions, to
avoid direct function calls to the mixer API. So if we ever need to do
something with all callers of pausing and resuming in the future, or we
switch to a different audio backend, the work is already done for us.

Also it just looks cleaner to be calling our musicclass function instead
of doing a direct API call to the mixer.
2021-04-02 16:13:54 -04:00
Misa
92b3c0b413 Factor fade amount calculation to separate function
This makes it so to reuse this code, we don't have to copy-paste it.

Additionally, I added a check for the milliseconds being 0, to avoid a
division by zero. Logically and mathematically, if the fade amount is 0
milliseconds, then that means the fade should happen instantly -
however, dividing by zero is undefined (both in math and in C/C++), so
this check needs to be added.
2021-04-02 16:13:54 -04:00
Misa
f6ea05f521 Factor timestep calculation out to Game function
This is so we can grab the game's timestep anywhere else in the codebase
without copy-pasting it.
2021-04-02 16:13:54 -04:00
Misa
f8f6f3b96e Add option to re-enable 1-frame input delay
This is an option for speedrunners whose muscle memory is precisely
trained and used to the 1-frame input delay that existed in 2.2 and
below. It is located in Game Options -> Advanced Options, and is off by
default.

To re-add the 1-frame input delay, we simply move the key.Poll() to the
start of the frame, instead of before an input function gets ran -
undoing what #535 did.

There is a frame ordering-sensitive issue here, where toggling
game.inputdelay at the wrong time could cause double-polling. However,
we only toggle it in an input function, which regardless is always
guaranteed to be ran after key.Poll() (it either happened at the start
of the frame or just before the input function got ran), so this is not
an issue. But, in case we ever need to toggle this variable in the
future, we can just use the defer callbacks system to defer the toggle
to the end of the frame - also added by #535.

Added at the request of Habeechee on the VVVVVV speedrunning Discord
server.
2021-04-02 11:18:32 -04:00
Misa
4b3409e2e8 Remove advancetext check from teleporter prompt logic
This fixes being unable to use teleporters while the "- Press ACTION to
advance text -" prompt is up, which is used to perform credits warp.

In 2.2 and 2.0, this advancetext check was only in gamerender() for
rendering the "- Press ENTER to Teleport -" prompt and didn't affect any
logic. In 2.3, I moved the check (and the rest of the conditional it was
in) to gamelogic() - same as the activity zone prompt conditionals - so
if you gained control while being in a prompt zone, the prompt wouldn't
suddenly appear[1].

As a side effect, this ended up aligning rendering and logic together,
so if you couldn't see the teleporter prompt, you weren't able to
teleport - whereas in 2.2 and 2.0, you could still use the teleporter
even though the prompt wasn't up.

So by removing the advancetext check, you are now able to use the
teleporter again, AND the "- Press ENTER to Teleport -" prompt will also
show up as well.

Habeechee reported this regression on the VVVVVV speedrunning Discord
server.

[1]: f07a8d2143, PR #421
2021-04-02 11:16:42 -04:00
Misa
cd2f5ccde9 Add period to glitchrunner mode description text
This is to be consistent with the other options in the Advanced Options
menu.
2021-04-02 11:15:53 -04:00
Misa
91bc438d79 Fix funcs not being reassigned if delta func is last func in list
One of the solutions to the quit signal unfocus pause regression is to
add a no-op delta func to the unfocused func table. However, this
results in the game being stuck in unfocus pause forever, because when
it reaches the end of a list on a delta func, it won't reassign the
active functions - only when the end of a list is a fixed func will it
do so. A workaround is to then add a no-op fixed func afterwards, but
that's inelegant.

The solution in the end to the quit signal regression is to not bother
with adding a delta func, so the game as of right now actually never has
a delta func at the end of a list, and probably never will - but this is
one piece of technical debt I don't want to leave laying around. In case
we're ever going to put a delta function at the end of a list, I've made
it so that delta functions will now reassign the list of active funcs if
they happen to be at the end of the func list.
2021-04-02 11:15:18 -04:00
Misa
03a0b1feb2 Call VVV_exit() when SDL_QUIT is received
This fixes a regression introduced by #535 where a quit signal (e.g.
Ctrl-C) sent to the window while the game was in unfocus pause wouldn't
close the game.

One problem was that key.quitProgram would only be checked when control
flow switched back to the outer loop in main(), which would only happen
when the loop order state machine switched to a delta function. As the
unfocused func table didn't have any delta functions, this means
key.quitProgram would never be checked.

So a naïve solution to this would just be to add a no-op delta func
entry to the unfocused func table. However, we then run into a separate
issue where a delta function at the end of a func list never reassigns
the active funcs, causing the game to be stuck in the unfocus pause
forever. Active func reassignment only happens after fixed funcs. So
then a naïve solution after that would be to simply add a no-op fixed
func entry after that. And indeed, that would fix the whole issue.

However, I want to do things the right way. And this does not seem like
the right way. Even putting aside the separate last-func-being-delta
issue, it mandates that every func list needs a delta function. Which
seems quite unnecessary to me.

Another solution I considered was copy-pasting the key.quitProgram check
to the inner loops, or adding some sort of signal propagation to
the inner loops - implemented by copy-pasting checks after each loop -
so we didn't need to copy-paste key.quitProgram... but that seems really
messy, too.

So, I realized that we could throw away key.quitProgram, and simply call
VVV_exit() directly when we receive an SDL_QUIT event. This fixes the
issue, this removes an unnecessary middleman variable, and it's pretty
cleanly and simply the right thing to do.
2021-04-02 11:14:24 -04:00
Terry Cavanagh
4ebbfe476a
Merge pull request #588 from InfoTeddy/general-bug-fixes
Fix tile of disappearing platforms during final stretch
2021-04-02 17:51:30 +10:30
Terry Cavanagh
886a57ab83
Merge pull request #667 from InfoTeddy/general-improvements
Add text outlines to textboxless textboxes and gravitron text
2021-04-02 17:49:20 +10:30
Misa
ef091de23e Outline all gravitron text
This includes all text from the Gravitron and Super Gravitron.

This is to make the text more readable if they are placed in weird
situations - for example, in custom levels, where the background these
texts get placed on could be anything (custom level makers are crazy!).
2021-03-30 23:57:00 -07:00
Misa
6538d1e5dd Add Graphics::bigrprint()
Same as bigbprint(), we duplicate some of the calculations because it's
better than duplicating another text printing function.
2021-03-30 23:57:00 -07:00
Misa
f7173027ce Add Graphics::bigbprint()
It's just like bigprint() except it duplicates some of the calculations
because I didn't want to make a bigprintoff() function which would
duplicate even more code. I'm beginning to think these text printing
functions are completely horrible to work with...
2021-03-30 23:57:00 -07:00
Misa
827b3e430b Outline textboxless textboxes
In case they get drawn against a non-contrasting background, it's still
useful to keep them readable by outlining them. This could happen if
someone were to use the Game Complete gamestate sequence in a custom
level (or presses R during Game Complete).
2021-03-30 23:57:00 -07:00
Misa
0c72260c5d Fix oversight with unfocus pause screen in Flip Mode
Flip Mode flips all the unfocus pause screen text upside-down, to make
it read in reverse order. This looks kind of strange to me, and I don't
think it was intended. So I'm flipping the text again so it's the right
way up in Flip Mode.
2021-03-30 23:55:42 -07:00
Misa
f9e76d9dc0 Fix tile of disappearing platforms during final stretch
During the final stretch, after Viridian turns off the Dimensional
Stability Generator, the map goes all psychedelic and changes colors
every 40 frames. Entities change their colors too, including conveyors,
moving platforms, and disappearing platforms.

But play around with the disappearing platforms for a bit and you'll
notice they seem a bit glitchy. If you run on them at the right time,
the tile they use while disappearing seems to abruptly change whenever
the color of the room changes. If there's a color change while they're
reappearing (when you die and respawn in the same room as them), they'll
have the wrong tile and look like a conveyor. And even if you've never
interacted with them at all, dying and respawning in the same room as
them will change their tile to something wrong and also look like a
conveyor.

So, what's the problem? Well, first off, the tile of every untouched
disappearing platform changing into a conveyor after you die and respawn
in the same room is caused by a block of code in gamelogic() that gets
run on each entity whenever you die. This block of code is the exact
same block of code that gets ran on a disappearing platform if it's in
the middle of disappearing.

As a quick primer, every entity in the game has a state, which is just a
number. You can view each entity's state in
entityclass::updateentities().

State 0 of disappearing platforms is doing nothing, and they start with
an onentity of 1, which means they turn to state 1 when they get
touched. State 1 moves to state 2. State 2 does some decrementing, then
moves to state 3 and sets the onentity to 4. State 3 also does nothing.
After being touched, state 4 makes the platform reappear and move to
state 5, but state 5 does the actual reappearing; state 5 then sets the
state back to 0 and onentity back to 1.

So, back to the copy-pasted block of code. The block of code was
originally intended to fast-forward disappearing platforms if they were
in the middle of disappearing, so the player respawn code would properly
respawn the disappearing platform, instead of leaving it disappeared.
What it does is keep updating the entity, while the state of the entity
is 2, until it is no longer in state 2, then sets it to state 4.

Crucially, the original block of code only ran if the disappearing
platform was in state 2. But the other block of code, which was
copy-pasted with slight modifications, runs on ALL disappearing
platforms in final stretch, regardless of if they are in state 2 or not.

Thus, all untouched platforms will be set to state 4, and state 4 will
do the animation of the platform reappearing, which is invalid given
that the platform never disappeared in the first place. So that's why
dying and respawning in the same room as some disappearing platforms
during final stretch will change their tiles to be conveyors.

It seems to me that doing anything with death is wrong, here. The root
cause is that map.changefinalcol() "resets" the tile of every
disappearing platform, which is a function that gets called on every
color change. The color change has nothing to do with dying, so why
fiddle with the death code?

Thus, I've deleted that entire block of code.

What I've done to fix the issue is to make it so the tile of
disappearing platforms aren't manually controlled. You see, unlike other
entities in the game, the tile of disappearing platforms gets manually
modified whenever it disappears or reappears. Other entities use the
tile as a base and store their tile offset in the separate walkingframe
attribute, which will be added to the tile attribute to produce the
drawframe, which is the final thing that gets rendered - but for
disappearing platforms, their tile gets directly incremented or
decremented whenever they disappear or reappear, so when
map.changefinalcol() gets ran to update the tile of every platform and
conveyor, it basically discards the tile offset that was manually added
in.

Instead, what I've done is make it so disappearing platforms now use
walkingframe, and thus their final drawframe will be their tile plus
their walkingframe. Whenever map.changefinalcol() gets called, it is now
free to modify the tile of disappearing platforms accordingly - after
all, the tile offset is now stored in walkingframe, so no weird
glitchiness can happen there.
2021-03-30 23:55:34 -07:00
Misa
a405635cb2 Replace other usage of PHYSFS_getBaseDir() with SDL_GetBasePath()
Ethan, you forgot this other one.

I do have to rejiggle the control flow of the function a bit, so it
doesn't leak memory upon failure. (Although the SDL message box leaks
memory anyway because of X11 so... whatever.) Also, there's a NULL check
for if SDL_GetBasePath() fails now.
2021-03-31 02:51:52 -04:00
Ethan Lee
051fe9eba9 Skip the icon on Apple targets, it also has the icns 2021-03-31 02:50:38 -04:00
Ethan Lee
b99abaf1d5 Put Misa at the top of GitHub Friends 2021-03-31 02:43:15 -04:00
Ethan Lee
4c4f8de0d5 Fix leaking GetBasePath result 2021-03-31 02:43:15 -04:00
Misa
8cf79aaf72 Fix memory leak when pasting text
According to SDL documentation[1], the returned pointer needs to be
freed. A glance at the source code confirms that the function allocates,
and also Valgrind complains about it.

Also if it couldn't allocate, the game no longer segfaults (std::strings
do not check if the pointer is non-NULL for operator+=).

[1]: https://wiki.libsdl.org/SDL_GetClipboardText
2021-03-31 02:29:36 -04:00
Ethan Lee
9d2ebbc982
Re-sync CONTRIBUTORS and GitHub Friends lists 2021-03-30 23:08:25 -04:00
Ethan Lee
b87f6e487a Use SDL_GetBashPath instead of PHYSFS_getBaseDir, latter is made of fail on macOS 2021-03-30 12:18:09 -04:00
Ethan Lee
076a870ba2 VS2010 buildfix 2021-03-30 11:51:42 -04:00
Misa
367e77fb59 Fix targets (question marks) not showing up on minimap
Followup to #635: I had misread the original '==0' comparisons as being
truthy comparisons instead of being falsy comparisons. Whoops.
2021-03-26 00:02:26 -04:00
Misa
892e7c93fc Fix background not changing when pressing Esc on main menu
Followup to #664: Pressing "quit game" instead of using Esc changes the
background, but pressing Esc doesn't. Whoops.
2021-03-25 23:32:59 -04:00
Misa
100d986431 Remove mainmenu from Game
Since mainmenu is only ever used in Input.cpp, I might as well make it
clearer by moving it into a static global variable in Input.cpp. (The
same applies to fadetolab/fadetomenu, but I didn't think much about
those at the time... that'll be a refactor for later.)
2021-03-25 23:32:39 -04:00
Misa
be10487c5c Set fademode to temp 0 when going to in-game options
While I've decoupled fademode from gamemode starting, being faded out on
the title screen results in a black screen and you being unable to make
any input. So we'll need to store the current fademode in a temporary
variable when going to in-game options, then put it back when we return
to the pause menu. Yes, you can turn on glitchrunner mode during the
in-game options, and then immediately return to the pause menu to
instantly go back to the title screen; this is intended.

Due to frame ordering, putting the fademode back needs to be deferred to
the end of the frame to prevent a 1-frame flicker.

It's actually sufficient enough to do this temporary fademode storage to
fix the whole thing, but I also decided to decouple fademode and
gamemode starting just to be sure.
2021-03-25 23:32:39 -04:00
Misa
82dfa0b86c Decouple fademode from starting gamemode
Assuming glitchrunner mode is off, if you open the pause menu while
fully faded-out and then go to Graphic Options or Game Options, then the
'mode' that you selected previously will kick in again and you'll be
suddenly warped back.

So if you previously started a new game in the main game (mode 0, also
the selected mode if you do this from command-line playtesting), and
then open the pause menu and go to in-game options, then you'll suddenly
go back to starting a new game again. If you had started a custom level,
doing this will warp you back to the start of the level again.

The problem is simple - when the title screen is fully faded out, it
calls startgamemode(). So the solution is simple as well - just decouple
the fademode from calling startgamemode(), and use a different variable
to know when to actually call startgamemode().
2021-03-25 23:32:39 -04:00
Misa
cd0c9ccb31 De-duplicate setting mainmenu and fademode
This makes my work easier later.
2021-03-25 23:32:39 -04:00
Misa
6a6c09f69d Fix 1-frame flicker entering room with warp lines and entity on edge
Custom levels can have warp lines. If you have a warp line and a warping
background in the same room, the warp line takes precedence over the
warp background.

However, whenever you enter a room with a warp line and warp background,
any entities on the warping edges will be drawn with screenwrapping for
one frame, even though they never wrapped at all.

This is due to frame ordering: when the warp line gets created,
obj.customwarpmode gets set to true. Then when the screen edges and
warping logic gets ran, the very first thing that gets checked is this
exact variable, and map.warpx/map.warpy get set appropriately - so
there's no way the entity could legitimately screenwrap.

However, that happens in gamelogic(). gamelogic() is also the one
responsible for creating entities upon room load, but that happens after
the obj.customwarpmode check - so when the game gets around to rendering
in gamerender(), it sees that map.warpx or map.warpy is on, and draws
the screenwrapping, even though map.warpx/map.warpy aren't really on at
all. Only when gamelogic() is called in the frame later do map.warpx and
map.warpy finally get set to false.

To fix this, just set map.warpx and map.warpy to false when creating
warp lines.
2021-03-25 22:29:40 -04:00
Misa
8fd7210d37 De-duplicate warp line creation in createentity()
Copy-pasted code begone. This makes it much clearer what the difference
between all the warp line entities in this function are.
2021-03-25 22:29:40 -04:00
Misa
c8f4c37c88 Add bounds check for color of createentity number 55
I just spotted this one - if vy isn't bounds-checked, this causes bogus
input from the createentity() script command to commit Undefined
Behavior. Should've spotted this one when I was adding bounds checks to
the rest of createentity() earlier, but at least it's fixed now.
2021-03-25 21:43:51 -04:00
Misa
423c79b572 Add bounds checks to room explored getter and setter
This means you can no longer cause Undefined Behavior by exploring a
room that is outside the array of explored room statuses.
2021-03-24 15:55:34 -04:00
Misa
c5e999c1d5 Refactor explored rooms to use setters and getters
This makes it easier to add bounds checks to all accesses of
map.explored. Also, all manually-written existing bounds checks have
been removed, because they're going to go into the new getters and
setters.

The getter is mapclass::isexplored() and the setter is
mapclass::setexplored().
2021-03-24 15:55:34 -04:00
Misa
b340a6ccc4 Add bounds checks to room propety getters and setters
It is no longer possible to cause Undefined Behavior via accessing
out-of-bounds room properties.

What happens instead is - if you attempt to fetch an out-of-bounds room
property, you get a "blank" room property that just has all of the
defaults, plus its tileset is 1 because all tilesets that are nonzero
use tiles2.png, and it closely emulates the previous behavior where it
was some bogus value but definitely not zero. Its Direct Mode is also 1,
because the tiles contained within it are just mishmashed repeats of
existing tiles on the map, and we shouldn't autotile that.

The roomname also gets cleared in case the user attempts to set the room
name of an out-of-bounds room property.

If you attempt to set the property of an out-of-bounds room property,
then nothing happens.
2021-03-24 15:55:34 -04:00
Misa
945d5f244a Refactor room properties to use setter and getter funcs
This replaces all raw ed.level accesses with new setter and getter
funcs, which makes it easier to add bounds checks later. And I've also
removed all the manually-written bounds checks, since they will go into
the new getter and setter.

To get the room properties of a specific room, you use
editorclass::getroomprop(), which returns a pointer to the room
properties - then you just read off of that pointer. To set a room
property, you use editorclass::setroom<PROP>(), where <PROP> is the name
of the property. These are maintained using X macros to avoid
copy-pasting. editorclass::getroompropidx() is a helper function and
shouldn't be used directly.
2021-03-24 15:55:34 -04:00
Misa
cfd5be1bc5 Add bounds checks to tile setter and getters
This removes all traces of Undefined Behavior from getting and placing
tiles.

This mimics the previous behavior (2.2 and below) as reasonably as
possible. `vmult` was previously a vector, there was a bunch of unused
space directly after the end of the usable space of the vector, which
was all filled with zeroes. The same goes for `contents`, having
previously been a vector, and so having a bunch of zeroes immediately
following the end of the in-bounds space. That's why both are 0 if you
index them out of bounds.
2021-03-24 15:55:34 -04:00
Misa
344c93e754 Refactor tiles to use setter and getter functions
This makes it easier to add bounds checks to all accesses of
ed.contents.

To do this, I've added editorclass::gettile(), editorclass::settile(),
and editorclass::getabstile() (with a helper function of
editorclass::gettileidx() that really shouldn't be used directly), and
replaced all raw accesses of ed.contents with those functions
appropriately.

This also makes the code more readable, as a side effect.
2021-03-24 15:55:34 -04:00
Misa
ccdb0c9148 Fix bounds checks in drawentity()
The existing bounds checks were correct sometimes but other times were
not.

The bounds check for 2x2 and 2x1 sprites only covered the top-left
sprite drawn; the other sprites could still be out of bounds. But if the
top-left sprite was out of bounds, then none of the other sprites
wouldn't be drawn - although it ought to be that the other sprites still
get attempted to be drawn. So I've updated the bounds checks
accordingly, and now an out of bounds top-left sprite won't prevent the
drawing of the rest of the sprites.

Similarly, if the sprite of a Gravitron square was out of bounds, that
would prevent its indicators from being drawn. But the indicators
weren't being bounds-checked either (2.3 lets you have less than 1200
tiles in a given tilesheet). So the bounds check has been moved to only
cover the drawframe and the indicator indexes accordingly, and an out of
bounds sprite won't prevent attempting to draw the indicators.
2021-03-24 15:42:28 -04:00
Misa
4e52dccdae Initialize temporary edentity when loading levels
It is possible for any of the QueryIntAttribute()s to fail, most
commonly if the attributes don't exist. If that happens, then that part
of the temporary edentity won't be initialized, and we'll end up having
a partially-uninitialized edentity - then doing much of anything with it
will result in undefined behavior.

To fix this, just initialize the temporary edentity.
2021-03-24 15:26:09 -04:00
Misa
69b0f0b650 Add missing pText NULL checks
If an XML tag doesn't contain anything inside, pText will be NULL. If
that happens without being checked, then NULL will be passed to
SDL_strcmp(). SDL_strcmp() will either call libc strcmp() or use its own
implementation; both implementations will still dereference the NULL
without checking it.

This is undefined behavior, so I'm fixing it. The solution is to do what
is done with all other XML parsing functions, and to make sure pText
gets set to a safe empty string (which is just a pointer to a null
terminator) if it happens to be NULL.
2021-03-24 15:26:09 -04:00
Misa
0da9b5069a Capitalize "OFF" when invincibility is off
All other settings capitalize "ON" and "OFF", so this one should, too.
2021-03-21 20:53:40 -04:00
Misa
9ab61af1da Add period to text outline description
This is to be consistent with all other option descriptions, which all
end in a period as well.
2021-03-21 20:53:40 -04:00
Misa
d45e6f6254 Remove game.gametimer in favor of game.frames
PR #279 added game.gametimer solely for the editor ghosts feature. It
seems that whoever originally wrote it (Leo for the now-dead VVVVVV:
Community Edition, I believe) forgot that the game already had its own
timer, that they could use.

The game timer does increment on unfocus pause (whereas this doesn't),
but that's a separate issue, and it ought to not do that.
2021-03-21 20:53:11 -04:00
Misa
17169320b4 Fix text box deltaframe flashing on deltaframes after fully opaque
So #434 didn't end up solving the deltaframe flashing fully, only
reduced the chances that it could happen.

I've had the Level Complete image flash a few times when the Game Saved
text box pops up. This seems to be because the Level Complete image is
based off of the text box being at y-position 12, and the Game Saved
text box is also at y-position 12. Level Complete only gets drawn if the
text box additionally has a red channel value of 165, and the Game Saved
text box has a red channel value of 174. However, there is a check that
the text box be fully opaque first before drawing special images. So
what went wrong?

Well, after thinking about it for a while, I realized that even though
there is indeed an opaqueness check, the alpha of the text box updates
BEFORE it gets drawn. And during the deltaframes immediately after it
gets updated, the text box is considered fully opaque. It's completely
possible for the linear interpolation to end up with a red channel value
of 165 during these deltaframes, while the text box is opaque as well.

As always, it helps if you have a high refresh rate, and run the game
under 40% slowdown.

Anyways, so what's the final fix for this issue? Well, use the text box
'target' RGB values instead - its tr/tg/tb attributes instead of its
r/g/b attributes. They are not subject to interpolation and so are
completely reliable. The opaqueness check should still be kept, though,
because the target values don't account for opaqueness. And this way, we
get no more deltaframe flashes during text box fades.

An even better fix would be to not use magic RGB values to draw special
images... but that'd be something to do later.
2021-03-21 19:01:36 -04:00
Ethan Lee
415d4790e2
Fix a crash on first unfocus 2021-03-21 17:19:08 -04:00
Ethan Lee
7abd4bb8d8
Visual Studio buildfix 2021-03-21 17:15:36 -04:00
Misa
4a79f02842 Add braces around sub-object in unfocused_func_list
Clang warns on this. This doesn't fix anything but it does ensure that
whoever's reading it won't be focused as to whether or not omitting the
second set of braces is legal or not.
2021-03-21 17:03:17 -04:00
Misa
02560ca6e5 Remove now-unneeded kludge for finalmode entity colors
Previously, with the wrong loop order, this kludge needed to exist so
entities in finalmode didn't have wrong colors for 1 frame when entering
a room. But now the loop order has been fixed, and so this kludge is no
longer needed.
2021-03-21 02:55:42 -04:00
Misa
287061c768 Fix filter/screenshake/flash update order
In 2.2, at render time, the game rendered screenshakes and flashes if
their timers were above 0, and then decremented them afterwards. The
game would also update the analogue filter right before rendering it,
too.

In 2.3, this was changed so the flash and screenshake timers were
unified, and also done at the end of the frame - right before rendering
happened. This resulted in 1-frame flashes and screenshakes not
rendering at all. The other changes in this patchset don't fix this
either. The analogue filter was also in the wrong order, but that is
less of an issue than flashes and screenshakes.

So, what I've done is made the flash and screenshake timers update right
before the loop switches over to rendering, and only decrements them
when we switch back to fixed functions (after rendering). The analogue
filter is also updated right before rendering as well. This restores
1-frame flashes and screenshakes, as well as restores the correct order
of analogue filter updates.
2021-03-21 02:55:42 -04:00
Misa
094209bd12 Move logical onground/onroof updates to start of gamelogic
This reintroduces 2-frame edge-flipping after the 1-frame input delay
got removed. This is because along with processing input and moving
Viridian, logical onground/onroof assignments need to processed in the
same between-render sequence as well - otherwise Viridian only gets 1
frame of edge-flipping due to frame ordering.
2021-03-21 02:55:42 -04:00
Misa
63a60b11cc Split onground/onroof into visual and logical variables
I will need to separate these into two different variables because I
will need to move logical onground/onroof assignments to the start of
gamelogic() - if I kept them together, however, that would change the
visuals of onground/onroof, which I want to keep consistent with 2.2.
2021-03-21 02:55:42 -04:00
Misa
d5d9d9ba96 Re-remove 1-frame input delay
To do this, GAMEMODE input needs to be processed, and Viridian needs to
be moved, in the same sequence between render frames. So just move
gameinput to after gamerender. Yes, this is not 2.2 order, but gameinput
only handles player input and nothing else - plus a 1-frame input delay
feels really awful to play with in over-30-mode.
2021-03-21 02:55:42 -04:00
Misa
f1f434accc Move key.Poll() calls to just before input funcs
In order to re-remove the 1-frame input delay, we will have to poll
input right after rendering a frame - in other words, just before an
input function gets called.

To do this, I've added a new function enum type - Func_input - that is
the same as a fixed function, but before its function gets called,
key.Poll() gets called. And all input functions have been updated to use
this enum accordingly.
2021-03-21 02:55:42 -04:00
Misa
ab26985fde Re-fix crewmate directions (without copy-pasting)
This once again fixes the facing directions of crewmates upon room load,
except now it covers more cases.

So, here is the saga so far:

- 2.0 (presumably) to 2.2: crewmate direction fix is special-cased at
  the end of mapclass::loadlevel(). Only covers crewmates created during
  the room load, does not cover crewmates created from scripts, only
  covers state 18 of crewmates.

- 2.3 currently (after #220): crewmate direction fix is moved to
  entityclass::createentity(), which covers every avenue of crewmate
  creation (including from scripts), but still only covers state 18.

- This commit: crewmate direction fix now covers every possible state of
  the crewmate, also does not copy-paste any code.

What I've done instead is to make it so createentity() will immediately
call updateentities() on the pushed-back entity. This is kludge-y, but
is completely okay to do, because unlike other entities, crewmate
entities never change their state or have any side-effects from
double-evaluation, meaning calling updateentities() on them is
idempotent and it's okay to call their updateentities() more than once.

This does have the slight danger that if the states of crewmates were to
change in the future to no longer be idempotent, this would end up
resulting in a somewhat hard-to-track-down double-evaluation bug, but
it's worth taking that risk.

This fix is not applied to entity 14 (the supercrewmate) because it is
possible that calling updateentities() on it will immediately remove the
entity, which is not idempotent (it's changing the state of something
outside the object). Supercrewmates are a bit difficult to work with
outside of the main game anyways, and if you spawn them you could
probably just use the changedir() script command to fix their direction,
so I'm not inclined to fix this for them anyway.
2021-03-21 02:55:42 -04:00
Misa
cb5d181ce8 Remove entityclass::createentity() deltaframe kludge
This copy-pasted code only existed because the previous loop order was
incorrect and rendered entities before they would get properly updated
by the fixed render function. Now, the fixed render function is
guaranteed to be called before the render function, so we can rely on
that to update the drawframe and realcol of entities instead of
duplicating the code ourselves in createentity().

The drawframe assignment is still kept to fix the case where dying while
completestop is active (i.e. during a trinket or crewmate rescue
cutscene) and respawning in a different room won't turn everything into
Viridian sprites.
2021-03-21 02:55:42 -04:00
Misa
2d9d0cffa5 Fix 1-frame glitch when going to in-game options from edsettings
The menu creation of Graphic Options or Game Options, as well as the
map.nexttowercolour() call, all need to be deferred until the end of the
frame.
2021-03-21 02:55:42 -04:00
Misa
951679b1f8 Fix 1-frame background glitch when returning from in-game options
The background would change for 1 frame before sending you back to the
pause menu or editor settings. The map.nexttowercolour() call needs to
be deferred until the end of the frame.
2021-03-21 02:55:42 -04:00
Misa
5088ff40e9 Fix 1-frame text glitch when returning to editor settings from options
The returnmenu() needs to be deferred until the end of the frame.
2021-03-21 02:55:42 -04:00
Misa
8a3e292041 Fix 1-frame text glitch returning to pause menu from in-game options
The new loop order introduces a glitch where the menu would display
whichever menu was saved to kludge_ingametemp for 1 frame right as the
user returned to the pause menu. This happened because the
game.returntomenu() happens in titleinput(), which comes before
titlerender(). To fix this, we just need to defer it to the end of the
frame.
2021-03-21 02:55:42 -04:00
Misa
3da0e31215 Remove game.shouldreturntoeditor in favor of using defer callback
game.shouldreturntoeditor was added to fix a frame ordering issue that
was causing a bug where if you started playtesting in a room with a
horizontal/vertical warp background, and exited playtesting in a
different room that also had a horizontal/vertical warp background and
which was different, then the background of the room you exited in would
slowly scroll offscreen, when you re-entered the editor, instead of the
background consisting entirely of the actual background of the room.

Namely, the issue was that the game would render one more frame of
GAMEMODE after graphics.backgrounddrawn got set to false, and re-set it
to true, thus negating the background redraw, so the editor background
would be incorrect.

With defer callbacks, we can now just use a couple lines of code,
instead of having to add an extra kludge variable and putting handling
for it all over the code.
2021-03-21 02:55:42 -04:00
Misa
32be2fcd81 Update player lerpoldxp/yp in moveplayer()
Just like gotoposition(), the player would otherwise appear to "zip"
after the command got run. This did not happen in the previous loop
order.
2021-03-21 02:55:42 -04:00
Misa
c8537beac1 Add deferred callbacks to game loop
Sometimes, there needs to be code that gets ran at the end of the game
loop, otherwise rendering issues might occur. Currently, we do this by
special-casing each deferred routine (e.g. shouldreturntoeditor), but it
would be better if we could generalize this deference instead.

Deferred callbacks can be added using the DEFER_CALLBACK macro. It takes
in one argument, which is the name of a function, and that function must
be a void function that takes in no arguments. Also, due to annoying C++
quirks, void functions taking no arguments cannot be attributes of
objects (because they have an implicit `this` parameter), so it's
recommended to create each callback separately before using the
DEFER_CALLBACK macro.
2021-03-21 02:55:42 -04:00
Misa
c8958de537 Update player lerpoldxp/yp in gotoposition()
Otherwise, the player would appear to "zip" during the deltaframes
between their previous position and their new position. This did not
happen in the previous game loop order and only happens in the new one.
2021-03-21 02:55:42 -04:00
Misa
af70076088 Remove now-unneeded deltaframe fix when going to in-game settings
Previously, before the game loop order got fixed, going to the in-game
settings would switch over to the new render function too early, causing
a deltaframe glitch that had to be fixed. But now, the render function
only gets switched when the current gamestate's function list gets
finished executing, so the game won't suddenly switch to titlerender()
in the middle of the ACTION press to the in-game settings screen.

As a consequence, titleupdatetextcol() no longer needs to be exported to
Input.cpp.
2021-03-21 02:55:42 -04:00
Misa
c26b701f5b Remove gravity line kludge from Graphics::drawgravityline()
Now that the game loop order is now fixed, there is no longer any need
for this kludge.
2021-03-21 02:55:42 -04:00
Misa
5e2fc6f0fe Move updating lerpoldxp/yp to start of gamelogic()
The previous location of this loop was placed there because it happened
just after the end of the render function. Now that the loop order is
fixed, the first thing that happens after the render function is the
start of gamelogic(), so this loop should go there now, else entity
positions won't be interpolated.

Also it now preincrements instead of postincrements because I like
preincrements.
2021-03-21 02:55:42 -04:00
Misa
585ae47d78 Remove script.dontrunnextframe kludge
Now that the game loop order is fixed, this kludge (on top of kludge) is
no longer needed, and can be safely removed.
2021-03-21 02:55:42 -04:00
Misa
c82c2afbbd Unindent unfocused_run() and focused_begin() from previous commit
As always, indentation changes are applied in a separate commit to
minimize diff noise.
2021-03-21 02:55:42 -04:00
Misa
1e9fb6aac0 Generalize game loop order and fix it to what it was in 2.2
Okay, so the reason why all render functions were moved to the end of
the frame in #220 is because it's simpler to call two fixed functions
and then a delta function instead of one fixed function, then a delta
function, and then another fixed function.

This is because fixed functions need special handling inside
deltaloop(), and you can't simply duplicate this handling after calling
a delta function. Oh, and to make matters worse, it's not always
fixed-delta-fixed, sometimes (like in MAPMODE and TELEPORTERMODE) it's
delta-fixed-fixed, so we'd need to handle that somehow too.

The solution here is to generalize the game loop and factor out each
function, instead of hardcoding it. Instead of having hardcoded
case-switches directly in the loop, I made a function that returns an
array of functions for a given gamestate, along with the number of
functions, then the game loop processes it accordingly. In fixedloop(),
it iterates over the array and executes each function until it reaches a
delta function, at which point it stops. And when it reaches the end of
the array, it goes back to the start of the array.

But anyway, if it gets to a delta function, it'll stop the loop and
finish fixedloop(). Then deltaloop() will call the delta function. And
then on the next frame, the function index will be incremented again, so
fixedloop() will call the fixed functions again.

Actually, the previous game loop was actually made up of one big loop,
with a gamestate function loop nested inside it, flanked with code that
ran at the start and end of the "big loop". This would be easy to handle
with one loop (just include the beginning and end functions with the
gamestate functions in the array), except that the gamestate functions
could suddenly be swapped out with unfocused functions (the ones that
run when you unfocus the window) at any time (well, on frame boundaries,
since key.isActive only got checked once, guarding the entire "inner
loop" - and I made sure that changing key.isActive wouldn't immediately
apply, just like the previous game loop order) - so I had to add yet
another layer of indirection, where the gamestate functions could
immediately be swapped out with the unfocused functions (while still
running the beginning and end code, because that was how the previous
loop order worked, after all).

This also fixes a regression that the game loop that #220 introduced
had, where if the fixed functions switched the gamestate, the game would
prematurely start rendering the gamestate function of the new gamestate
in the deltaframes, which was a source of some deltaframe glitches. But
fixing this is likely to just as well cause deltaframe glitches, so it'd
be better to fix this along with fixing the loop order, and only have
one round of QA to do in the end, instead of doing one round after each
change separately.

Fixes #464... but this isn't the end of the patchset. There are bugs
that need to be fixed, and kludges that need to be reverted.
2021-03-21 02:55:42 -04:00
Misa
5e440ac48d Remove special text box checks for y-position 180
Y-position 180 would be the position of the Level Complete and Game
Complete special text boxes in Flip Mode. However, since the y-position
of flipme text boxes actually no longer change (because we have to
accomodate changing Flip Mode on-the-fly), these text boxes will never
actually be y-position 180 - so we should remove these checks for
clarity.
2021-03-21 02:53:25 -04:00
Misa
596696dcf3 Make foundtrinket() Flip Mode-aware
A-ha! I've spotted an inconsistency! The normal trinket collection text
boxes (gamestate 1000-1003) is aware of Flip Mode, and will position
themselves accordingly to read the correct way in Flip Mode. However,
foundtrinket() doesn't do this.

Well, now it does.
2021-03-21 02:53:25 -04:00
Misa
db9ee0d8e3 Switch flipme script command to use flipme textbox attribute
This is why the text box attribute was named flipme, after all.

You may have noticed that the flipme command inverts textflipme instead
of simply setting it to true. Well, that's because it should be the same
as the previous behavior, which was essentially to invert it instead of
setting it to true - i.e. calling flipme twice would keep the original
text box position in Flip Mode, which means it would be upside-down
(this is a lot of flipping to keep track of...) - because flipme added
to texty in-place instead of simply assigning to it. (It did the
calculation incorrectly in 2.2 and previous, but I digress.)

Similarly, textflipme is not reset in hardreset(), because none of the
other script text box variables are reset either.
2021-03-21 02:53:25 -04:00
Misa
6939563e6d Switch all flippable text boxes to use createtextboxflipme
This ensures that if the player decides to toggle Flip Mode while one of
these text boxes is up, they won't be oriented improperly. Additionally,
it also de-duplicates a bunch of Flip Mode check code, which is also a
win.
2021-03-21 02:53:25 -04:00
Misa
c7cc2f4adc Add createtextboxreal() and createtextboxflipme()
createtextboxreal() is the same as createtextbox(), but with a flipme
parameter added to create text boxes that have their flipme attribute
set to true. createtextbox() just calls createtextboxreal() with flipme
set to false, and createtextboxflipme() just calls createtextboxreal()
with flipme set to true; this is because I do not want to use C++
function overloading.
2021-03-21 02:53:25 -04:00
Misa
1a9f2d9342 Add flipme attribute to textboxclass
Instead of calculating the y-position of the text box when it's created,
we will store a flag that says whether or not the text box should be
flipped in Flip Mode (and thus stay right-side-up), and when it comes
time to draw the text box, we will check Flip Mode and calculate the
position then.
2021-03-21 02:53:25 -04:00
Misa
2ac13815e4 Remove textrect attribute from textboxclass
Instead of duplicating the same variables over and over again,
Graphics::drawgui() can just make its own SDL_Rect. It's not that hard.

As far as I can tell, textrect was always being properly kept up to date
by the time Graphics::drawgui() got around to rendering
(textboxclass::resize() keeps being called a LOT), so this shouldn't be
a noticeable change from the user perspective.
2021-03-21 02:53:25 -04:00
Misa
334302c800 Remove unused x/y textboxclass attributes
These unused variables distract from properly analyzing the code when
you read it, since the xp/yp attributes of textboxclass already exist,
too.
2021-03-21 02:53:25 -04:00
Misa
30719b87db De-duplicate "Game Saved" telesave textbox
The "Game Saved" text box, along with its associated telesave() call,
exists in both Game.cpp and Script.cpp, so one of them is the copy-paste
of the other. Unfortunately this copy-paste resulted in an inconsistency
where both of them don't check for the same things when deciding whether
or not the telesave should actually happen (this is why you don't
copy-paste, kids... it's scary!).

Either way, de-duplicating this now is less work for me later.
2021-03-21 02:53:25 -04:00
Misa
5de884f584 De-duplicate Level Complete sequence textboxes
Every Level Complete sequence is the same copy-pasted thing, but with
minor changes. To make my work easier, I'm de-duplicating them so I have
less text boxes to change later, and less grind to grind.
2021-03-21 02:53:25 -04:00
Misa
b7ca408076 Remove default arguments from createtextbox()
These default arguments are never used anywhere. And if they were used
anywhere, it'd be better to explicitly say 255,255,255 than make readers
have to look at the header file to see what these default to. Also, this
creates four different overloads of createtextbox(), instead of only
two - but we ought to not be using function overloading anyway.
2021-03-21 02:53:25 -04:00
Misa
d5ed49d8dc Remove commented-out code from Game.cpp and Script.cpp
These commented-out code blocks just get in the way of clarity when I'm
refactoring flipped textboxes created in the gamestate system. So I'm
getting rid of them. If we need them back, we always have Git history.
2021-03-21 02:53:25 -04:00
Misa
5f2b9409b2 De-duplicate Gravitron initial message
Since the only difference is the y-positions, I've decided to remove the
copy-pasted code. A better solution would be to have a function that
draws multiline text and handles it accordingly in Flip Mode, but that
could be done later.
2021-03-21 02:06:39 -04:00
Misa
c7e807541c De-duplicate Flip Mode textbox crewmate rendering
The only difference between Flip Mode and normal mode is the y-position
and sprite used to draw the crewmates. Everything else is the same, so
I've removed the copy-pasted portion.

The diff might look a bit ugly due to the unindentation.
2021-03-21 02:06:39 -04:00
Misa
52a7d42672 De-duplicate Flip Mode text printing
Since the only difference in Flip Mode is the positiveness/negativeness
of the iterator variable, plus the starting y-offset, I've removed the
copy-pasted code and did this instead.

The diff might look a bit ugly due to the unindentation.
2021-03-21 02:06:39 -04:00
Misa
f6ecf83190 Ensure oldfadeamount is updated when fadeamount is
Like cutscene bars, I've added Graphics::setfade(), to ensure that no
deltaframe rendering glitches happen due to oldfadeamount not being
updated properly.

And indeed, this fixes a deltaframe rendering glitch that happens if you
return to the editor from playtesting on a faded-out screen, then fade
out again (by either re-entering playtesting and then cause a fadeout to
happen again, or by quitting from the editor afterwards). The same
glitch also happens outside of in-editor playtesting if you exit to the
menu while the screen is faded out.
2021-03-21 01:06:29 -04:00
Misa
f22756dd99 Ensure oldcutscenebars is updated when cutscenebarspos is
To do this, I've added Graphics::setbars(), to make sure
oldcutscenebarspos always gets assigned when cutscenebarspos is. This
fixes potential deltaframe rendering issues if these two mismatch.
2021-03-21 01:06:29 -04:00
Misa
9e2716b253 Fix a few missing implicit void arg declarations
While working on #535, I noticed that editormenuactionpress() still
didn't do the explicit void declaration. Then I ran `rg 'void.*\(\)'`
and found three other functions that I somehow missed in #628. Whoops.
Well, now they no longer are missed.
2021-03-19 10:27:54 -04:00
Misa
2c8d338e47 Add graphic options and game options to editor settings
This is a small quality-of-life tweak that makes it so if you're in the
middle of editing a level, you don't have to save the level, exit to the
menu, change whatever setting you wanted, re-enter the editor, and type
in the level name, just to change one setting. This is the same as
adding Graphic Options and Game Options to the in-game pause menu,
except for the editor, too.

To do this, I'm reusing Game::returntopausemenu() (because all of its
callers are the same callers for returning to editor settings) and
renamed it to returntoingame(), then added a variable named
ingame_editormode to Game. When we're in the options menus but still in
the editor, BOTH ingame_titlemode and ingame_editormode will be true.
2021-03-18 23:01:00 -04:00
Misa
fc8c7d034d Add being able to press Esc to return to previous menu
This is a small quality-of-life thing that makes it so you don't have to
move your menu selection all the way over to the "return" button in
order to return to the previous menu. You can just press Escape instead
to return to the previous menu. The previous behavior of pressing Escape
was to bring up the 'confirm quit' menu, or if you were in an options
menu in-game, return to the pause menu.

If you're on the main menu (and thus don't have any previous menu) and
press Escape, the game will instead bring up the 'confirm quit' menu.
For consistency, the "quit game" option on the main menu will also bring
up the 'confirm quit' menu as well, instead of immediately closing the
game.

Pressing the controller button mapped to Escape will also work as well.

The only menus that don't have return buttons are the 'countdown' menus
- so the game will not let you press Escape if there's a menu countdown
happening.

Now that pressing Escape in the 'continue' menu will just bring you back
to the 'play' menu, there's no need to specifically put
map.nexttowercolour() first when canceling the 'confirm quit' menu.
2021-03-18 20:38:23 -04:00
Misa
61e5b819e4 Fix VVVVVV-Man not being interpolated
This is because it directly uses the xp and yp of the player instead of
the interpolated xp and yp. Whoops.
2021-03-18 18:00:45 -04:00
Misa
e70586b154 Inline cutscene bars timer for gamemodes that used it in 2.2
As part of my work in #535, I've noticed that 2.3 currently with 2.2
loop order doesn't have interpolated cutscene bars. This is because
cutscene bars in 2.3 get updated at the start of the frame, which
interpolates them correctly until the render functions are put in their
proper place.

There is, however, a somewhat bigger issue, outside the scope of #535,
where cutscene bars always get updated regardless of which gamemode you
are in. Previously in 2.2 and previous, cutscene bars only got updated
in GAMEMODE and TELEPORTERMODE; sometime during 2.3, the cutscene bars
timer got pulled out of all the individual game modes and moved to the
very start of the loop. (I was probably the one who did this change;
I've been caught in a trap of my own devising.)

Thus, going to MAPMODE during the cutscene bars animation doesn't keep
their position paused like it would in 2.2. This is also categorically a
more-than-visual change, since the untilbars() script command depends
on the cutscene bars timer. I see no reason for the cutscene bars to
behave differently in this way than 2.2; #535 would also end up doing
the same fix more-or-less anyway.

Since TELEPORTERMODE currently uses the same renderfixed function as
MAPMODE, I've had to add a teleporterrenderfixed() that just calls
maprenderfixed(), but also does the cutscene bars timer.
2021-03-17 11:03:14 -04:00
Misa
4e12c162d4 Add SDL2 version number to desktop_version/ README
As a partial fix for #618, adding the SDL2 version number to the README
will clarify that you need a specific version of SDL2 in order to
compile (and run) the current version of the game (2.3 at the time of
writing); in the future, the SDL2 dependency will be upgraded with each
SDL release.

This is to avoid error messages that complain about missing symbols like
SDL_zeroa() (added in SDL 2.0.14) not being present at the time of
compilation.

Closes #626.
2021-03-17 10:58:13 -04:00
Misa
2608db9151 Directly toggle fullscreen if keybind pressed in key.Poll()
This moves the responsibility of toggling fullscreen when any of the
three toggle fullscreen keybinds are pressed (F11, Alt+Enter, Alt+F)
directly into key.Poll() itself, and not its caller (which is main() -
more specifically, fixedloop()). Furthermore, the fullscreen toggle
itself has been moved to a separate function that key.Poll() just calls,
to prevent cluttering key.Poll() with more business logic (the function
is already quite big enough as it is).

As part of my work in re-removing the 1-frame input delay in #535, I'm
moving the callsite of key.Poll() around, and I don't want to have to
lug this block of code around with it. I'd rather refactor it upfront
than touch any more lines than necessary in that PR.
2021-03-17 03:01:19 -04:00
Misa
babd86916c Move resumesong assignment to songend()
This fixes a bug where the resumemusic() script command would always
play MMMMMM track 15 (or, if you're using PPPPPP, just not work). This
is because musicclass::haltdasmusik() assigns resumesong AFTER calling
Mix_HaltMusic(), but the songend() callback fires before the resumesong
assignment, meaning resumesong gets set to -1 instead of whatever
currentsong was previously.

To fix this, just move the assignment into the callback itself (I don't
know why this wasn't done before). I could have moved it to before the
Mix_HaltMusic() call, but moving it into the callback itself fixes it
for all cases of the music stopping (such as when the music fades out).
2021-03-10 09:45:20 -05:00
Misa
d4f52fd6fe Reset room name hide timer if in an unnamed room
This avoids the room name awkwardly moving back up if the cursor is at
the bottom of the screen in a room with a room name, then the user
switches to a room without a room name, then moves the cursor away from
the bottom, then switches to a named room - even though the cursor was
already away from the bottom of the screen.

Conversely, if the user moves their cursor to the bottom of the screen
in an unnamed room, then switches into a named room, the room name will
already have been hidden and they won't need to wait for it to hide.
2021-03-10 09:44:02 -05:00
Misa
f20a703bf3 Reset drawer timer if not in Direct Mode room
This fixes the drawer suddenly popping up only to disappear, if the user
leaves a Direct Mode room into a non-Direct Mode room when the drawer
hasn't closed all the way, and then re-enters a Direct Mode room.
2021-03-10 09:44:02 -05:00
Misa
988a7720b9 Interpolate Direct Mode drawer closing
This is pretty simple to interpolate, since ed.dmtileeditor is
guaranteed to have only changed by 1, and then it's just multiplied by
4.
2021-03-09 22:05:56 -05:00
Misa
b202e02578 Move room name hiding update to editorrenderfixed
Now the room name hiding animation won't be based on FPS.
2021-03-09 22:05:56 -05:00
Misa
136c940586 Move Direct Mode drawer close to editorrenderfixed
Now the drawer closing animation won't be based on deltatime.
2021-03-09 22:05:56 -05:00
Misa
4c01d64c33 Move gravity line correction to editorrenderfixed
Gravity line correction no longer happens on every deltaframe. This
means less CPU time is wasted. Although, there's probably no need to
correct gravity lines on every single frame... hm... well, that's an
optimization for later (there's plenty of other stuff to cache, like
minimap drawing or editor foreground drawing).
2021-03-09 22:05:56 -05:00
Misa
22d71affba De-duplicate number of menu text bytes
I've moved this to a define that gets declared in Game.h. I could've
made it a const int, but that's only legal in C++ mode.
2021-03-06 22:14:24 -05:00
Misa
40c6a01917 Make saveFilePath not an std::string
Since it only ever gets assigned from FILESYSTEM_getUserSaveDirectory(),
and that function returns a C string, and the variable is only ever read
from again, this doesn't need to be an std::string.
2021-03-06 22:13:39 -05:00
Misa
36e91a9bb1 Fix MMMMMM and Flip Mode options ignoring save failure
In #553, when Dav999 added error messages to settings menus if the game
was unable to successfully save the changed settings, he seemed to have
forgotten the PPPPPP/MMMMMM toggle option.

However, I can fully blame him for only that miss. The Flip Mode options
were using game.savemystats (which was removed in #591), so if he
searched for all instances of game.savestats()
(game.savestatsandsettings() was only added in #557), he would've missed
the game.savemystats.

Later, when I did #591, I didn't realize that I should've replaced the
ones in the Flip Mode options with game.savestatsandsettings_menu(), so
part of the blame does fall on me.

Anyways, this is fixed now.
2021-03-06 22:10:22 -05:00
Misa
be379733b6 De-duplicate toggling flip mode in Input.cpp
Flip Mode toggling is now no longer copy-pasted.
2021-03-06 17:55:01 -05:00
Misa
8169a26f46 Fix glitchy behavior switching soundtracks on silence
If there was absolutely no music playing, and you went to the in-game
options to switch between MMMMMM and PPPPPP, the behavior would be a bit
glitchy.

If you started with PPPPPP, switching once to MMMMMM wouldn't play
anything, but then switching back to PPPPPP would play MMMMMM track 15.
Then switching back to MMMMMM wouldn't do anything, but then switching
back to PPPPPP again would play PPPPPP track 15 - and from there, the
behavior is stable.

If you started with MMMMMM, switching once to PPPPPP would play MMMMMM
track 15. Then switching back to MMMMMM wouldn't do anything, but then
switching back to PPPPPP would play PPPPPP track 15 - and as above, the
behavior is stable after that.

Anyways, the point is, -1 shouldn't be passed to musicclass::play()
unless you want glitchy things. And I'm not patching -1 out of
musicclass::play() itself, because passing negative numbers results in a
useful glitch (that's existed since 2.2) where you can play MMMMMM
tracks while having PPPPPP selected, effectively doubling the amount of
usable music tracks within a custom level; it also seems like the game
does -1 checks elsewhere, so I'm just being consistent with the rest of
the game (although, yes, I am technically single-case patching this).
2021-03-06 17:54:06 -05:00
Misa
0f36cdce0d Use SDL_floor() instead of libc floor()
Now there is one less dependency on libc.
2021-03-06 16:01:29 -05:00
Misa
acfe4c294d Fix transitive includes in GraphicsUtil.cpp
I ran Include What You Use on the file, and a BUNCH of transitive
includes showed up.

colourTransform is used in the file, so GraphicsUtil.h needs to be
included. libc floor() is used in the file, so math.h needs to be
included (I'm removing this next...). NULL is used, so stddef.h. And
stdlib.h is used because we use rand() directly instead of going through
fRandom(). Speaking of which, we use fRandom(), so Maths.h needs to be
included, too.
2021-03-06 16:01:29 -05:00
Misa
c1572de9e2 Make one-way recolors check for specific files
So, 2.3 added recoloring one-way tiles to no longer make them be always
yellow. However, custom levels that retexture the one-way tiles might
not want them to be recolored. So, if there are ANY custom assets
mounted, then the one-ways will not be recolored. However, if the XML
has a <onewaycol_override>1</onewaycol_override> tag, then the one-way
will be recolored again anyways.

When I added one-way recoloring, I didn't intend for any custom asset to
disable the recoloring; I only did it because I couldn't find a way to
check if a specific file was customized by the custom level or not.

However, I have figured out how to do so, and so now tiles.png one-way
recolors will only be disabled if there's a custom tiles.png, and
tiles2.png one-way recolors will only be disabled if there's a custom
tiles2.png.

In order to make sure we're not calling PhysFS functions on every single
deltaframe, I've added caching variables, tiles1_mounted and
tiles2_mounted, to Graphics; these get assigned every time
reloadresources() is called.
2021-03-06 16:00:57 -05:00
Misa
34865a8ef1 Add FILESYSTEM_isAssetMounted()
This function will check if a specific file is a mounted per-level
custom asset, instead of being a variable that's true if ANY file is a
mounted asset.
2021-03-06 16:00:57 -05:00
Misa
ca4afcc140 De-duplicate one-way recolor conditional
Now you only have to call one function (and pass it a tile number) to
figure out if you should recolor a one-way tile or not, and you don't
have to copy-paste.
2021-03-06 16:00:57 -05:00
Misa
b4dd516d7d Move assetdir off of Graphics
It's only used in FileSystemUtils and never anywhere else, especially
not Graphics. Why is this on Graphics again?

It's now a static variable inside FileSystemUtils. It has also been
renamed to assetDir for consistency with saveDir and levelDir. Also,
it's a C string now, and is no longer an STL string.
2021-03-06 16:00:57 -05:00
Misa
22ced8b59b Don't use std::strings when comparing key names
There's no need to create an std::string for every single element just
to see if it's a key name.

At least in libstdc++, there's an optimization where std::strings that
are 16 characters or less don't allocate on the heap, and instead use
the internal 16-char buffer directly in the control structure of the
std::string. However, it's not guaranteed that all the element names
we'll get will always be 16 chars or less, and in case the std::string
does end up allocating on the heap, we have no reason for it to allocate
on the heap; so we should just convert these string comparisons to C
strings instead.
2021-03-06 13:40:31 -05:00
Terry Cavanagh
1163a8cca4
Merge pull request #647 from InfoTeddy/general-bug-fixes-4
Copy blend mode to recreated surfaces, fixing room name background during menu animation in Flip Mode
2021-03-06 16:56:28 +10:30
Terry Cavanagh
4f27c9366b
Merge pull request #646 from InfoTeddy/general-bug-fixes-3
Fix up/down controls being reversed in in-game menu in Flip Mode
2021-03-06 16:55:55 +10:30
Misa
255a6108c8 Fix up/down being reversed in in-game menu in Flip Mode
This bug is technically NOT a regression - the code responsible for it
has been around since the source release.

However, it hasn't been a problem until Graphic Options and Game Options
were added to the pause screen. Since then, if you opened the pause menu
in Flip Mode, pressing up would move to the menu option below, and
pressing down would move to the menu option above. Notably, left and
right still remain the same.

This is because the map screen input code assumes that the menu options
will be flipped around - however, this has never been the case. What
happens instead is that the menu options get flipped around time when in
Flip Mode - flipping what's already flipped - so it ends up the same
again.

(Incidentally enough, the up/down reversing code is present on the title
screen, and is correct - if you happen to set graphics.flipmode to true
on the title screen, the title screen doesn't negate the flipped menu
options, so pressing up SHOULD be treated like pressing down, and vice
versa. However, in 2.3, it's not really possible to set
graphics.flipmode to true on the title screen without using GDB or
modifying the game. In 2.2 and previous, you can just complete the game
in Flip Mode, and the variable won't be reset; 2.3 cleaned up all exit
paths to the menu to make sure everything got reset.)

This isn't a problem when there's only two options, but since 2.3 adds
two more options to the pause screen, it's pretty noticeable.

Anyway, this is fixed by simply removing the branch of the
graphics.flipmode if-else in mapinput(). The 'else' branch is now the
code that gets executed unconditionally. Don't get confused by the diff;
I decided to unindent in the same commit because it's not that many
lines of code.
2021-03-05 20:59:03 -08:00
Misa
d19a6cc437 Copy blend mode to recreated surface
This fixes a "root cause" bug (that's existed since 2.2 and below) where
recreated surfaces wouldn't preserve the blend mode of their original
surface.

The surface-level (pun genuinely unintended) bug that this root bug
fixes is the one where there's no background to the room name during the
map menu animation in Flip Mode.

This is because the room name background relies on graphics.backBuffer
being filled with complete black. This is achieved by a call to
ClearSurface() - however, ClearSurface() actually fills it with
transparent black (this is not a regression; in 2.2 and previous, this
was an "inlined" FillRect(backBuffer, 0x00000000)). This would be okay,
and indeed the room name background renders fine in unflipped mode - but
it suddenly breaks in Flip Mode.

Why? Because backBuffer gets fed through FlipSurfaceVerticle(), and
FlipSurfaceVerticle() creates a temporary surface with the same
dimensions and color masks as backBuffer - it, however, does NOT create
it with the same blend mode, and kind of sort of just forgets that the
original was SDL_BLENDMODE_NONE; the new surface is SDL_BLENDMODE_BLEND.
Thus, transparency applies on the new surface, and instead of the room
name being drawn against black, it gets drawn against transparency.
2021-03-05 20:58:46 -08:00
Misa
aca33e5587 De-duplicate surface recreation in GraphicsUtil
Here I'm using "surface recreation" to mean allocating a new surface
with almost the exact same properties as a given previous. As you can
see, GraphicsUtil likes to recreate surfaces all the time - copying the
masks and flags (unused lol) of an existing surface - and only varies it
by the dimensions of the new surface.

As you can see, this is a lot less wordy and a lot less repetitive than
copy-pasting it a bunch.
2021-03-05 20:47:47 -08:00
Misa
d0e497a95a Reverse menu animation direction in Flip Mode
In normal mode, the room name is at the bottom of the screen. When you
bring up the map screen, it appears as if the room name is moving up
from the bottom of the screen, and the map screen is "pushing" it up.
The effect is pretty seamless, and when I first played the game (back in
2014), I thought it was pretty cool.

However, in Flip Mode, the room name is at the top of the screen. So one
would expect the menu animation to come from above the screen. Well, no,
it still goes from the bottom of screen; ruining the effect because it
seems like there are two room names on the screen, when there ought to
be only one.

To be fair, I only noticed this while fixing another bug now, but it's
one of those things you can't unsee (I have cursed you with knowledge!);
not to mention that I probably only didn't notice this because I don't
play in Flip Mode that often (and I'd wager almost no one does; Flip
Mode previous to 2.3 seems to have been really untested, like I said
in #165). It feels like a bit of an oversight that the direction of the
animation is the same direction as in unflipped mode. So I'm fixing
this.
2021-03-05 19:24:54 -08:00
Misa
7ce87d7b13 Fix prompt fade out when activating overlapping zones
If you stood in two activity zones at once, you'll automatically select
the one that got created first. And when you activated it, the activity
zone prompt would switch to fading out the prompt of the OTHER activity
zone, the one you didn't activate.

This wasn't a problem in 2.2 and previous, because the fading animation
was simply bugged and defaulted to being solid black. However, in 2.3,
the fading animation is fixed, so this is possible.

Also, this really only happens in the main game. Since there's only one
type of useful activity zone in custom levels - namely the terminal
activity zone - if two activity zones did happen to overlap, activating
one of them wouldn't result in visibly fading out a different activity
zone (because they both look the same); furthermore custom level makers
are careful to not overlap terminal activity zones, lest this result in
player confusion; furthermore the placed activity zones only cover a
small area, whereas in the main game, crewmates' activity zones are
pretty big.

(Technically, you CAN create main game activity zones in custom levels,
but those are hardcoded to call main game scripts, and basically nobody
uses them.)

So what's the solution? Simply adding game.hascontrol and script.running
checks to the updating of game.activity_last[prompt|r|g|b].

Why not add those checks to the assignment of game.activeactivity, just
above? Because that would introduce a frame ordering issue (that
would NOT be (automatically) fixed by #535) where the eligibility of
pressing Enter on an activity zone now checks if you were standing in an
activity zone LAST frame, and not THIS frame. (I tested this with
libTAS.) Better to fiddle with the rendering code than fiddle with the
actual physics code.

The specific spot I used to test this was standing in Violet's activity
zone and the activity zone of the ship radio terminals (the three
terminals on the ground in her room); the ship radio terminals are
first-placed, so if you're testing this (and you should!), make that the
prompt is of the ship radio activity zone before activation.
2021-03-05 18:00:20 -05:00
Misa
9f69506acf Move activity prompt render updating to gamerenderfixed()
This probably should've been moved to RenderFixed a while ago, because
it's unnecessary to run this on every single deltaframe.

The only minor wrinkle here is that this means rendering of activity
zone fades will be delayed for 1 frame, but #535 will fix that.
2021-03-05 18:00:20 -05:00
Misa
84928f8210 Fix regression being able to activate activity zones during cutscenes
Since you're now allowed to bring up the map screen during cutscenes,
you've also been able to activate activity zones and teleporter prompts
during cutscenes. This only really affects custom levels; nowhere in the
main game can you overlap with an activity zone while in a cutscene.

To fix this, I've just added a script.running check to Enter keybind
processing.
2021-03-05 13:28:57 -05:00
Misa
4896f475ca Fix returning from game pad options not updating tower color
I was looking through all calls to game.returnmenu(), and I noticed that
the return option in the game pad screen didn't have a
map.nexttowercolour(). I tested it and, yep, returning from there
doesn't update the background color.

So that should be fixed now.
2021-03-05 10:03:35 -05:00
Misa
98bfd43887 Remove music.niceplay() when returning from credits6
I'm... not sure why this was here? It's absolutely not needed.

I'm guessing maybe at one point during development, there might have
been wanted a special song to be played during the credits, or no song
at all (although the function being niceplay() instead of play() seems
to support the first possibility) - but there's no need for this to be
here.
2021-03-05 10:03:35 -05:00
Misa
37947814aa Remove unnecessary currentmenuoption reassignments
Now that recreating the same menu keeps currentmenuoption, we can remove
all these superfluous assignments. This means repeating ourselves less;
in case the option numbers change in the future, we won't have to
remember to update these reassignments, too.
2021-03-05 10:03:35 -05:00
Misa
ac04281a9f Keep currentmenuoption if it's the same menu
When recreating the same menu, there's basically no reason to reset the
currently-selected menu option. (Also, no need to worry about indexing
out of bounds or anything - the number gets checked while iterating over
all menu options; it's never used to actually index anything. At worst
there might be a 1-frame flicker as the bounds code in gameinput() kicks
in, but that shouldn't happen anyways.)
2021-03-05 10:03:35 -05:00
Misa
502a34bf64 Add previous song option to editor music screen
This is just a small quality-of-life feature - it's annoying to have to
press ACTION 15 times in order to cycle back through to the previous
song.
2021-03-05 00:57:11 -08:00
Misa
b419cfe29e Ignore zip files in level metadata loading
Zip files that have been successfully mounted in editorclass::loadZips()
will now be ignored when the game does its second pass over the levels
directory. Otherwise, this would produce a superfluous error message,
because the game would attempt to parse the zip file as a level file
(when it's not a level file and is in fact a binary file).
2021-03-04 20:07:47 -05:00
Misa
9e4076a418 Add FILESYSTEM_isMounted()
This returns if the file given is mounted or not. 2.3 added level zip
support, so whenever the game loads level metadata, it will mount any
zip files in the levels directory; this function can be used to check if
any of those files have been mounted, and ignore them if so.
2021-03-04 20:07:47 -05:00
Misa
6efed0740b Ignore directories when loading level metadata
Otherwise, this would produce a superfluous warning message in the
console. Directories are now ignored and never attempted to be opened;
so now any warning messages printed out are genuine file that something
has genuinely gone wrong with.

Well, there's still a warning message printed if there's a symlink to a
directory; this is rarer, but it's still a false positive.
2021-03-04 20:07:47 -05:00
Misa
838ffbe68f Add FILESYSTEM_isFile()
This function will be used to differentiate files from directories.

Or at least that was the hope. Symlink support was added in 2.3, but it
doesn't seem like PHYSFS_stat() lets you follow the symlink to check if
what it points to is itself a file or directory. And there doesn't seem
to be any function to follow the symlink yourself...

So for now, this function considers symlinks to directories to be files.
2021-03-04 20:07:47 -05:00
Misa
d938a18504 Abstract zip loading to FileSystemUtils
editor.cpp no longer calls PhysFS functions directly; its physfs.h
include can now be dropped.
2021-03-04 19:10:53 -05:00
Misa
7316833f95 Fix return value of PHYSFS_readBytes() being stored in a smaller size
PHYSFS_readBytes() returns a PHYSFS_sint64, but we forcefully shove it
into a 32-bit signed integer.

Fixing the type of this doesn't have any immediate consequences, but
it's good for the future in case we want to use the return value for
files bigger than 2 gigabytes; it doesn't harm us in any way, and it's
just better housekeeping.
2021-03-04 18:22:31 -05:00
Misa
5af570e75b Set length to 0 if PHYSFS_fileLength() is negative
PHYSFS_fileLength() returns -1 if the file size can't be determined. I'm
going to set it to 0 instead, because it seems like that's more
well-behaved with consumers.

Take lodepng_decode24() or lodepng_decode32(), for example - from a
quick glance at the source, it only takes in a size_t (an unsigned
integer) for the filesize, and one of the first things it does is malloc
with the given filesize. If the -1 turns into SIZE_MAX and LodePNG
attempts to allocate that many bytes... well, I don't know of any
systems that have 18 exabytes of memory. So that seems pretty bad.
2021-03-04 18:22:31 -05:00
Misa
888844cd3a Fix return value of PHYSFS_fileLength() being stored in a smaller size
The function returns a PHYSFS_sint64, but we forcefully shove it into a
PHYSFS_uint32. This means we throw away all the negative numbers, which
is bad because the function returns -1 if the size of the file can't be
determined; plus, we also throw away 32 bits of information, reducing
our range of supported file sizes from 9 exabytes to 4 gigabytes.

File size support is only as good as the weakeast link, and it looks
like one of the consumers of FILESYSTEM_loadFileToMemory(),
SDL_RWFromConstMem(), only takes in a signed 32-bit integer of size;
however, I would still like to do at least the bare minimum to support
as many file sizes as we can, and changing types around is one of those
bare minimums.
2021-03-04 18:22:31 -05:00
Misa
88b3390e7d Fix regression with background of level minimaps not being black
I had misread this line in #629 and thought that it was just clearing
the entire surface, when really it was filling the surface with opaque
black. ClearSurface() would instead make it transparent, which would
mean when it got drawn, it would get drawn against blue, and not black.
Whoops.
2021-03-03 15:24:20 -05:00
Misa
9789848b36 Remove unused x and y attributes of blockclass
These float attributes are assigned to, and then never read again. The
coordinate systems of blocks are a bit of a mess - some use xp/yp, some
use xp/yp and rect.x/rect.y - but I can confidently say that these are
never used, because it compiles fine if I remove the attributes from the
class, plus remove all assignments to it.
2021-02-27 18:27:28 -05:00
Misa
758d201296 Fix transitive includes in Screen.cpp
Screen.cpp wasn't explicitly including SDL.h, instead relying on
Screen.h to include it.

It was also relying on SDL.h to include stdio.h on Linux, which breaks
because SDL.h doesn't include stdio.h on Windows. So stdio.h is now
explicitly included as well.

stdlib.h is not used in this file.
2021-02-27 14:26:08 -05:00
Misa
9997b28757 Add comment to magic numbers in FILESYSTEM_mountassets()
Now you know why there's a 7, a 14, and a 1 in the first SDL_strlcpy()
of the function.
2021-02-27 01:40:05 -05:00
Misa
57df734b1c Remove "data" checks from FILESYSTEM_mountassets()
After reasoning about it for a bit, there's no reason for these checks
to be here. `zip_normal` will either be
/home/infoteddy/.local/share/VVVVVV/levels if the asset directory is a
directory, or levels/levelname.zip if the asset directory is inside the
same zip as the level is. I don't see how they could ever be data.zip.

My guess is because of the VCE bug where it messed up its search path,
and before that bug was fixed, it had to be worked around here by
explicitly blacklisting data.zip here. When the assets mounting stuff
was ported from VCE to vanilla, vanilla didn't have the problem, and so
this data.zip blacklisting stuff was unnecessary.

Either way, I see no reason for this, so I'm going to remove it.
2021-02-27 01:40:05 -05:00
Misa
b2e748cad1 Refactor FILESYSTEM_mount[assets] to not use the STL
There is no need to use heap-allocated strings here, so I've refactored
them out. I've also cleaned up both of the functions a bit, because the
line spacing of the previous version was completely non-existent, brace
style was same-line instead of next-line, and the variable names were a
bit misleading (in FILESYSTEM_mountassets(), there is a `zippath` AND a
`zip_path`, which are two completely different variables).

Also, FILESYSTEM_mount() now prints an error message and bails if
PHYSFS_getRealDir() returns NULL, whereas it didn't do that before.
2021-02-27 01:40:05 -05:00
Misa
452ef3b511 Rename FILESYSTEM_directoryExists() to FILESYSTEM_exists()
The function is literally just an alias for PHYSFS_exists(), which does
not exclusively check for directories. Plus, the function is also used
to check if a non-directory file exists. Why is this function named
"directoryExists"?!
2021-02-27 01:40:05 -05:00
Misa
f372c22ce2 Don't export FILESYSTEM_directoryExists()
This function is never used outside of FileSystemUtils.cpp, so there's
no reason to export it.
2021-02-27 01:40:05 -05:00
Misa
c27bbaaecc Fix up log messages in FILESYSTEM_mountassets()
The info message when a .data.zip file is mounted is now differentiated
from the message when an actual directory is mounted (the .data.zip
message specifies ".data.zip").

The error message for an error occurring when loading or mounting a .zip
is now capitalized.

The "Custom asset directory does not exist" now uses puts(), because
there's no need to use printf() here.
2021-02-27 01:40:05 -05:00
Misa
c68efac032 Remove unneeded pKey when loading room property XML elements
I don't know why this is here; it's unused. I don't know why the
compiler doesn't warn about this being unused either - maybe it's
secretly being used? That also means I'm not sure if the compiler is
optimizing this away or not. Anyway, this is getting removed.
2021-02-27 01:40:05 -05:00
Misa
d590463834 Use less STL when loading entity XML elements
The STL here cannot be completely eliminated (because the custom entity
object uses std::string), but at least we can avoid unnecessarily making
std::strings until the very end.
2021-02-27 01:40:05 -05:00
Misa
2aeb3f35ce Remove std::strings from levelZipCallback()
There's basically no reason to use heap-allocated strings here, so it's
all gone now.
2021-02-27 01:40:05 -05:00
Misa
5d4c1b7e9d Refactor endsWith() to not use the STL
There's not really any reason for this function to use heap-allocated
strings. So I've refactored it to not do that.

I would've used SDL_strrstr(), if it existed. It does not appear to
exist. But that's okay.
2021-02-27 01:40:05 -05:00
Misa
3171a97160 Replace all SDL_RWFromMem() with SDL_RWFromConstMem()
Since we're not going to be writing to any of these RWops, we might as
well just ensure that we don't by using SDL_RWFromConstMem().
2021-02-25 19:39:48 -05:00
Misa
4a8c0a38ee Use SDL allocators for PhysFS
PhysFS by default just uses system malloc(), realloc(), and free(); it
provides a way to change them, with a struct named PHYSFS_Allocator and
a function named PHYSFS_setAllocator().

According to PhysFS docs, this function should be called before
PHYSFS_init(), which is why this allocator stuff is handled in
FileSystemUtils.cpp.

Also, I've had to make two "bridge" functions, because PHYSFS_Allocator
wants pointers to functions taking in `PHYSFS_uint64`s, not `size_t`s.
2021-02-25 19:39:23 -05:00
Misa
38d5664601 Change all surface-clearing FillRect()s to use ClearSurface()
ClearSurface() is less verbose than doing it the old way, and also
conveys intent clearer. Plus, some of these FillRect()s had hardcoded
width and height values, whereas ClearSurface() doesn't - meaning this
change also has better future-proofing, in case the widths and heights
of the surfaces involved change in the future.
2021-02-25 19:38:25 -05:00
Misa
e545c89b5e Combine declaration/initialization of color in two FillRect()s
Just a small simplification, so it's both declared and initialized on
the same line.
2021-02-25 19:38:25 -05:00
Misa
e641063d68 Fix two versions of FillRect() unnecessarily creating SDL_Rects
When you pass NULL in for the SDL_Rect* parameter to SDL_FillRect(), SDL
will automatically fill the entire surface with that color. There's no
need for us to create the SDL_Rect ourselves.
2021-02-25 19:38:25 -05:00
Misa
163b85d206 Add ClearSurface()
This is a function that does what it says - it clears the given surface.
This just means doing a FillRect(), but it's better to use this function
because it conveys intent better.
2021-02-25 19:38:25 -05:00
Misa
994f47e7d8 Remove commented-out code from Graphics.cpp and GraphicsUtil.cpp
This is pretty old commented-out code from earlier versions of the game;
they are no longer useful, and are just distracting. If we need them, we
can always refer back to this commit (but I sincerely doubt that we'll
need them).
2021-02-25 19:38:25 -05:00
Misa
6a3a1fe147
Explicitly declare void for all void parameter functions (#628)
Apparently in C, if you have `void test();`, it's completely okay to do
`test(2);`. The function will take in the argument, but just discard it
and throw it away. It's like a trash can, and a rude one at that. If you
declare it like `void test(void);`, this is prevented.

This is not a problem in C++ - doing `void test();` and `test(2);` is
guaranteed to result in a compile error (this also means that right now,
at least in all `.cpp` files, nobody is ever calling a void parameter
function with arguments and having their arguments be thrown away).
However, we may not be using C++ in the future, so I just want to lay
down the precedent that if a function takes in no arguments, you must
explicitly declare it as such.

I would've added `-Wstrict-prototypes`, but it produces an annoying
warning message saying it doesn't work in C++ mode if you're compiling
in C++ mode. So it can be added later.
2021-02-25 17:23:59 -05:00
Misa
0e313d0d75 Remove unneeded <algorithm> includes
One of these days, I need to get around to running Include What You Use
on this codebase. Until then, while I was working on #624, I noticed
these; I'm removing them now.
2021-02-20 00:12:11 -05:00
leo60228
c1950037c2
Use SDL_OpenURL in FILESYSTEM_openDirectory (#623)
The recently released SDL 2.0.14 adds a native function for opening URIs
from the host system, superseding the OS-specific implementations of
FILESYSTEM_openDirectory.
2021-02-19 21:16:19 -05:00
Misa
4e7d63cf09 Remove assigning block type to -1 when disabling them
This fixes a regression where moving platforms had no collision. Because
their width and height would be maintained, but their type would be -1.
(Also because I didn't test enough.)

In #565, I decided to set blocks' types to -1 when disabling them, to be
a bit safer in case there was some code that used block types but not
their width and heights. However, this means that when blocks get
disabled and re-created in the platform update loops, their types get
set to -1, which effectively also disables their collision.

In the end, I'll just have to compromise and remove setting blocks to
type -1. Because in a better world, we shouldn't be destroying and
creating blocks constantly just to move some platforms - however, fixing
such a fundamental problem is beyond the scope of at least 2.3 (there's
also the fact that this problem also results in some bugs that are a
part of compatibility, whether we like it or not). So I'll just remove
the -1.
2021-02-19 18:21:40 -05:00
Misa
cd5408e396 Fix potential out-of-bounds in next_split_s() wrt buffer length
next_split_s() could potentially commit out-of-bounds indexing if the
amount of source data was bigger than the destination data.

This is because the size of the source data passed in includes the null
terminator, so if 1 byte is not subtracted from it, then after it passes
through the VVV_min(), it will index 1 past the end of the passed buffer
when null-terminating it.

In contrast, the other argument of the VVV_min() does not need 1
subtracted from it, because that length does not include a null
terminator (next_split() returns the length of the substring, after all;
not the length of the substring plus 1).

(The VVV_min() here also shortens the range of values to the size of an
int, but we'll probably make size_t versions anyway; plus who really
cares about supporting massively-sized buffers bigger than 2 billion
bytes in length? That just doesn't make sense.)
2021-02-19 17:17:24 -05:00
Misa
72b8afcf7d Print if PHYSFS_enumerate() has an error
If PHYSFS_enumerate() isn't successful, we now print that it wasn't
successful, and print the PhysFS error message. (We should get that
logging thing going sometime...)
2021-02-19 07:12:13 -05:00
Misa
83976016c7 Refactor level dir listing to not use STL data marshalling
Note that level dir listing still uses plenty of STL (including the end
product - the `LevelMetaData` struct - which, for the purposes of 2.3,
is okay enough (2.4 should remove STL usage entirely)); it's just that
the initial act of iterating over the levels directory no longer takes
four or SIX(!!!) heap allocations (not counting reallocations and other
heap allocations this patch does not remove), and no longer does any
data marshalling.

Like text splitting, and binary blob extra indice grabbing, the current
approach that FILESYSTEM_getLevelDirFileNames() uses is a temporary
std::vector of std::strings as a middleman to store all the filenames,
and the game iterates over that std::vector to grab each level metadata.
Except, it's even worse in this case, because PHYSFS_enumerateFiles()
ALREADY does a heap allocation. Oh, and
FILESYSTEM_getLevelDirFileNames() gets called two or three times. Yeah,
let me explain:

1. FILESYSTEM_getLevelDirFileNames() calls PHYSFS_enumerateFiles().

2. PHYSFS_enumerateFiles() allocates an array of pointers to arrays of
   chars on the heap. For each filename, it will:

   a. Allocate an array of chars for the filename.

   b. Reallocate the array of pointers to add the pointer to the above
      char array.

      (In this step, it also inserts the filename in alphabetically -
      without any further allocations, as far as I know - but this is a
      COMPLETELY unnecessary step, because we are going to sort the list
      of levels by ourselves via the metadata title in the end anyways.)

3. FILESYSTEM_getLevelDirFileNames() iterates over the PhysFS list, and
   allocates an std::vector on the heap to shove the list into. Then,
   for each filename, it will:

   a. Allocate an std::string, initialized to "levels/".

   b. Append the filename to the std::string above. This will most
      likely require a re-allocation.

   c. Duplicate the std::string - which requires allocating more memory
      again - to put it into the std::vector.

      (Compared to the PhysFS list above, the std::vector does less
      reallocations; it however will still end up reallocating a certain
      amount of times in the end.)

4. FILESYSTEM_getLevelDirFileNames() will free the PhysFS list.

5. Then to get the std::vector<std::string> back to the caller, we end
   up having to reallocate the std::vector again - reallocating every
   single std::string inside it, too - to give it back to the caller.

And to top it all off, FILESYSTEM_getLevelDirFileNames() is guaranteed
to either be called two times, or three times. This is because
editorclass::getDirectoryData() will call editorclass::loadZips(), which
will unconditionally call FILESYSTEM_getLevelDirFileNames(), then call
it AGAIN if a zip was found. Then once the function returns,
getDirectoryData() will still unconditionally call
FILESYSTEM_getLevelDirFileNames(). This smells like someone bolting
something on without regard for the whole picture of the system, but
whatever; I can clean up their mess just fine.

So, what do I do about this? Well, just like I did with text splitting
and binary blob extras, make the final for-loop - the one that does the
actual metadata parsing - more immediate.

So how do I do that? Well, PhysFS has a function named
PHYSFS_enumerate(). PHYSFS_enumerateFiles(), in fact, uses this function
internally, and is basically just a wrapper with some allocation and
alphabetization.

PHYSFS_enumerate() takes in a pointer to a function, which it will call
for every single entry that it iterates over. It also lets you pass in
another arbitrary pointer that it leaves alone, which I use to pass
through a function pointer that is the actual callback.

So to clarify, there are two callbacks - one callback is passed through
into another callback that gets passed through to PHYSFS_enumerate().

The callback that gets passed to PHYSFS_enumerate() is always the same,
but the callback that gets passed through the callback can be different
(if you look at the calling code, you can see that one caller passes
through a normal level metadata callback; the other passes through a zip
file callback).

Furthermore, I've also cleaned it up so that if editorclass::loadZips()
finds a zip file, it won't iterate over all the files in the levels
directory a third time. Instead, the level directory only gets iterated
over twice - once to check for zips, and another to load every level
plus all zips; the second time is when all the heap allocations happen.

And with that, level list loading now uses less STL templated stuff and
much less heap allocations.

Also, ed.directoryList basically has no reason to exist other than being
a temporary std::vector, so I've removed it. This further decreases
memory usage, depending on how many levels you have in your levels
folder (I know that I usually have a lot and don't really ever clean it
up, lol).

Lastly, in the callback passed to PhysFS, `builtLocation` is actually no
longer hardcoded to just the `levels` directory, since instead we now
use the `origdir` variable that PhysFS passes us. So that's good, too.
2021-02-19 07:12:13 -05:00
Misa
532dbee4fd Contextualize PHYSFS_mountHandle() failure output
If PHYSFS_mountHandle() failed to mount a zip file, we would print
PhysFS's error message straight, without any surrounding context. This
seems a little weird, and doesn't maximize understanding for readers;
I've made it so now the error message is "Could not mount <zip file>:
<PhysFS error>".
2021-02-19 07:12:13 -05:00
Misa
f22691c32a Remove hardcoded check for "data" in level dir listing
When Ethan added PhysFS to the game, he put in a hardcoded check (marked
with a FIXME) that explicitly removed all filenames that were "data"
returned by PHYSFS_enumerateFiles(). Apparently this was due to a weird
bug with the function putting in "data" strings in its output in PhysFS
2.0.3; however, the game now uses PhysFS 3.0.2, and I could not
reproduce this bug on my system. (I also tested, and this also
straight-up ignores legitimate level filenames that just happen to be
"data" (without the .vvvvvv extension).)

After talking with Ethan in Discord DMs, I asked if we could remove this
check, and he said that we could. So I'm doing it now.
2021-02-19 07:12:13 -05:00
Misa
bf97e23fb6 Add static keywords to replace_all and tag finder funcs
These functions will never be used outside editor.cpp; they should
therefore be marked with the `static` keyword.
2021-02-19 07:12:13 -05:00
Misa
141181dee7 Refactor extra binary blob tracks to not use STL data marshalling
Just like I refactored text splitting to no longer use std::vectors,
std::strings, or temporary heap allocations, decreasing memory usage and
improving performance; there's no reason to use a temporary
heap-allocated std::vector to grab all extra binary blob indices, when
instead the iteration can just be more immediate.

Instead, what I've done is replaced binaryBlob::getExtra() with
binaryBlob::nextExtra(), which takes in a pointer to an index variable,
and will increment the index variable until it reaches an extra track.
After the caller processes the extra track, it is the caller's
responsibility to increment the variable again before passing it back to
getExtra().

This avoids all heap allocations and brings down the memory usage of
processing extra tracks.
2021-02-19 07:07:39 -05:00
Misa
7a02f174ac Add option to not use bundled TinyXML-2, PhysFS, and UTF8-CPP
If you configure the build with -DBUNDLE_DEPENDENCIES=OFF, then VVVVVV
will dynamically link with TinyXML-2 and PhysicsFS instead of using the
bundled source code in third_party/ and statically linking with them.

Unfortunately, it doesn't seem like distros package LodePNG, and LodePNG
isn't intended to be packaged, so we can't dynamically link with it, nor
can we use some system LodePNG header files somewhere else because those
don't exist.

UTF8-CPP is a special case, because no matter what, it's going to be
statically linked with the binary (it doesn't come as a shared object
file in any way). So with -DBUNDLE_DEPENDENCIES=OFF, we will use the
system UTF8-CPP header files instead of the bundled ones, but it will
still be statically linked in with the binary.

The main motivation for doing this is so if VVVVVV ever gets packaged in
distros, distro maintainers would be more likely to accept it if there
was an option to compile the game without bundled dependencies. Also, it
discourages modifying the third-party dependencies we have, because it's
always possible for someone to compile those dependencies without our
changes, with this CMake option.
2021-02-17 09:05:11 -05:00
Misa
3927bb9713 Fix entity and block indices after destroying them
This patch restores some 2.2 behavior, fixing a regression caused by the
refactor of properly using std::vectors.

In 2.2, the game allocated 200 items in obj.entities, but used a system
where each entity had an `active` attribute to signify if the entity
actually existed or not. When dealing with entities, you would have to
check this `active` flag, or else you'd be dealing with an entity that
didn't actually exist. (By the way, what I'm saying applies to blocks
and obj.blocks as well, except for some small differing details like the
game allocating 500 block slots versus obj.entities's 200.)

As a consequence, the game had to use a separate tracking variable,
obj.nentity, because obj.entities.size() would just report 200, instead
of the actual amount of entities. Needless to say, having to check for
`active` and use `obj.nentity` is a bit error-prone, and it's messier
than simply using the std::vector the way it was intended. Also, this
resulted in a hard limit of 200 entities, which custom level makers ran
into surprisingly quite often.

2.3 comes along, and removes the whole system. Now, std::vectors are
properly being used, and obj.entities.size() reports the actual number
of entities in the vector; you no longer have to check for `active` when
dealing with entities of any sort.

But there was one previous behavior of 2.2 that this system kind of
forgets about - namely, the ability to have holes in between entities.
You see, when an entity got disabled in 2.2 (which just meant turning
its `active` off), the indices of all other entities stayed the same;
the indice of the entity that got disabled stays there as a hole in the
array. But when an entity gets removed in 2.3 (previous to this patch),
the indices of every entity afterwards in the array get shifted down by
one. std::vector isn't really meant to be able to contain holes.

Do the indices of entities and blocks matter? Yes; they determine the
order in which entities and blocks get evaluated (the highest indice
gets evaluated first), and I had to fix some block evaluation order
stuff in previous PRs.

And in the case of entities, they matter hugely when using the
recently-discovered Arbitrary Entity Manipulation glitch (where crewmate
script commands are used on arbitrary entities by setting the `i`
attribute of `scriptclass` and passing invalid crewmate identifiers to
the commands). If you use Arbitrary Entity Manipulation after destroying
some entities, there is a chance that your script won't work between 2.2
and 2.3.

The indices also still determine the rendering order of entities
(highest indice gets drawn first, which means lowest indice gets drawn
in front of other entities). As an example: let's say we have the player
at 0, a gravity line at 1, and a checkpoint at 2; then we destroy the
gravity line and create a crewmate (let's do Violet).

If we're able to have holes, then after removing the gravity line, none
of the other indices shift. Then Violet will be created at indice 1, and
will be drawn in front of the checkpoint.

But if we can't have holes, then removing the gravity line results in
the indice of the checkpoint shifting down to indice 1. Then Violet is
created at indice 2, and gets drawn behind the checkpoint! This is a
clear illustration of changing the behavior that existed in 2.2.

However, I also don't want to go back to the `active` system of having
to check an attribute before operating on an entity. So... what do we
do to restore the holes?

Well, we don't need to have an `active` attribute, or modify any
existing code that operates on entities. Instead, we can just set the
attributes of the entities so that they naturally get ignored by
everything that comes into contact with it. For entities, we set their
invis to true, and their size, type, and rule to -1 (the game never uses
a size, type, or rule of -1 anywhere); for blocks, we set their type to
-1, and their width and height to 0.

obj.entities.size() will no longer necessarily equal the amount of
entities in the room; rather, it will be the amount of entity SLOTS that
have been allocated. But nothing that uses obj.entities.size() needs to
actually know the amount of entities; it's mostly used for iterating
over every entity in the vector.

Excess entity slots get cleaned up upon every call of
mapclass::gotoroom(), which will now deallocate entity slots starting
from the end until it hits a player, at which point it will switch to
disabling entity slots instead of removing them entirely.

The entclass::clear() and blockclass::clear() functions have been
restored because we need to call their initialization functions when
reusing a block/entity slot; it's possible to create an entity with an
invalid type number (it creates a glitchy Viridian), and without calling
the initialization function again, it would simply not create anything.

After this patch is applied, entity and block indices will be restored
to how they behaved in 2.2.
2021-02-16 19:31:23 -05:00
Misa
99276d7cdd Fix using ii instead of i in script serialization
Otherwise this results in an infinite loop. Whoops.
2021-02-16 00:13:39 -05:00
Misa
b243b116c9 Fix VVV_isxdigit() accepting A-Za-z instead of A-Fa-f
This function accepted more characters than it should have. (I made the
same mistake on my PR to SDL, embarassingly enough...)
2021-02-15 23:50:08 -05:00
Misa
3225be6d9e Fix is_number() accepting "-" as a number
The function would accept the string "-" as a number, when it isn't one.

To fix this, don't put it as part of the loop, just add another special
case.
2021-02-15 23:50:08 -05:00
Misa
28caa994e9 Add a blank line to the end of is_number()
This is to be consistent with is_positive_num().
2021-02-15 23:50:08 -05:00
Misa
4a853fec97 Add a blank line to the end of is_positive_num()
The return statement is too close to the block above it, so I want to
space it out for aesthetics.
2021-02-15 23:50:08 -05:00
Misa
30b0fc828f Change is_number() increment to pre-increment
This is also to be consistent with is_positive_num().
2021-02-15 23:50:08 -05:00
Misa
62533300b0 Change is_number() iterator type to size_t
This is to be consistent with is_positive_num().
2021-02-15 23:50:08 -05:00
Misa
7ea70ad1d6 Fix is_number() returning true for empty strings
Just like is_positive_num(), an empty string is not a number.

I've also decided to unroll iteration 0 of the loop here so readability
is improved; this happens to also knock out the whole "accepting empty
string" thing, too.
2021-02-15 23:50:08 -05:00
Misa
b9bf2be436 Fix is_positive_num() returning true for empty strings
To account for empty strings, we simply have to special-case them.
Simple as that.

This was also a problem with the previous std::string implementation of
this function; regardless, this is fixed now.
2021-02-15 23:50:08 -05:00
Misa
89bddf98b6 Refactor is_positive_num() to use C-strings
This makes the future move to C easier, as well as just shows that it's
plain unnecessary to use an std::string here.
2021-02-15 23:50:08 -05:00
Misa
825dbac228 Make const the 'hex' argument of is_positive_num()
Always good to get in the habit of clearly marking arguments you won't
mutate as const. Makes the code easier to read.
2021-02-15 23:50:08 -05:00
Misa
62808ca97a Remove unnecessary static_cast in is_number()
Just like is_positive_num(), the implicit conversion here is completely
okay, and adding an explicit cast here introduces noise.
2021-02-15 23:50:08 -05:00
Misa
623f4edc6e Remove unnecessary static_casts in is_positive_num()
The implicit conversion is completely okay. Adding an explicit cast here
just creates noise for the reader to filter out.
2021-02-15 23:50:08 -05:00
Misa
84ac4a40c1 Refactor loading arrays from XML to not use the STL
The current way "arrays" from XML files are loaded (before this commit
is applied) goes something like this:

1. Read the buffer of the contents of the tag using TinyXML-2.

2. Allocate a buffer on the heap of the same size, and copy the
   existing buffer to it. (This is what the statement `std::string
   TextString = pText;` does.)

3. For each delimiter in the heap-allocated buffer...

   a. Allocate another buffer on the heap, and copy the characters from
      the previous delimiter to the delimiter you just hit.

   b. Then allocate the buffer AGAIN, to copy it into an std::vector.

4. Then re-allocate every single buffer YET AGAIN, because you need to
   make a copy of the std::vector in split() to return it to the caller.

As you can see, the existing way uses a lot of memory allocations and
data marshalling, just to split some text.

The problem here is mostly making a temporary std::vector of split text,
before doing any actual useful work (most likely, putting it into an
array or ANOTHER std::vector - if the latter, then that's yet another
memory allocation on top of the memory allocation you already did; this
memory allocation is unavoidable, unlike the ones mentioned earlier,
which should be removed).

So I noticed that since we're iterating over the entire string once
(just to shove its contents into a temporary std::vector), and then
basically iterating over it again - why can't the whole thing just be
more immediate, and just be iterated over once?

So that's what I've done here. I've axed the split() function (both of
them, actually), and made next_split() and next_split_s().

next_split() will take an existing string and a starting index, and it
will find the next occurrence of the given delimiter in the string. Once
it does so, it will return the length from the previous starting index,
and modify your starting index as well. The price for immediateness is
that you're supposed to handle keeping the index of the previous
starting index around in order to be able to use the function; updating
it after each iteration is also your responsibility.

(By the way, next_split() doesn't use SDL_strchr(), because we can't get
the length of the substring for the last substring. We could handle this
special case specifically, but it'd be uglier; it also introduces
iterating over the last substring twice, when we only need to do it
once.)

next_split_s() does the same thing as next_split(), except it will copy
the resulting substring into a buffer that you provide (along with its
size). Useful if you don't particularly care about the length of the
substring.

All callers have been updated accordingly. This new system does not make
ANY heap allocations at all; at worst, it allocates a temporary buffer
on the stack, but that's only if you use next_split_s(); plus, it'd be a
fixed-size buffer, and stack allocations are negligible anyway.

This improves performance when loading any sort of XML file, especially
loading custom levels - which, on my system at least, I can noticeably
tell (there's less of a freeze when I load in to a custom level with
lots of scripts). It also decreases memory usage, because the heap isn't
being used just to iterate over some delimiters when XML files are
loaded.
2021-02-15 23:24:31 -05:00
Misa
52959396bb Remove unneeded comments from scriptclass::scriptclass()
These comments were probably remnants of some late-night coding session
or something. Anyway, they're not needed; there's nothing to do with SDL
here, and the "Init" is obvious because the function is a constructor.
2021-02-15 23:24:31 -05:00
Misa
a113662050 Clean up resetting editor contents and scripts
Contents and scripts should be reset in editorclass::reset(); there's no
reason to reset them again right before you load them from an XML file
in editorclass::load().

Additionally, the resets now consistently use SDL_zeroa() (for contents)
and scriptclass::clearcustom() (for scripts).
2021-02-15 23:24:31 -05:00
Misa
fe77ada160 Clean up comments in script XML parsing code
I'm partial to slash-asterisk-style comments, so I'll use those here.
Also, having a space after the start of comments is good. I've also
removed the "Add the script if we have a preceding header" comments
since it can be inferred by reading the surrounding code.
2021-02-15 23:24:31 -05:00
Misa
4f224e1657 Clean up whitespace in script XML parsing code
There is now a space between all 'if's and their opening parentheses.
I've also cleaned up the blank line spacing so the code looks better.
2021-02-15 23:24:31 -05:00
Misa
ca973a6547 Unindent from collapsing empty pText check
As always, the diff algorithms do terribly with unindentation, so I'm
doing the actual unindenting in this commit.
2021-02-15 23:24:31 -05:00
Misa
8d5829dd26 Remove indentation level from checking for empty pText
Instead of checking the length() of an std::string, just check if
pText[0] is equal to '\0'.

This will have to be done anyway, because I'm going to get rid of the
std::string allocation here, and I noticed this inefficiency in the
indentation, so I'm going to remove it.

The actual unindent will be done in the next commit.
2021-02-15 23:24:31 -05:00
Misa
968bb5f960 De-duplicate <customlevelscore> loading to use LOAD_ARRAY_RENAME()
This now means every XML array loading is done with common,
re-duplicated code. The only exceptions to this are special cases other
than the the majority of cases; the majority being a simple matter of
reading an array of integers and putting it into another array.

Seems like the only reason I hadn't caught the <customlevelscore> tag
until now was because I was focused on de-duplicating all the array
loads in Game::loadstats() and below, forgetting about
Game::loadcustomlevelstats().
2021-02-15 23:24:31 -05:00
Misa
6dc423e78e Move LOAD_ARRAY[_RENAME] macros to before loadcustomlevelstats()
In order to be able to use the LOAD_ARRAY() and LOAD_ARRAY_RENAME()
macros in Game::loadcustomlevelstats(), they have to be moved to earlier
in the file.
2021-02-15 23:24:31 -05:00
Misa
3fcdc084d0 Refactor editor gotoroom shortcut to not use split()
Even if split() didn't use the STL, using this function here is a bit
unnecessary, because a simple SDL_strchr() suffices. Refactoring split()
to not use the STL will break this caller anyway, so I might as well
just refactor this to not use split() in the first place.

This refactor also properly checks if the inputs are valid integers. And
since split() is no longer used, it also rejects inputs ending with a
trailing comma as being invalid, too; this didn't happen previously.
It's intentional that I used is_number() here instead of
is_positive_num(), thus accepting negative numbers; in the future it
might be possible to have negative room coordinates.
2021-02-15 23:24:31 -05:00
Misa
fe8d163041 Fix reading uninitialized memory when creating Menu::levellist
Valgrind reported this.

The error here is that the buffer here is only guaranteed to be
initialized up until (and including) the null-terminator, by
SDL_snprintf(). Iterating over the entire allocated buffer is bad and I
should feel bad as the girl who wrote this code; doing that reads
uninitialized memory and passes it to SDL_tolower().

As a bonus, the iterator increment is now a preincrement instead of a
postincrement.
2021-02-15 23:07:35 -05:00
Misa
3226d4f312 Free loaded file in editorclass::getLevelMetadata()
This fixes memory leaking every single time a file gets loaded(!) when
the list of custom levels gets loaded(!!!), which Valgrind reports. This
memory leak is completely my bad; 2.2 properly frees the loaded file,
and VCE uses an std::unique_ptr - which I decided to ignore and not
think about why it would be there.

It's safe to do this free after uMem gets copied into std::string;
although, in the future, I *am* thinking about refactoring this function
(and the tag finder function) to not use std::strings, and I'll have to
be careful to make sure that the memory management with the file is
correct when I do so.
2021-02-15 23:07:35 -05:00
Misa
16fef54ae1 Pass 1 to Mix_LoadMUS_RW() in MusicTrack::MusicTrack()
This makes the freesrc argument of Mix_LoadMUS_RW() 1 instead of 0. If
the argument is nonzero, then the passed SDL_RWops will be automatically
freed when m_music is freed, too.

I don't know why this was 0 before. Setting it to 1 fixes a memory leak
that Valgrind reports (which turns into an actual leak every time custom
assets are mounted or unmounted).
2021-02-15 23:07:35 -05:00
Misa
8052f61cef Check if file can't be loaded in SoundTrack::SoundTrack()
This adds a check that the pointer passed to
FILESYSTEM_loadFileToMemory() isn't NULL, and if it is, just returns
early in the function, instead of continuing later and producing a
different, slightly-misleading error message.
2021-02-15 23:07:35 -05:00
Misa
f401cc6230 Unconditionally call FILESYSTEM_freeMemory() in SoundTrack constructor
Previously, it was guarded behind a check for the length, which is... I
guess still perfectly fine behavior, but there's no reason to have a
length check here; FILESYSTEM_freeMemory() uses SDL_free(), which does a
check that the pointer passed is non-NULL (the pointer that is passed
here, despite not being initialized upon declaration, is guaranteed to
be initialized by FILESYSTEM_loadFileToMemory() anyway, so).
2021-02-15 23:07:35 -05:00
Misa
b19daebeef Bail for all SDL_malloc() failures
Following Ethan's example of bailing (calling VVV_exit()) if
binaryBlob::unPackBinary() couldn't allocate memory, I've searched
through and found every SDL_malloc(), then made sure that if it returned
NULL, the caller would bail (because you can't do much when you're out
of memory).

There should probably be an error message printed when the process is
out of memory, but unPackBinary() doesn't print an error message for
being out of memory, so this can probably be added later. (Also we don't
really have a logging system, I'd like to have something like that added
in first before adding more messages.)

Also, this doesn't account for any allocators used by STL stuff, but
we're working on removing the STL, and allocation failure just results
in an abort anyway, so there's not really a problem there.
2021-02-15 23:07:35 -05:00
Misa
8aa5bb8aab Clean up all program close paths to use VVV_exit()
Wow, there are a lot of these. All of these exit paths now use
VVV_exit() instead, which attempts to save unlock.vvv and settings.vvv,
and also frees all resources so Valgrind is happy. This is a good thing,
because previously unlock.vvv/settings.vvv wouldn't be written to if we
decided to bail for a given reason.
2021-02-15 23:07:35 -05:00
Misa
de1e773b7f Remove <string.h> #include from main.cpp
We no longer use libc string functions, instead preferring SDL's; this
unused include should be removed.
2021-02-15 23:07:35 -05:00
Misa
d39fe96cf2 Move Script.cpp <limits.h> #include to proper place
It should be between the include of the corresponding header file for
the source file (Script.h) and the includes of other local header files
(the files that are specific to this codebase only); this is the
convention that includes in all other source files follow.

However, it seems like I misplaced this, so I'm fixing it now.
2021-02-15 23:07:35 -05:00
Misa
4964cb7bc3 Add VVV_exit()
This is just a function that calls the cleanup() in main.cpp, as well as
calls exit().

I would have liked to use SDL_ExitProcess() here, because that function
has ifdefs for different runtime environments. But alas, it's an
internal function and isn't exported. Ah well; exit() seems to be fine
anyway.
2021-02-15 23:07:35 -05:00
Misa
efbaeeffde Clean up all leftover resources in cleanup()
If there's a resource that doesn't otherwise need to be cleaned up and
is still alive upon program shutdown, then it should go in cleanup().

This cleans up Screen, GraphicsResources, Graphics buffers, Graphics
tiles, and musicclass audio upon program shutdown.

Even we technically don't NEED to clean these resources up ourselves
(the kernel is going to get rid of all of it anyway, else it'd be a
security problem), I'm doing this because otherwise Valgrind will
complain about these, and then it'd be difficult to see which memory
leaks are real and which are just "well this isn't really a leak but you
haven't freed this thing when the process exited, and that's technically
what a memory leak is".

These are all resources whose cleanup functions can be safely called
even if they haven't initialized anything yet.
2021-02-15 23:07:35 -05:00
Misa
39e316828e Add NULL guards to Game::savestats() and savesettings()
This is so those functions can safely be called when
graphics.screenbuffer is NULL.
2021-02-15 23:07:35 -05:00
Misa
1f1b39a77a Free base tilesheet image after processing it
This isn't a memory leak (not even Valgrind complains), because it gets
properly cleaned up in GraphicsResources::destroy(). Still, it's memory
that is just laying around not being used, and in the name of
deallocating things as soon as you no longer need them, we should
deallocate the base tilesheet images after we split all of them into
tiles.

This reduces the memory cost of all tilesheet images by half, since we
were essentially keeping around duplicates for nothing; this doesn't
really have much of an impact with conventional tilesheet sizes, since
they're usually small enough, but since 2.3 allowed for tilesheet images
of any size, this is a pretty big deal for really big tilesheet images.

It's okay to do this, even though they also get freed in
GraphicsResources::destroy(), because SDL_FreeSurface() does a NULL
check on the pointer passed to it, and we set the pointer to NULL after
freeing the surfaces.
2021-02-15 23:07:35 -05:00
Misa
6077015fdc Guard PHYSFS_deinit() with PHYSFS_isInit()
A quick glance at PhysFS source code will show that PhysFS will bail if
PHYSFS_deinit() is called if it's not initialized.

"Bail" here just means setting an error code and returning early, so
it's not that bad. Still, it's the principle of the thing, and I just
want to ensure that FILESYSTEM_deinit() can be safely called no matter
if the filesystem hasn't initialized yet; having an error set by PhysFS
kind of taints the whole safety thing, even if it does nothing wrong,
no?

(although, speaking of which, we should be handling all errors by
PhysFS, but that's for later...)
2021-02-15 23:07:35 -05:00
Misa
951e1653a6 Move cleanup code to separate function
I will need to re-use this code, so it's best that it not be
copy-pasted.
2021-02-15 23:07:35 -05:00