This removes the magic numbers previously used for controlling the fade
mode, which are really not readable at all unless you already know what
they mean.
0: FADE_NONE
1: FADE_FULLY_BLACK
2: FADE_START_FADEOUT
3: FADE_FADING_OUT
4: FADE_START_FADEIN
5: FADE_FADING_IN
There is also the macro FADEMODE_IS_FADING, which indicates when the
intention is to only check if the game is fading right now, which wasn't
clearly conveyed previously.
I also took the opportunity to clean up the style of any lines I
touched. This included rewriting if-else chains into case-switches,
turning one-liner if-then statements into proper blocks, fixing up
comments, and even commenting the `fademode == FADE_NONE` on the tower
spike checks (which, it was previously undocumented why that check was
there, but I think I know why it's there).
As for type safety, we already get some by transforming the variable
types into the enum. Assignment is prohibited without a cast. But,
apparently, comparison is perfectly legal and won't even give so much as
a warning. To work around this and make absolutely sure I made all
existing comparisons now use the enum, I temporarily changed it to be an
`enum class`, which is a C++11 feature that makes it so all comparisons
are illegal. Unfortunately, it scopes them in a namespace with the same
name as a class, so I had to temporarily define macros to make sure my
existing code worked. I also had to temporarily up the standard in
CMakeLists.txt to get it to compile. But after all that was done, I
found the rest of the places where a comparison to an integer was used,
and fixed them.
Previously, it was copy-pasted and slightly different, when really, they
ought to both be the exact same code.
It kind of pains me that the room name, glitch name, and hidden name
don't own their own memory, but, that's to be addressed later.
What's a bit annoying is that the `temp` variable used in
`teleporterrender` also ends up being reused later in the function. In
this case, I opted to just redeclare them when they are used anyway, to
make it clearer.
Apart from `teleporterrender` no longer calling `map.area` or caring
about `map.custommode`, it also no longer cares about
`graphics.fademode` being 0. I could never actually get this condition
to be false in practice, and I have absolutely no idea why it's there.
I'm guessing it could be some weird edge case rendering issue if the
screen is fully black? But I wouldn't know how to trigger that, and
anyway it should probably be fixed elsewhere. So I'm just going to
remove that conditional.
This is quite simple. Just use a function pointer that switches out
which function we're going to use.
...Or not. C++ syntax makes this a bit awful since the function is a
member of a class. Did I mention how much I don't like C++?
This enum is to just make each mode be readable, instead of mysterious
0/1/2 values. It's not a strictly-typed enum because we still have to
serialize it as ints in the XML, but it's better than just leaving them
as ints.
This also adds a NUM_SCALING_MODES enum, so we don't have to hardcode
that 3 when cycling scaling modes anymore.
This is mainly to make sure the game is definitely set to fullscreen in
Big Picture and on the Steam Deck, and to also remove windowed options
that wouldn't make sense if you're not on a desktop (toggling
fullscreen, resize to nearest). Those options would also be removed on
console and mobile too.
There's a bit of an annoying bug where if you launch the game in forced
fullscreen mode, but then exit and relaunch in normal mode, your game
will have fullscreen window sizes but it won't be fullscreen. This is
because forced fullscreen mode tries to preserve your non-forced
fullscreen setting, but due to the way window sizes are stored and
queried, it can't preserve the non-forced window size. This is a bit
difficult to work around, so I'm just putting in a FIXME here because we
can fix it later and I'd rather have a slightly buggy forced fullscreen
mode than not have one at all.
Closes#849.
Here's my notes on all the existing functions and what kind of time
formats they output:
- Game::giventimestring(int hrs, int min, int sec)
H:MM:SS
MM:SS
- Game::timestring()
// uses game.hours/minutes/seconds
H:MM:SS
MM:SS
- Game::partimestring()
// uses game.timetrialpar (seconds)
MM:SS
- Game::resulttimestring()
// uses game.timetrialresulttime (sec) + timetrialresultframes (1/30s)
MM:SS.CC
- Game::timetstring(int t)
// t = seconds
MM:SS
- Game::timestringcenti(char* buffer, const size_t buffer_size)
// uses game.hours/minutes/seconds/frames
H:MM:SS.CC
MM:SS.CC
- UtilityClass::timestring(int t)
// t = frames, 30 frames = 1 second
S:CC
M:SS:CC
This is kind of a mess, and there's a lot of functions that do the same
thing except using different variables. For localization, I also want
translators to be able to localize all these time formats - many
languages use the decimal comma instead of the decimal point (12:34,56)
maybe some languages really prefer something like 1時02分11秒44瞬...
Which I don't know to be correct, but it's good to be prepared for it
and not restrict translators arbitrarily to only changing ":" and "."
when we can start making the system better in the first place.
I added a new function, UtilityClass::format_time. This is the place
where all time formats come together, given the number of seconds and
optionally frames. I have simplified the above-mentioned functions
somewhat, but I haven't given them a complete refactor or renaming -
I mainly made sure that they all use the same backend so I can make the
formats consistent and properly localizable.
(And before we start shoving more temporary char buffers everywhere
just to get rid of the std::string's, maybe we need to think of a
globally used working buffer of size SCREEN_WIDTH_CHARS+1, as a
register of sorts, for when any line of text needs to be made or
processed, then printed, and then goes unused. Maybe help.textrow,
or something like that.)
As for this commit, the available time formats are now more consistent
and changed a little in some places. Leading zeroes for the first unit
are now no longer included, time trial results and the Super Gravitron
can now display hours when they went to 60 minutes before, and we now
always use .CC instead of :CC. These are the formats:
- H:MM:SS
- H:MM:SS.CC
- M:SS
- M:SS.CC
- S.CC (only used when always_minutes=false, for the Gravitrons)
Here's what changes to the current functions:
- Game::partimestring() is removed - it was used in two places, and
could be replaced by game.timetstring(game.timetrialpar)
- Game::giventimestring(h,m,s) and Game::timestring() are now wrappers
for the other functions
- The four remaining functions (Game::resulttimestring(),
Game::timetstring(t), Game::timestringcenti(buffer, buffer_size)
and UtilityClass::timestring(t)) are now wrappers for the "central
function", UtilityClass::format_time.
- UtilityClass::twodigits(int t) is now unused so it's also removed.
- I also added int UtilityClass::hms_to_seconds(int h, int m, int s)
I know earlier I removed the gameScreen extern in favor of using
screenbuffer, but that was only to be consistent. After further
consideration, I have found that it's actually really stupid.
There's no reason to be accessing it through screenbuffer, and it's
probably an artifact of 2.0-2.2 passing stack-allocated otherwise-global
classes everywhere through function arguments. Also, it leads to stupid
bugs where screenbuffer could potentially be NULL, which has already
resulted in various annoying crashes in the past. Although those could
be fixed by simply initializing screenbuffer at the very top of main(),
but, why not just scrap the whole thing anyway?
So that's what I'm doing.
As a nice side effect, I've removed the transitive include of Screen.h
from Graphics.h. This could've been done already since it only includes
it for the pointer anyway, but it's still good to do it now.
It's been long overdue that this variable be named properly. 2.2 added
integer scaling mode (thanks Ethan), 2.3 renamed it to scaling mode. Now
2.4 will properly call it what it is so people won't be confused by it.
The ScreenSettings struct member is renamed from stretch to scalingMode
along with the Screen class member being renamed, as well as the
toggleStretchMode function being renamed to toggleScalingMode as well.
Unfortunately, due to compatibility, we can't change the <stretch> XML
tag.
VVV_min/max are functions that only operate on ints, and SDL_min/max are
macros that operate on any type but double-evaluate everything.
I know I more-or-less said earlier that SDL_min/max were dumb but I've
changed my mind and think it's better to use them, taking care to make
sure you don't double-evaluate, rather than trying to generate your own
litany of functions with either your own hand-rolled generation macros,
C++ templates, C11 generics, or GCC extensions (that last one you'd
technically use in a macro but it doesn't really matter), all of which
have more downsides than just not double-evaluating.
And the upside of not double-evaluating is that you're disencouraged
from having really complicated single-line min/max expressions and
encouraged to precompute the values beforehand anyway so the final
min/max is more readable. And furthermore you'll notice when you
yourself end up doing double-evaluations anyway. I removed a couple
instances of Graphics::len() being double-evaluated in this commit (as
well as cleaned up some other min/max-using code). Although the only
downside to those double-evaluations was unnecessary computation,
rather than checking the wrong result or having multiple side effects,
thankfully, it's still good to minimize double-evaluations where
possible.
This is just in case the background gets changed by a custom level or
something to be something that would otherwise result in bad contrast.
Also if it needs to go outside the box for some reason. And I just like
the look of the outline.
This adds centiseconds to the in-game timer, as well as the time trial
timer.
This is to aid speedrun moderators in determining when exactly a run was
completed, which they can't easily do if the timer only has a precision
up to a second.
The problem was that it also needed to check that game.swnmode was true,
in addition to game.swngame being 1, to actually check that the Super
Gravitron was being played.
* Add `setactivityposition(x,y)`, add new textbox color `transparent`
This commit adds a new internal command as a part of the visual activity zone changes I've been making.
This one allows the user to reposition the activity zone to anywhere on the screen.
In addition, this commit adds the textbox color `transparent`, which just sets r, g and b to 0.
rgb(0, 0, 0) normally creates the color black, however in VVVVVV textboxes, it makes the background
of them invisible, and makes the text the off-white color which the game uses elsewhere.
* add new variables to hardreset
* Fix unwanted text centering; offset position by 16, 4
It makes sense for `setactivityposition(0, 0)` to place the activity zone in the default position,
so the x has been offset by 16, and the y has been offset by 4.
Text was being automatically centered, meaning any activity zone which wasn't centered had misplaced text.
This has been fixed by calculating the center manually, and offsetting it by the passed value.
It is impossible to get on the quicksave screen in time trials, because
Enter is always bound to restarting time trials in a time trial, and
there's no way to open the map screen otherwise.
So, I've decided to add a fun little message in case someone somehow
manages to get to this screen in a time trial.
As is typical, the code was copy-pasted to account for Flip Mode, and
then copy-pasted again to account for custom levels, leading to four
instances of the same code.
I clean this up while also improving code style. This is where the new
FLIP macro and the fixed PrintWrap help a lot - otherwise the "Game
saved ok!" screen would look really wrong without the height
corrections.
Flipping objects in Flip Mode needs to account for the heights of those
objects (that's why flipme text boxes in Flip Mode in 2.2 were
positioned wrongly).
Also, turn it into a macro instead of an inline function.
This changes the positions of all existing de-duplicated map menu text
in Flip Mode, but it'll be more correct.
Between accounting for Flip Mode and custom levels, this code was
copy-pasted three times, leading to _four_ instances of one code!
Anyways, I've cleaned it up. The position of the text in Flip Mode is
going to differ by 4 pixels from how it was previously, but that really
shouldn't matter.
While dying in No Death Mode was fixed to no longer say "One trinkets"
in 2.3, if you win in No Death Mode with one trinket, the game would say
"One trinkets".
So to fix this, just slot a ternary in there. The code is already kind
of bad anyways and is going to be refactored/de-STLed in the future
regardless, so I'm not feeling too badly about shoving a ternary in
there like that.
SDL just got an API to toggle VSync without having to tear down the
renderer ( libsdl-org/SDL#4157 ). We can remove the workaround and use
that instead. For now, we are putting it behind an ifdef until SDL
2.0.18 officially releases in November.
Fixes#831.
Constants.h will house constants like the screen size and others. But
basically only the screen size for now.
Now we don't have to type that "4 bytes per 40 chars (whole screen)"
comment everywhere...
Since those are all downstream recipients of either static storage or
memory that doesn't move for the duration of the custom level, it's okay
to make these be `const char*`s without having to redo any of the RAII
memory management.
mapclass::currentarea() is included in this as well. I also cleaned up
Tower.cpp's headers to fix some transitive includes because I was
removing UtilityClass.h includes from all other level files too.
The "Untitled room" names no longer show any coordinates, because doing
so would require complicated memory management that's completely
unneeded. No one will ever see them, and if they do they already know
they have a problem anyway. The only time they might be able to see them
is if they corrupted the areamap, but this was only possible in 2.2 and
previous by dying outside the room deaths array in Outside Dimension
VVVVVV, which has since been patched out. Besides, sometimes the
"Untitled room" gets overwritten by something else anyway (especially in
Finalclass.cpp), so it really, really doesn't matter.
Turns out, the r, g, and b arguments don't actually do anything!
There was a call to RGBf() in the function. RGBf() is just getRGB() but
first adds 128 and then divides by 3 to each of the color channels
beforehand. Unfortunately, RGBf() does not have any side effects, and
the function threw away the return value. Bravo.
This also reveals that the website images drawn in the credits in the
main menu are only recolored because of a stale `ct` set by the previous
graphics.bigprint(), and not because any color values were passed in to
drawimagecol()... What fun surprises the game has in store for me every
day.
I'm honestly not too sure why drawcustompixeltextbox ever existed? All
it seemed to do was draw even more horizontal/vertical tiles to finish
any gaps in the tiling... which was all completely unnecessary and
wasteful, because even the previous drawpixeltextbox implementation
covered all gaps in all custom level map sizes that I tried.
Anyway, that at least gets rid of one copy-pasted function.
This is a pretty hefty commit! But essentially, I made a new editorclass
object, and moved all functions and variables that only get used in the
in-game level editor to that class. This cleanly demarcates which things
are in the editor and which things are just general custom level stuff.
Then I fixed up all the callers. I also fixed up some NO_CUSTOM_LEVELS
and NO_EDITOR ifdefs, too, in several places.
This accompanies the editor.cpp -> CustomLevels.cpp change; I'll be
splitting out the editor functions in the next commit. The name of the
include guard has been changed as well, but not anything else.
This is because the y-position of the graphics.onscreen() check was a
little too high. Then their name (under Beta Testing) would suddenly
disappear too early. You'd have to look real close to spot it, but it
does happen. It's cuz the credits are all kinda hardcoded, which is
probably bad, but fixing that would have to come later...
I talked with Ethan earlier about this. For 2.3, he wanted me in GitHub
contributors (well, still separate from the rest), to really highlight
the source-code-release community-driven nature of 2.3, but he said it'd
be fine to put me in C++ credits in 2.4.
To match the option to nuke all main game save data, there is also now
an option to nuke all custom level save data separately (which is just
all custom level quicksaves, along with stars for level completion). It
has its own confirmation menu too. It does not delete any levels from
the levels folder.
Custom level quicksaves are NOT affected by the clear data menu, so the
player should be able to delete quicksaves this way. The quicksave
confirmation menu now has an extra option to delete the save (and that
option also has its own confirmation menu before deleting).
This error case can happen, but if it does, non-console users get an
ERROR page with no further information. So use setLevelDirError if this
failure mode happens. And Menu::errorloadinglevel needs to be changed to
accomodate that.
It's possible to get one page of levels by removing all the built-ins,
either by removing them directly from data.zip or by putting files with
the same filenames as them in your level folder that don't contain
nothing.
And hey, there's already a check for if no levels exist at all, so why
not check for this too?
If a zip file is improperly structured, a message will be displayed when
the player loads the level list.
This will only display the last-displayed improper zip, because there
only needs to be one displayed at a time. Also because doing anything
more would most likely require heap allocation, and I don't want to do
that.
Some people prefer the 2.2 behavior where unfocusing pauses the game,
but the music still plays. One such person is Trinket9 on the VVVVVV
Discord server, who wanted it that way.
The reason audio pausing was added in the first place was to prevent
desyncing music in levels with cutscenes that synced to music. Rather
than reverting it, let's add this option instead.
This is because the fade delay did not last long enough.
I was under the mistaken impression that the fade animation lasts for 15
frames. However, this does not account for the fact that the offset of
each fade bar is dependent on RNG, and the worst case scenario is that
they have an offset of 96 pixels (in the opposite direction of the
fade).
The actual fade animation timer accounts for the worst case scenario, so
the fade animation actually lasts for (320 pixels plus 96 pixels is 416
pixels, 416 pixels divided by 24 pixels per frame equals 17.333...
frames, but since the actual timer keeps adding/subtracting 24 pixels
per frame until it passes the 416-pixel threshold, that gets rounded up
to...) 18 frames.
And an extra frame to make it so deltaframe interpolation doesn't
suddenly stop on the last deltaframes before the screen is completely
black.
I also need to draw the screen black on the map screen when glitchrunner
mode is off, if there's a fadeout going on. Else that would introduce
yet another frame flicker.
Previously, turning glitchrunner mode on essentially locked you to
emulating 2.0, and turning it off just meant normal 2.3 behavior. But
what if you wanted 2.2 behavior instead? Well, that's what I had to ask
when a TAS of mine would desync in 2.3 because of the two-frame delay
fix (glitchrunner off), but would also desync because of 2.0 warp lines
(glitchrunner on).
What I've done is made it so there are three states to glitchrunner mode
now: 2.0 (previously just the "on" state), 2.2 (previously a state you
couldn't use), and "off". Furthermore, I made it an enum, so in case
future versions of the game patch out more glitches, we can add them to
the enum (and the only other thing we have to update is a lookup table
in GlitchrunnerMode.c). Also, 2.2 glitches exist in 2.0, so you'll want
to use GlitchrunnerMode_less_than_or_equal() to check glitchrunner
version.
In order to help players spot the difference between outlined text and
non-outlined text, we now outline the text outline text itself (if text
outline is enabled, of course). But drawing the outline alone doesn't
stand out enough, so we have to draw a solid backing against the text as
well, in order to properly show the contrast.
This makes the text much more readable against certain backgrounds (if
you have text outline enabled), especially against the Warp Zone
background (when you start in "This is how it is").
The text box drawn at the bottom of the map screen isn't wide enough, so
it's possible to see the corners on the right side of the text box if
you have custom graphics like I do.
The solution is to increase the width of the text box by one tile.
This is a lot of copy-pasted code, but a little bit of copy-pasting
never hurt anyone...
The keybind to interact with activity zones and teleporters is now
separate from the keybind to open the map, or return to the editor from
in-editor playtesting, or restart a time trial. The keybind is now E,
and the default controller bind is X. No controller button prompts, but
the game didn't have controller button prompts anyways, so whatever.
Doing this now because if people's muscle memory are going to be broken
by not being able to spam the map keybind anymore, at least we can help
a bit by changing the keybind so they can keep spamming it - their
muscle memory is going to be broken anyways.
This option has to be enabled by going to the speedrunner menu options
and selecting "interact button". It is disabled by default.
All prompt text needs to be string-interpolated every time they are
drawn, because it is possible for people to change which interact button
they use in the middle of gameplay, via the in-game options.
Closes#736.
This fixes the finalstretch tile shifting persisting if you return to
the main dimension and final_colormode isn't reset properly.
It's possible to do so in the main game by using a teleporter in
finalmode while having the Intermission 1 or 2 companion active.
For custom levels, level makers can make a setup that automatically
turns on finalstretch, goes to finalmode, and then returns to the main
dimension. The only thing being... as a level maker myself, this tile
shifting REALLY doesn't seem useful (and no one has ever used it because
the setup to do so hadn't really been found or documented until this
year). For one, the exact shift is randomized every time (there's an
fRandom() call to cycle the colors). For two, it goes away after the
player saves and reloads the level. And for three, it doesn't animate
like it does in finalmode (this is the biggest reason IMO).
Nevertheless, I've decided to keep support for this in custom levels, in
case someone in the future does want to use it and is okay with the
limitations.
This factors out the slowdown and invincibility conditionals to a
function. This means less copy-pasted code, and it also conveys intent
(that we don't want to allow competitive options if we have either of
these cheats enabled).
This function isn't implemented in the header because then we would have
to include Map.h for map.invincibility, and transitive includes are
evil. Although, map.invincibility ought to be on Game instead (it was
only mapclass due to 2.2-and-previous argument passing), but that's a
bunch of variable reshuffling that can be done later.
The game dereferences graphics.screenbuffer without checking it first...
it's unlikely to happen, but the least we can to do be safe is to add a
check and assert here.
This is a simple change - we draw minimap.png, instead of the generated
custom map, if it is a per-level mounted custom asset.
Custom levels have already been able to utilize minimap.png, but it was
limited - they could do gamemode(teleporter) in a script, and that would
show their customized minimap.png, but it's not like the player could
look at it during gameplay.
I would have done this earlier if I had figured out how to check if a
specific asset was mounted or not.
The config option has been removed. I'm going to implement something
that automatically shows and hides the mouse cursor whenever
appropriate, which is better than a config option.
In 2.2 and previous, the game would call resetgameclock() every frame
for the last 30 frames of the time trial countdown in order to make sure
it gets reset. This was in a render function, and didn't get brought out
in 2.3, so 2.3 resets the game clock *while rendering*, which is kinda
bad and is an oversight on my part for not noticing.
Instead of doing that, just add a conditional to the timer so that it
won't tick during the time trial countdown. This fixes#699 even further
by making it so the time trial par can't even be lost during the
countdown, because the timer won't tick up - so you can never get a sad
squeak to play by pausing the game or unfocus-pausing it during the
countdown.
This adds music and volume sliders to the audio options. To use the
sliders, you navigate to the given option, then press ACTION, and your
selection will be transferred to the slider. Pressing left or right will
move the slider accordingly. Then you can press ACTION to confirm the
volume is what you want and deselect it, or you can press Esc to cancel
the volume change, and it will revert to the previous volume; both
actions will write your settings to disk.
Most of this commit is just adding infrastructure to support having
sliders in menus (without copy-pasting code), which is a totally
completely new user interface that has never been used before in this
game. If we're going to be adding something new, I want to make sure
that it at least is done the RIGHT way.
Closes#706.
This 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.
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!).
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().
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.
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.
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.
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.
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.
This does the same thing as the last commit, but for No Death Mode
instead of Time Trials. Whenever you die in No Death Mode, or complete
it, all the relevant variables get copied to variables prefixed with
'ndmresult' that never get reset by script.hardreset(), and these
variables are what titlerender() use, instead of the "live" ones.
This makes it so when a Time Trial gets completed, all the relevant
variables get copied onto variables prefixed with 'timetrialresult',
which never get reset by script.hardreset(). Then titlerender() will use
those variables accordingly.
During 2.3 development, there's been a gradual shift to using SDL stdlib
functions instead of libc functions, but there are still some libc
functions (or the same libc function but from the STL) in the code.
Well, this patch replaces all the rest of them in one fell swoop.
SDL's stdlib can replace most of these, but its SDL_min() and SDL_max()
are inadequate - they aren't really functions, they're more like macros
with a nasty penchant for double-evaluation. So I just made my own
VVV_min() and VVV_max() functions and placed them in Maths.h instead,
then replaced all the previous usages of min(), max(), std::min(),
std::max(), SDL_min(), and SDL_max() with VVV_min() and VVV_max().
Additionally, there's no SDL_isxdigit(), so I just implemented my own
VVV_isxdigit().
SDL has SDL_malloc() and SDL_free(), but they have some refcounting
built in to them, so in order to use them with LodePNG, I have to
replace the malloc() and free() that LodePNG uses. Which isn't too hard,
I did it in a new file called ThirdPartyDeps.c, and LodePNG is now
compiled with the LODEPNG_NO_COMPILE_ALLOCATORS definition.
Lastly, I also refactored the awful strcpy() and strcat() usages in
PLATFORM_migrateSaveData() to use SDL_snprintf() instead. I know save
migration is getting axed in 2.4, but it still bothers me to have
something like that in the codebase otherwise.
Without further ado, here is the full list of functions that the
codebase now uses:
- SDL_strlcpy() instead of strcpy()
- SDL_strlcat() instead of strcat()
- SDL_snprintf() instead of sprintf(), strcpy(), or strcat() (see above)
- VVV_min() instead of min(), std::min(), or SDL_min()
- VVV_max() instead of max(), std::max(), or SDL_max()
- VVV_isxdigit() instead of isxdigit()
- SDL_strcmp() instead of strcmp()
- SDL_strcasecmp() instead of strcasecmp() or Win32 strcmpi()
- SDL_strstr() instead of strstr()
- SDL_strlen() instead of strlen()
- SDL_sscanf() instead of sscanf()
- SDL_getenv() instead of getenv()
- SDL_malloc() instead of malloc() (replacing in LodePNG as well)
- SDL_free() instead of free() (replacing in LodePNG as well)
This patch cleans up unnecessary exports from header files (there were
only a few), as well as adds the static keyword to all symbols that
aren't exported and are specific to a file. This helps the linker out in
not doing any unnecessary work, speeding it up and avoiding silent
symbol conflicts (otherwise two symbols with the same name (and
type/signature in C++) would quietly resolve as okay by the linker).
Since INTERIM_COMMIT is a char array whose size we know for sure at
compile time, and which we also know is an array (instead of being a
pointer), we can take the SDL_arraysize() of it. However,
SDL_arraysize() doesn't account for the null terminator unlike
SDL_strlen(), so we'll have to do it ourselves. But at least we are
guaranteed to get a constant value at compile time, unlike if we use
SDL_strlen(), which would be repeatedly evaluating a constant value at
runtime.
To my knowledge, there's no equivalent SDL_arraysize() for constant
strings, and a quick `rg` (ripgrep) for "sizeof" in the SDL include/
folder doesn't show anything like that. So we'll just have to use the
SDL_arraysize() - 1 and deal with it.
The previous implementation of showing the commit hash on the title
screen used a preprocessor definition added at CMake time to pass the
hash and date. This was passed for every file compiled, so if the date
or hash changed, then every file would be recompiled. This is especially
annoying if you're working on the game and switching branches all the
time - the game has at least 50 source files to recompile!
To fix this, we'll switch to using a generated file, named
Version.h.out, that only gets included by the necessary files (which
there is only one of - Render.cpp). It will be autogenerated by CMake
(by using CONFIGURE_FILE(), which takes a templated file and does a
find-and-replace on it, not unlike C macros), and since there's only one
file that includes it, only one file will need to be recompiled when it
changes.
And also to prevent Version.h.out being a required file, it will only be
included if necessary (i.e. OFFICIAL_BUILD is off). Since the C
preprocessor can't ignore non-existing include files and will always
error on them, I wrapped the #include in an #ifdef VERSION_H_EXISTS, and
CMake will add the VERSION_H_OUT_EXISTS define when generating
Version.h.out. The wrapper is named Version.h, so any file
that #includes the commit hash and date should #include Version.h
instead of Version.h.out.
As an added bonus, I've also made it so CMake will print "This is
interim commit [HASH] (committed [DATE])" at configure time if the game
is going to be compiled with an interim commit hash.
Now, there is also the issue that the commit hash change will only be
noticed in the first place if CMake needs to be re-ran for anything, but
that's a less severe issue than requiring recompilation of 50(!) or so
files.
Changing settings would most of the time attempt to save unlock.vvv and
now also settings.vvv, but there would be no feedback whether the files
have been saved successfully or not. Now, if saving fails when changing
settings in the menu, a warning message will be shown. The setting will
still be applied of course, but the user will be informed it couldn't
be saved. This message can be silenced until the game is restarted,
because I can imagine this could get very annoying when someone already
knows their settings aren't writeable.
Also, some options didn't even save settings in the first place. These
are turning off invincibility, and by coincidence precisely all the
options in the advanced options menu. I made sure these options now do
so.
It wasn't a direct duplicate of key.sensitivity, but it was still
basically the same thing. Although to be fair, at least the case-switch
conversion didn't get duplicated everywhere unlike game.slowdown.
So now key.sensitivity functions the same as game.controllerSensitivity,
and it only gets converted to its actual value whenever a joystick input
happens in key.Poll(), unlike previously where it got converted every
single frame on the title screen (there was even a comment that said
"TODO bit wasteful doing this every poll").
game.gameframerate seems to exist for converting the value of
game.slowdown into an actual timestep value, when really the timestep
value should just use game.slowdown directly with a fast lookup table.
Otherwise, there's a bunch of duplicated game.slowdown case-switches
everywhere, which adds up to a large, annoying pile should the values be
changed in the future. But now the duplicate variable has been removed,
and with it, all the copy-pasted case-switches.
Also, the game speed text rendering in Menu::accessibility and
Menu::setslowdown has been factored out to a function and de-duplicated
as well.
There were some duplicate Screen configuration variables that were on
Game, when there didn't need to be.
- game.fullScreenEffect_badSignal is a duplicate of
graphics.screenbuffer->badSignalEffect
- game.fullscreen is a duplicate of !graphics.screenbuffer->isWindowed
- game.stretchMode is a duplicate of graphics.screenbuffer->stretchMode
- game.useLinearFilter is a duplicate of
graphics.screenbuffer->isFiltered
These duplicate variables have been removed now.
I put indentation when handling the ScreenSettings struct in main() so
the local doesn't live for the entirety of main() (which is the entirety
of the program).
Apparently, the amount of digits in a commit hash that git will output
varies depending on how many objects are in the repository that the hash
gets pulled from. The more objects, the more digits needed to avoid a
hash collision.
Sources:
https://stackoverflow.com/q/18134627/#comment26560283_18134919https://stackoverflow.com/a/21015031/
So that means we'll have to dynamically account for the length of the
commit hash in order to get it properly right-aligned with the rest of
the text.
In C++, when you have two variables in different scopes with the same
name, the inner scope wins. Except you have to be really careful because
sometimes they're not (#507). So it's better to just always have unique
variable names and make sure to never clash a name with a variable in an
outer scope - after all, the C++ compiler and standard might be fine
with it, but that doesn't mean humans can't make mistakes reading or
writing it.
Usually I just renamed the inner variables, but for tx/ty in editor.cpp,
I just got rid of the ridiculous overcomplicated modulo calculations and
replaced them with actual simple modulo calculations, because the
existing ones were just ridiculous. Actually, somebody ought to find
every instance of the overcomplicated modulos and replace them with the
actual good ones, because it's really stupid, quite frankly...
With the previous commit in place, we can now simply move some usages of
the previous towerbg to use a separate object instead. That way, we
don't have to mess with a monolithic state, or some better way to phrase
what I just said, and we instead have two separate objects that can
coexist side-by-side.
Previously, the tower background was controlled by a disparate set of
attributes on Graphics and mapclass, and wasn't really encapsulated. (If
that's what that word means, I don't particularly care about
object-oriented lingo.) But now, all relevant things that a tower
background has has been put into a TowerBG struct, so it will be easy to
make multiple copies without having to duplicate the code that handles
it.
This is because if they are manually written out, they are more likely
to contain mistakes.
In fact, after further review, there are several functions with
incorrect manually-written bounds checks:
* entityclass::entitycollide()
* entityclass::removeentity()
* entityclass::removeblock()
* entityclass::copylinecross()
* entityclass::revertlinecross()
All of those functions forgot to do 'greater than or equal to' instead
of 'greater than' when comparing against the size of the vector. So they
were erroneous. But they are now fixed.
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
This is a follow-up to #421.
The game would draw the activity zone if there was one active at all,
and would ignore game.act_fade. Normally this wouldn't be a problem, but
since #421, it's possible that there could be an active activity zone
but game.act_fade would still be 5. This would happen if you entered a
new activity zone while a cutscene was running.
To fix this, just make it so that the prompt gets drawn on its own and
only depends on the state of game.act_fade (or game.prev_act_fade), and
won't be unconditionally drawn if there's an active activity zone. As a
bonus, this de-duplicates the drawing of the activity prompt, so it's no
longer copy-pasted.
This moves the teleporter, activity prompt, and trophy text "don't draw"
conditionals to the part where the game checks collision with them,
instead of whenever the game draws them.
This makes it so that the game smoothly does the fade-in/fade-out
animation instead of suddenly stopping rendering them whenever their
"don't draw" conditions apply. Now, the "Press ENTER to activate
terminal" prompt will no longer suddenly disappear whenever you activate
one, and the "- Press ENTER to Teleport -" prompt will smoothly fade
back in after teleporting, instead of suddenly popping in on screen.
This fixes a possible graphical issue where the "Press ENTER to activate
terminal" prompt gets drawn on top of the "- Press ACTION to advance
text -" prompt, which looks bad. Trophy text gets drawn on top, too, so
there's a check there as well.
I've also made it so the activity prompt doesn't get drawn if the player
doesn't have control. After all, what use is it to say "press ENTER" if
the player isn't allowed to?
The game time is moved a little to the left, and the trinket count a
little to the right. To fix#376 for real, the trinket count is now
positioned automatically based on its length. The trinket icon is now
also displayed at the far right (instead of to the left of the count)
for better symmetry, and so that switching between tele save and quick
save doesn't make the trinket icon move if the trinket counts have
different lengths.
While I'm going to fix#376 anyway to make longer numbers (like Seventy
Eight) fit, I decided to also make the box a little wider in advance of
the game being localized, those extra chars could be a lifesaver, while
you wouldn't really notice the difference if you play the game in
English.
Okay, so basically here's the include layout that this game now
consistently uses:
[The "main" header file, if any (e.g. Graphics.h for Graphics.cpp)]
[blank line]
[All system includes, such as tinyxml2/physfs/utfcpp/SDL]
[blank line]
[All project includes, such as Game.h/Entity.h/etc.]
And if applicable, another blank line, and then some special-case
include screwy stuff (take a look at editor.cpp or FileSystemUtils.cpp,
for example, they have ifdefs and defines with their includes).
Including a header file inside another header file means a bunch of
files are going to be unnecessarily recompiled whenever that inner
header file is changed. So I minimized the amount of header files
included in a header file, and only included the ones that were
necessary (system includes don't count, I'm only talking about includes
from within this project). Then the includes are only in the .cpp files
themselves.
This also minimizes problems such as a NO_CUSTOM_LEVELS build failing
because some file depended on an include that got included in editor.h,
which is another benefit of removing unnecessary includes from header
files.
Graphics::drawmenu() no longer has copy-pasted code for each individual
case. Instead, the individual cases have their own adding on to common
code, which is far easier to maintain.
Also, the only difference Graphics::drawlevelmenu() does is in some
y-positioning stuff. There's no reason to make it a whole separate
function and duplicate everything AGAIN. So it's been consolidated into
Graphics::drawmenu() as well, and I've added a boolean to draw a menu
this way if it's the level menu.
So, originally, I wanted to keep them on Game, but it turns out that if
I initialize it in Game.cpp, the compiler will complain that other files
won't know what's actually inside the array. To do that, I'd have to
initialize it in Game.h. But I don't want to initialize it in Game.h
because that'd mean recompiling a lot of unnecessary files whenever
someone gets added to the credits.
So, I moved all the patrons, superpatrons, and GitHub contributors to a
new file, Credits.h, which only contains the list (and the credits max
position calculation). That way, whenever someone gets added, only the
minimal amount of files need to be recompiled.
The problem we're running into is entirely contained in the Screen - we need to
either decouple graphics context init from Screen::init or we need to take out
the screenbuffer interaction from loadstats (which I'm more in favor of since we
can just pull the config values and pass them to Screen::init later).
This change was half-backported from the localization branch, except I
just came up with "scaling mode" as a better term than the more generic
"graphics mode". It doesn't make sense to still have the option be
called "toggle letterbox" because a third option (integer mode) was
added at some point.
The options for fullscreen and scaling mode were at the top, then there
were various other graphical options, and then the option to resize to
the nearest window size that is of an integer multiple was all the way
below that. Now that last option is moved to be right below the other
options related to window sizing.
VVVVVV's menus are kind of packed to the brim, so I thought it was time
to recategorize the menus a little bit. There's now a new "advanced
options" menu which holds the following options which were moved out of
graphic options, game options and especially accessibility options:
- toggle mouse
- unfocus pause
- fake load screen
- room name background
- glitchrunner mode
I also made the positioning of the titles and descriptions more
consistent, and made some options which were moved to the new menu not
so abbreviated ("load screen" and "room name bg")
The game for some reason had this thing where it would not draw the
diagonal background tiles if you had animated backgrounds turned off.
Which is weird, because spikes with that background are still drawn as
spikes with that background. And also, it doesn't do this for any of the
tower hallway rooms, which is inconsistent.
Better to simplify the logic in Render.cpp anyways by removing
graphics.drawtower_nobackground() and making it really clear what
exactly we'll do if backgrounds are turned off. ("Aren't we already not
drawing the background? What's this _nobackground() function for?")
Flip Mode will now be in the game options menu if either:
(1) You're playing the M&P version.
(2) You have it unlocked and you came here from the in-game pause
screen.
This is because if you're playing M&P, you'd have to close the game,
edit unlock.vvv, and re-launch the game to toggle Flip Mode, since
there's no other way to do so. And if you're playing the full version,
you'd have to save and exit your session in order to toggle Flip Mode.
If you want your game window to simply be exactly 320x240, or 640x480,
or 960x720 etc. then it's really annoying that there's no easy way to do
this (to clarify, this is different from integer mode, which controls
the size of the game INSIDE the window). The easiest way would be having
to close the game, go into unlock.vvv, and edit the window size
manually. VCE has a 1x/2x/3x/4x graphics option to solve this, although
it does not account for actual monitor size (those 1x/2x/3x/4x modes are
all you get, whether or not you have a monitor too small for some of
them or too big for any of them to be what you want).
I discussed this with flibit, and he said that VCE's approach (if it
accounted for monitor size) wouldn't work on high-retina displays or
high DPIs, because getting the actual multiplier to account for those
monitors is kind of a pain. So the next best thing would be to add an
option that resizes to the nearest perfect multiple of 320x240. That way
you could simply resize the window and let the game correct any
imperfect dimensions automatically.
It's sometimes unwanted by people, and it's unwanted enough that there
exist instructions to hexedit the binary to remove it (
https://distractionware.com/forum/index.php?topic=3247.0 ).
Fun fact, the unfocus pause didn't exist in 2.0.
Centiseconds won't be saved to any save file or anything. This is just
to make speedrunning a bit more competitive, being able to know the
precise time of a time trial or full game run.
The time trial par time on the result screen always has ".99" after it.
This is basically due to the game comparing the number of seconds to the
par number of seconds using less-than-or-equal-to instead of simply
less-than.
This was fixed in 2.3 because one of the side effects of this janky
system was being able to accidentally immediately quit to the title if
the screen was black during a cutscene, which is something very likely
to happen to casual players.
Anyway, credits warp uses this gamestate-based system because it
utilizes quitting to the title screen doing gamestate 80. From there,
you increment the gamestate to gamestate 94 to use the Space Station 2
expo script.
Glitchrunner mode is intended to re-enable glitches that existed in
older versions of VVVVVV. These glitches were removed because they could
legitimately affect a casual player's experience. Glitches like various
R-pressing screwery, Space Station 1 skip, telejumping, Gravitron
out-of-bounds, etc. will not be patched.
All menus had a hardcoded X position (offset to an arbitrary starting
point of 110) and a hardcoded horizontal spacing for the "staircasing"
(mostly 30 pixels, but for some specific menus hardcoded to 15, 20 or
something else). Not all menus were centered, and seem to have been
manually made narrower (with lower horizontal spacing) whenever text
ran offscreen during development.
This system may already be hard to work with in an English-only menu
system, since you may need to adjust horizontal spacing or positioning
when adding an option. The main reason I made this change is that it's
even less optimal when menu options have to be translated, since
maximum string lengths are hard to determine, and it's easy to have
menu options running offscreen, especially when not all menus are
checked for all languages and when options could be added in the middle
of a menu after translations of that menu are already checked.
Now, menus are automatically centered based on their options, and they
are automatically made narrower if they won't fit with the default
horizontal spacing of 30 pixels (with some padding). The game.menuxoff
variable for the menu X position is now also offset to 0 instead of 110
The _default_ horizontal spacing can be changed on a per-menu basis,
and most menus (not all) which already had a narrower spacing set,
retain that as a maximum spacing, simply because they looked odd with
30 pixels of spacing (especially the main menu). They will be made even
narrower automatically if needed. In the most extreme case, the spacing
can go down to 0 and options will be displayed right below each other.
This isn't in the usual style of the game, but at least we did the best
we could to prevent options running offscreen.
The only exception to automatic menu centering and narrowing is the
list of player levels, because it's a special case and existing
behavior would be better than automatic centering there.
This basically adds an extra '|| game.inintermission' because it seems
like the original code forgot about that conditional. You can't save in
level replays, so there's no need to say "You will lose any unsaved
progress." in intermission replays.
Any decent compiler will optimize this so that it's still only two
function calls (or it gets inlined). However, it's still not very
readable, so I've assigned the result to a variable and used that
instead.
Instead of copy-pasting everything and changing a few parts, just have
something that handles that tiny part. This reduces the amount of code
size the custom level CREW page takes up by half.
This will be used to keep some text positions the same when in Flip
Mode, instead of having to copy and paste code.
This function being at the very top of the file kind of violates
locality, but it has to be done because it can't be a macro.
Previously, the code to print all tab names was copied to every single
tab, resulting in 12 more superfluous print statements than needed. This
commit uses graphics.map_tab to de-duplicate all the code.
There is now an option in "graphic options" named "toggle fps", which
toggles whether the game visually runs at 1000/34 FPS or over 1000/34
FPS. It is off by default.
I've had to put the entire game loop in yet another set of braces, I'll
indent it next commit.
By "hidden names", I'm referring to "Dimension VVVVVV" and "The Ship"
popping up on the quit/pause/teleporter screens, even though those rooms
don't have any roomnames.
Apparently my commit to fix roomname re-draw bleed on the
quit/pause/teleporter screens exposed yet another hardreset()-caused
bug. The issue here is that since hardreset() sets game.roomx and
game.roomy to 0, map.area() will no longer work properly, and since the
hidden roomname check is based on map.area(), it will no longer display
"Dimension VVVVVV" or "The Ship" once you press ACTION to quit. It used
to do this due to the re-draw bleed, but now it doesn't.
I saw that roomnames didn't get reset in hardreset(), so the solution
here is to re-factor hidden names to be an actual variable, instead of
being implicit. map.hiddenname is a variable that's set in
mapclass::loadlevel(), and if isn't empty, it will be drawn on the
quit/pause/teleporter screens. That way it will still display "Dimension
VVVVVV" and "The Ship" when you press ACTION to quit to the menu.
EDIT: Since PR #230 got merged, this commit is no longer strictly
necessary, but it's still good to refactor hidden names like this.
This is only noticeable if you have a font with translucent pixels, like
I do. But it gets really noticeable really quickly with this over-30-FPS
patch because the render functions are continuously called every
delta-timestep. To prevent this, just fill the backbuffer with black
before rendering anything.
Otherwise it'll go by too quickly.
Also something subtle here - I didn't make it conditional on
game.advancetext, so now it'll still decrement even if you have
advancetext up.
