Closes#925.
My fix here is to delay the font change until all fading-out textboxes
have disappeared. See it as adding a sort of `untilbars` or `untilfade`
for text box fadeout, into setfont.
This doesn't prevent every possible way to change the font of an
existing textbox, but you would need to use internal scripting to still
do it (and basically be doing it on purpose) - the problem in
simplified scripting when you simply do textbox-setfont-textbox is
gone.
Showing the option on the "play a level" option feels to me as though
inexperienced players would think they're not supposed to open the
player levels, because the message says editor levels are unsupported,
right? But the message is only referring to the level editor, so in my
opinion, it's clearer to only show it there.
This commit removes the `NO_EDITOR` and `NO_CUSTOM_LEVELS` defines,
which cleans up the code a lot, and they weren't really needed anyways.
This commit also disables the editor on the Steam Deck, and adds a
program argument to re-enable the editor, `-enable-editor`.
For some reason, the credits button was always specifically removed from
M&P builds. After some discussion with Terry Cavanagh on the VVVVVV
Discord server, we agreed that there was no reason this should be
removed. So, it's getting put back in.
This puts the code to fix mouse coordinates in stretch mode directly
inside KeyPoll::Poll, preventing the need for any other instances of
mouse coordinate usage to copy-paste code.
If this text on the time trial results screen would overlap with the
time value, all rank labels will be displayed on the header line
instead ("TIME TAKEN:" etc). This works because the overlap with the
time most likely only happens with CJK fonts (where the time will be
very wide because of the font size) while strings like "TIME TAKEN"
take up very little space due to only needing 4 characters or so for
the same information.
- ERROR/WARNING screen title was overlapping with message
- Crewmate screen names and rescued statuses were overlapping with each
other
- Textboxes on Level Complete screen were overlapping with each other
and the crewmate was not vertically centered in the box
- Some strings were running into each other in flip mode, instead of
being moved out of each other (PR_CJK_HIGH and PR_CJK_LOW worked the
wrong way around because of FLIP macros being applied to Y coords)
- In-game esc menu was "bouncy" with selected menu options because of a
hardcoded 16 pixel offset
- Bindings in the gamepad menu were overlapping with each other
- Some Super Gravitron "Best Time" labels and values were a little too
close
This will be needed for the Simplified Chinese translation, of which
the first version has just been delivered!
This is the first language with a font bigger than 8x8 (this is 12x12),
so it might be a little rough in some places. Most of the game is
already prepared for it, though!
The only changes I made from the previous version (which was uploaded
on Discord a few times and also sent to the translator) was the …
character - it's often used twice in a row, and it was a little uneven
(looking like - - - - - - instead of - - - - - -); and the
semicolon, which was missing some pixels.
Two changes:
- The labels on the Game Complete! screen for number of trinkets/deaths
/time etc have been moved two pixels to the right, and had their
limits increased by 1 character
- The inaccuate limit for "quit to main menu" has been increased
These are errors and issues that have been reported, but are fixable by
us without needing to involve the translator in the fix, without too
much risk of accidentally breaking grammar rules.
The full list of fixes:
- Fixed some menus going offscreen by removing unneeded words (I'm
fairly confident after some cross-referencing and research) in
Portuguese BR, Portuguese PT, Spanish, and French
- Set 0 to singular in numbers.xml in Portuguese BR and French
- Removed the ** from dimensional stability generator to make it not go
offscreen, and aligned the text better, in Portuguese BR and Spanish
- Fixed some small casing and spacing errors in Portuguese BR, French
and Welsh. Think of misplaced spaces or sentences starting with a
lowercase letter
- Fixed a limit break in Spanish (the menu button already drops the
word "de" in "retraso de la entrada", so the title can too, right?)
- Corrected some typos in French and German (je continuer->je continue,
9: Finde->9: Feinde and NENÜ->MENU)
- Fixed spacing and alignment in teletype terminals in Welsh (like the
dimensional stability generator)
923efe54d6 added a note to the PR
template, but the best place is probably in the translators readme.
Hopefully this will make sure we no longer get people eager to start
a translation because nobody else had started one yet, and then have to
be disappointed with a reply from us saying we can't accept voluntary
translation contributions.
Another new language! It's based off of the old translator pack so
it'll need to be updated later. This commit also includes some fixes
to the originally submitted version, mainly:
- "2.0" and ".99" were broken by excel(?), now fixed
- Removed traces from extraneous rows and columns
- Small human errors
This includes the update received 2023-06-09.
If you provided any one of -playx, -playy, -playrx, -playry, -playgc, or
-playmusic in command-line arguments for command-line playtesting, then
the game would always try to play music, even if you passed a negative
-playmusic. This wouldn't do anything in that case, unless you had
MMMMMM installed, in which case it would play MMMMMM track 15
(Predestined Fate Final Level) due to the legacy wraparound bug.
To fix this, only play music if the track provided is greater than -1.
Additionally, to prevent it from playing Path Complete by default if you
specify any of the other save position arguments but n ot -playmusic,
it's now initialized to -1 instead of 0.
Turns out `graphics.drawrect` exists. Well, not anymore!
This was another function from before the renderer rewrite which tried
to draw a rectangle by using four filled rectangles. We can draw
outline rectangles properly now, so let's make sure everywhere does it!
As #974 states, the lab background only ever uses the first generated
value from `backboxint`, so let's change it to a normal variable
instead of an array. Also, stars don't need their width/height set to
2 constantly... they never change in the first place, Terry!
Some code still used rectangles to draw things like lines and pixels,
so this commit adds more draw functions to support drawing lines and
pixels without directly using the renderer.
Aside from making generated minimaps draw points instead of 1x1
rectangles, this commit also batches the point drawing for an
additional speed increase.
This fixes a bug where if a track was resumed, pausing it by unfocusing
the window (if enabled, of course) would not resume it after refocusing
the window.
This happens because resuming the music doesn't change currentsong back
from -1, and the window refocusing code checks that currentsong isn't -1
before resuming music.
haltedsong is only used when resuming music. It is set back to -1 when
resuming music or when playing a new track.
Autotiling was a mess of functions and if chains and switch statements.
This commit makes autotiling better by assigning each direction to one
bit in a byte, giving each different combination its own value. This
value is then fed into a lookup table to give fine control on which
tiles get placed where.
The lab tileset can now use the single tiles which were before unused
in the autotiler, and the warp zone's background tool now places the
fill used in the main game.
- Some levels used chars < 0x20 as non-collapsible spaces, those would
now show up as [?]
- I recently found out how making characters < 0x20 6 pixels wide
doesn't work if they're missing from the font altogether
Therefore, the font now starts at 0x00 again instead of 0x20, like it
used to.
Arguably it was an advantage that the game would look extremely messed
up if you made a mistake with the fonts. In particular, a common
mistake could be to copy the new Unicode font.png, but forget to copy
the corresponding font.txt. However, 2.3 didn't come with the unicode
font, 2.4 will, so it'll be a lot less common for people to need to
manually copy the font. And if they do, it's probably for their own
level, and they have something in mind for the font, and if it doesn't
work they'll know fast enough when whatever they're planning doesn't
work (and it would only affect their own level's text, not any menus).
New language! It's still based off the old translator pack so the
newest strings haven't been translated yet, but the language files are
synced and we still need to get these updated in most other languages
either way.
This does include the update delivered on 2023-05-24.
A little while ago the term "AGOKLAVO" (action-key) was chosen to
replace the older "AGBUTONO" (action-button). However, in a recent
language update, I mistakenly used the older term in a new string.
This has been fixed.
Also note that it's written in the accusative case for this string
(with an "N" suffixed) since it is always used as the object of the
sentence where it appears ("Premu AGOKLAVON..." = "Press ACTION...").
This fixes a regression where main game teleporter icons (which would be
target icons if flag 12 was on) would be rendered on the minimap after
loading from a custom quicksave.
This is because this was always enabled when loading from a custom
quicksave, but the game didn't start rendering them until PR #898, which
de-duplicated the minimap rendering code.
The best fix here is to just not enable the teleporters when loading
from custom quicksaves.
This adds an anonymous enum for time trial indexes (e.g. the bestrank,
bestlives, etc. arrays and timetriallevel), and replaces all integer
literals with them.
Just like the unlock arrays, these are indexes to an array in XML save
files, so the numbers matter, and therefore should not use strict
typechecking.
This adds an anonymous enum for the unlock and unlocknotify arrays and
unlocknum function, and replaces all integer literals with them.
This is not named and thus cannot be used for strict typechecking
because these are actually indexes into an array in XML save files, so
the numbers themselves matter a lot.
This replaces the swngame int variable with a named enum and enforces
strict typechecking on it.
Strict typechecking is okay here as the swngame variable is not part of
the API surface of the game in any way and is completely internal.
And just to make things clear, I've added a SWN_NONE enum to use for
initialization, because previously it was being initialized to 0, even
though 0 was the Gravitron.
The clock on the Game Saved quicksave screen has always been upside-down
in Flip Mode. And technically, the trinket was too, but this was
unnoticeable because the default trinket sprite is symmetrical.
To fix this, draw flipsprites.png if these sprites are being drawn in
Flip Mode instead of sprites.png.
There's not any ill effects of it being initialized to 0 that I am aware
of (because in most cases, it either gets overwritten anyways or there
isn't a track playing in the first place), but it shouldn't be 0,
because that's Path Complete, so fixing this just in case.
This adds an anonymous enum for sound effects and replaces all calls to
music.playef that use integer literals.
This is not a named enum (that can be used for strict typechecking)
because sound effect IDs are essentially part of the API of the game -
many custom levels use these numbers. This is just to make the source
code more readable without needing a comment to denote what number is
what sound.
This adds an anonymous enum for music tracks and replaces all calls to
music.play and music.niceplay that use integer literals. Additionally,
this is also done for integer literals for cl.levmusic (except 0) and
music.currentsong where appropriate, but _not_ the music areamap because
that would not make it look very aesthetically pleasing in the code.
This is not a named enum (that can be used for strict typechecking)
because music track IDs are essentially part of the API of the game -
almost every custom level uses these numbers. This is just to make the
source code more readable without needing a comment to denote what
number is what track.
This adds a "- Press {button} to skip -" prompt to both the credits and
ending picture sequences.
It was always possible to skip them by pressing Enter, but not many
people knew this. In fact, even I didn't know this until I saw Elomavi
do it a year or so ago. So it's not really intuitive that this is
possible.
The prompt only shows up if you've completed the game before, and
disappears after two seconds similar to the "[Press {button} to return
to editor]" text.
Unfortunately, given how the game works, game completion is detected
based on if you have unlocked Flip Mode or not. At this point, the
unlock for the game being completed (unlock 5) will already be set to
true no matter what during the Plenary fanfare, but the Flip Mode unlock
(unlock 18) won't be until the player hits "play" on the main menu. As a
special case, the prompt will always show up in M&P (because Flip Mode
is always unlocked in M&P).
This prevents deleting telesaves and quicksaves in special modes and
custom levels.
Otherwise, rolling credits in a custom level would still delete the main
game quicksave.
If you had the map button held before the game transitioned to the
credits and ending picture sequences, then you wouldn't be able to skip
them, because those gamemodes don't have logic to detect when you've
released the map button.
To fix this, just implement the map button release logic.
We do need a better input system soon...
This command was changed from setactivityposition(x,y) to
setactivityposition(y), but there's a small problem here:
```diff
else if (words[0] == "setactivityposition")
{
- obj.customactivitypositionx = ss_toi(words[1]);
obj.customactivitypositiony = ss_toi(words[2]);
}
```
This meant that the function still took two arguments, the first of
which was unused and the second of which was the Y position of the
activity zone. This is now fixed.
This fixes a long-standing bug where it's possible to play music during
the Game Over screen in No Death Mode. All you have to do is die while
music is fading out from one area to the next.
The easiest way to do this is in the entrance to Space Station 2, since
there's a music change to Passion for Exploring in Outer Hull (you will
need to go into the zone far enough to activate Pushing Onwards first),
which also contains spikes to die on.
Basically, it's a simple oversight because the nicefade system relies on
music fading out to start playing the next track, but in this case, No
Death Mode fades the music out without accounting for that. It's best to
just disable nicefade entirely when dying in No Death Mode.
Thanks to KSS for reporting this bug.
This calls Game::savestatsandsettings() after unlocking Master of the
Universe (the achievement for completing No Death Mode), instead of
before.
This is not a big deal since Game::savestatsandsettings() is called
anyway whenever the game is gracefully closed since 2.3, but it helps to
make sure people won't lose their achievement data.
2.3 already made it so that if you ran the `rollcredits` command during
in-editor playtesting, you wouldn't be returned to the title screen
while losing unsaved level changes. But there are plenty of other ways
to go back to the title screen from in-editor playtesting too. Namely,
gamestate 1015 (the gamestate after completing a level) and 82 (time
trial complete).
So just add the appropriate checks to those gamestates, and add a
catch-all check in Game::quittomenu(). Additionally,
Game::updatecustomlevelstats() should not update custom level stats
during in-editor playtesting (otherwise it would still happen even if
the game didn't bring you back to the title screen).
Editor notes will also be shown if the game prevents you from going to
the title screen.
Also, just to make things clear, I also added a level note for when the
level is completed during in-editor playtesting. This is just to make it
clear in cases where it might not be obvious that the game returned you
to the editor for this reason. E.g. you have a terminal that calls
gamestate(1013) in a level with 0 custom crewmates, but when you
activate it, it looks like the terminal didn't work for some reason and
just brought you back to the editor. But that's just only because you
literally just completed the level.
This fixes a bug where the wrong music can play on the title screen, as
reported by AllyTally on Discord.
The bug can be triggered by triggering a room transition right as
game.quittomenu() is called (which is easiest to achieve by placing the
player on an oscillating/"out of bounds" room border in a custom level
so they go back and forth between two rooms every frame, and triggering
gamestate 1013, which starts a fadeout to menu if all custom crewmates
are rescued).
When this happens, game.quittomenu() calls script.hardreset(), but the
rest of the frame still executes, even though we set game.gamestate to
TITLEMODE too (because game.quittomenu() was called by
game.updatestate() which was called by gamelogic(), and game.gamestate
is only checked at the start of the frame). This ends up triggering a
room transition, and since map.custommode is guaranteed to now be off
(because of script.hardreset()), the main game music area code kicks in,
and plays something that isn't Presenting VVVVVV.
The bug here is that we're resetting too early when we still have the
rest of an in-game frame to execute. So, instead, we should only reset
at the end of the frame, and this can be achieved with a defer callback.
This will actually do several things:
(1) Make the tile size checks apply to the appropriate graphics files
once again.
(2) Make the game print a fallback error message if the error message
hasn't been set on the levelDirError error screen.
(3) Use levelDirError for graphics errors too.
(4) Make the error message for tile size checks failing specify both
width and height, not just a square dimension.
(5) Make the error messages mentioned above translatable.
It turns out that (1) didn't happen after #923 was merged, since #923
removed needing to process a tilesheet into a vector of surfaces for all
graphics files except sprites.png and flipsprites.png. Thus, the game
ended up only checking the correct tile sizes for those files only.
In the process of fixing this, I also got rid of the PROCESS_TILESHEET
macros and turned them into two different functions: One to make the
array, and one to check the tile size of the tilesheet.
I also did (2) just in case FILESYSTEM_levelDirHasError() returns false
even though we know we have an error.
And (3) is needed so things are unified and we have one user-facing
error message system when users load levels. To facilitate this, I
removed the title string, since it's really not needed.
Unfortunately, (1) doesn't apply to font.png again, but that's because
of the new font stuff and I'm not sure what Dav999 has in store for
error checking. But that's also why I did (4), because it looks like
tile sizes in font.png files can be different (i.e. non-square).
game.quittomenu() correctly resets state, as it's the function that's
always used when quitting to menu. This fixes a bug where if a level
with assets failed to load, it wouldn't unload the assets.
This exports the previously-internal setLevelDirError function in
FileSystemUtils and uses it for if a level is not found or there was a
parsing error. Previously, if a level failed to load in these ways, it
would take you to the error screen with no error, while printing it to
the console. But this makes it more user-friendly.
As a bonus, the text is localizable, just like the existing usage of
FILESYSTEM_setLevelDirError for if a path couldn't be mounted.
After the scriptclass::startgamemode refactor, a lot of common code is
still being executed even if the level loading failed. This sets the
game-gamestate to TITLEMODE in gotoerrorloadinglevel(), and also returns
early just in case.
Fixes#975.
This is quite a simple bug: If you edit a script, then close it, you
will no longer be able to use the mute buttons (N and M).
The problem here is that in 2.3, key.disabletextentry() was called when
closing a script. However, #944 removed the call. Therefore, a
regression.
If you used command-line playtesting to load a level in a zip, the game
would print a warning saying the level wasn't found. This is because the
warning is printed when it tries to load a level before it loads zips,
inside the metadata load function itself.
To fix this, just move the responsibility for printing the error outside
the function, and put it on the caller.
This prints all binary blob check fails. It's an error if the game
rejects the header and will refuse to load it at all, and a warning if
the game continues on.
This also removes the EOF check (`offset + header->size > size`) as a
fatal error. It will only print a warning now. If the last header goes
past the end of file, it will be handled gracefully by PhysFS, which is
the same case in VVVVVV 2.2. This actually fixes a regression from 2.3
where certain custom level tracks that were working perfectly fine in
2.2 (e.g. Summer Spooktacular's track 15) refused to play since 2.3.
This checks the return value of PHYSFS_readBytes() in all cases, and
uses a wrapper to not duplicate common logic. If the read fails, then it
will vlog an error, else if the amount of bytes read was less than
expected, it will vlog a warning.
Additionally, the space allocated for a file in
FILESYSTEM_loadFileToMemory is SDL_calloc()ed instead of SDL_malloc()ed
so if there are less bytes than expected, the memory will at least be
zeroed. This also means we don't have to do the null termination,
because the last byte will already be zeroed.
The return value of PHYSFS_readBytes() when reading the headers of
binary blobs now assigns to `header->size`, so the call has been placed
after the increment to `offset` because `offset` needs the correct (i.e.
intended) size of the header.
In the past I was thinking we could use some kind of feature in VVVVVV
to track outdated strings across language files. But as it turns out, a
manual solution is actually *perfect* (in combination with automatic
syncing): duplicating strings and marking the old one as outdated. We
started doing it in recent PRs, so let's make it official by adding it
to the readme.
In Italian, "Credits" is "Riconoscimenti", which runs offscreen with
the 3x font size that this title uses in the rolling credits at the end
of the game. I'm not sure if the translators saw that specific
instance, or thought the limit complaint was about the main menu button
all along (which is more prominent and *does* stick out far enough that
the complaint could plausibly have been about that, from a translator's
perspective!)
Either way, it's solved now: this string's width is now checked, and if
it will run offscreen at 3x size, it will now be displayed at 2x size
instead. The limit has been increased from 13 to 20 in the language
files accordingly.
This already happened on 2023-03-16, but I held off on updating the
repo's version a bit long: I wanted to wait a little to batch it up
with the next update, but the next update only arrived today, so...
I noticed that in 2.3, the game would launch with default controller
binds upon first launch (e.g. no pre-existing unlock.vvv or
settings.vvv), but in 2.4, this wasn't the case, and the default binds
would only be set the next time the game was launched. This would result
in you being essentially unable to use the controller on first launch
save for the analogue stick and D-pad to move between menu selections
and move the player.
Bisecting pointed to commit 3ef5248db9 as
the cause of the regression. It turns out returning early upon error or
a file not existing didn't set the default controller binds, because
they were done at the end of Game::deserializesettings(). But the binds
would be set on the next launch because if the file didn't exist, a new
file would be written, not with the default binds, but then the next
launch would read the file, see there were no binds, and then set the
default binds accordingly.
To fix this, I made it so that the default controller binds are set when
Game is initialized. That way, it covers all cases where the game can't
read a settings file.
Ever since 2.3, if you bind a controller button to the "menu" action
(i.e. back/escape) you won't be able to change that button to any other
action, because pressing it anywhere in the binding menu will exit to
the previous menu, without applying the binding.
I know action sets will overhaul everything, but 2.4 may (probably
"should") come out before we have action sets. This part is very
broken, and the fix is very easy: just move the bind-assigning code to
above the menu-return-on-esc code, and add a return.
For some reason, the accessibility option that was meant to disable
flashes doesn't disable ALL flashes, only screen flashes and screen
shaking. This commit disables a lot more, most importantly randomness
in colors, the player flashing on death/respawn, and teleporters
flashing.
This updates the interpolation positions of the player when transforming
into and out of VVVVVV-Man.
Otherwise, it can be seen that the player "zips" quickly during these
transformations if the Secret Lab entrance cutscene is played with
screen effects off.
Unfortunately, 1de459d9b4 caused a
regression where musicclass::niceplay() didn't work, because fading out
the music would cause haltdasmusik() to be called, which would reset the
fade variables.
To fix this, haltdasmusik() will now only reset the fade variables if
it's not called from a fade, which is signaled with a function
parameter.
`platv` is a room property that controls platform speed, and it has
always worked (other than some weird storage issues due to a bug).
However, the editor has no way to edit it currently, so people had to
resort to editing the level file by hand, or with a third-party tool.
This commit simply adds an easy way to modify platform speed.
VED has a fill bucket subtool for tiles and backgrounds, which is
really useful when creating rooms. This commit adds a fill bucket as
well, with an adaptive tool highlight, unlike VED.
This fixes a regression from 2.3 where the very beginning of A New
Dimension isn't silent.
A New Dimension's level music is set to Predestined Fate, but there is a
script box the player touches right upon spawning that stops playing
music. Then after the player ascends upwards, they touch a script box
that plays Predestined Fate. But in 2.2 and before, the very beginning
is silent due to the script box that stops music.
However in 2.3, due to the changes made to playing music during a fade,
the initial level music trying to play Predestined Fate during a music
fadeout from the main menu resulted in Predestined Fate being stored in
the nice fade variables, which kicked in after halting the music since
halting didn't reset those variables.
This resets those variables whenever music is halted, and now the
beginning of A New Dimension is back to how it was in 2.2 and before.
This fixes a regression where the red channel 0 glitch didn't work,
because the surface was always whitened, because LoadSprites would
whiten the image before converting it to surface.
This regression happened because of #923.
Fixes#962.
Misa asked me if this should only work for non-transparent textboxes,
and it shouldn't - that was kind of an oversight.
To make it work for transparent textboxes as well, I made a little
restructuring to avoid duplicating the code - fill_buttons() is now
called textbox_line(), and it replaces the direct accessing of the
textbox lines in the printing loops. The code that checks the width
of the textbox does not need to be copied, since the text box is
naturally not drawn for transparent text boxes.
This makes it so that `CustomEntity`s, at least internally, do not use
global tile position. Instead, they will use room-x and room-y
coordinates, which will be separate from their x- and y- positions.
This makes it much easier to deal with `CustomEntity`s, because you
don't have to divide and modulo everywhere to use them.
Since editorclass::add_entity and editorclass::get_entity_at expect
global tile position in their arguments, I've added room-x and room-y
arguments to these functions too.
Of course, due to compatibility reasons, the XML files will still have
to use global tile position. For the same reason, warp token
destinations are still using global tile position too.
As reported by Lilithtreasure on the VVVVVV Discord server, it is
possible to get gray moving platforms and enemies in the main game.
This happens if you play the main game after loading a custom level with
a room that is gray at the same coordinates. E.g. if you play a custom
level with a gray room at (12, 4), then exit and go to Gantry and Dolly
in the main game which is also at (12, 4), then the platforms there
would be gray too.
This is because there is a missing map.custommode check.
The missing piece from sound effects was handling what to do when the
buffer ran out. Which seems to happen often when decoding from OGG,
unlike WAV. This handling involves callbacks to functions named
refillReserve and swapBuffers.
Without this code,some sound effects would be cut off early, as
documented in #953. This might also explain the division by 20 - which
I've copied too, just in case.
Now OGG sound effect tracks should be identical to music tracks (except
I've stripped the looping code out).
Fixes#953.
We were using this function to check if the format of the existing voice
is different from the format of the voice we intended to play. However,
whereas the format we use is the FAudioWaveFormatEx struct, the only
details we get from FAudioVoice_GetVoiceDetails is the
FAudioVoiceDetails struct, which has much less details and is missing
important things like whether the format is 8-bit or 16-bit or something
else.
And of course, if we don't check that the number of bits matches, then
it still leads to playback issues as described in #953.
There are no other functions in FAudio that operate on
FAudioWaveFormatEx structs. So instead, we'll just have to do it
ourselves, and store the format of the existing voice alongside the
format of the intended voice. And then use SDL_memcmp to make sure the
formats are the same before playing, and if not, then destroy and
re-create the voice.
Fixes#953.
There's a few places where textboxes are constructed through code, but
they pass in the color's RGB values in manually. This commit
unhardcodes most of them them, replacing them with a color lookup.
The ones that weren't changed are special cases, like `175, 174, 174`.
I thought 2.2 already had separate map and interact gamepad bindings,
and they simply got neglected and broken with 2.3's split Enter/E key
option. But actually, the new split Enter/E option also applied to
gamepad buttons, and a separate interact binding was added, without
really indicating anything if Enter and E are not split. And I guess
using the same button for map and interact by default also makes sense
for simplicity...
This commit makes sure the button glyph displayed in-game is at least
the correct button. The gamepad bindings menu is also slightly modified
to darken the interact option - the button glyphs code now
automatically causes them to show equal buttons anyway, so it wasn't
too big of a change to also darken the line and disable the binding
option. To me this says: "the interact key is fixed to be the same as
enter right now, but there is a way to change it."
It's still not ideal of course, and I know a similar change to the
gamepad menu to hide the interact option was rejected a year ago
because action sets would already fix it, but it's a year later now,
and showing misleading button glyphs should be fixed in 2.4, whether it
will already have action sets or not (And at this point I think the
plan already is to keep the existing input system for 2.4)
And it's a 3 line diff to darken and disable the option, compared to
fully hiding the option.
The language screen has a "Press Space, Z, or V to select" hint, which
I forgot to update for supporting button glyphs in #943, so this commit
does.
<action_hint>Press Space, Z, or V to select</action_hint>
<gamepad_hint>Press {button} to select</gamepad_hint>
This was easier than I expected - just add an optional buttons="1"
attribute to cutscenes.xml. It's treated like the speaker attribute -
it's only there as context for the translator, and for the cutscene
test.
Due to rebasing messiness and diff noise, it's probably best if pull
requests either don't sync all the language files at all (and only
modify the English ones) OR only do it as a final commit. It's still
something we need to figure out, lol.
If a controller button is pressed, a controller is connected (even at
startup!) or an axis is moved, the game will switch to displaying
controller glyphs. If a keyboard key is pressed or the last controller
is removed, the game will switch to displaying keyboard keys.
This adds mappings from SDL's Xbox-based SDL_GameControllerButton
constants, to glyphs for the following layouts:
- LAYOUT_NINTENDO_SWITCH_PRO,
- LAYOUT_NINTENDO_SWITCH_JOYCON_L,
- LAYOUT_NINTENDO_SWITCH_JOYCON_R,
- LAYOUT_DECK,
- LAYOUT_PLAYSTATION,
- LAYOUT_XBOX,
- LAYOUT_GENERIC,
There may still be errors in these, but they should be mostly correct.
I'm leaving it up to Ethan to make it show the correct button glyphs
for the correct controllers being connected (and possibly to fix these
mappings where needed).
Violet's dialogue now looks like this:
squeak(purple)
text(purple,0,0,2)
Remember that you can press {b_map}
to check where you are on the map!
position(purple,above)
textbuttons()
speak_active
The new textbuttons() command sets the next textbox to replace {b_map}
with the map button, and {b_int} with the interact button. The
remaining keys would be added as soon as they need to be added to
ActionSets.h as well.
This adds a function that converts an action (such as interacting
in-game) to the corresponding button text ("ENTER", "E") or button
glyph (PlayStation triangle, Steam Deck Y, etc). This function
currently only gives the existing ENTERs or Es, because I don't know
how best to detect controller usage, or whether the game is running on
a Steam Deck, or what buttons need to be displayed there. Still, it
should now be really easy to adapt the rendering of keyboard keys to
consoles, controllers, or rebound keys.
To identify the actions that currently need to be displayed, this
commit also adds the initial enums for action sets as described by
Ethan in a comment in #834 (Jan 18, 2022).
These visualize the horizontal gravity line kludge for rooms beside
eachother. When you enter another room, gravity lines which look like
they're connected between the rooms try to have the same activated
state.
Basically, if you're in room (1,4) and you go into (2,4), if a
gravity line in (1,4) is activated (gray, on cooldown) and it's
touching the gravity line in (2,4), that gravity line will also be
activated.
This uses DDA (https://w.wiki/6RSQ) to draw a line between the previous
frame's mouse position, and the current frame's mouse position. This
means that there will no longer be gaps in lines of tiles if you move
your mouse fast enough (which is actually rather slow, so it gets
annoying quickly).
The editor's timestep is no longer hardcoded to 24, as I assume that
was only done so there would be less gaps in lines of tiles drawn.
With interpolation, that is no longer an issue, so I've removed the
editor's special case for the timestep.
Scripts used a weird "hook" system, where script names were extracted
into their own list. This was completely unneeded, so it has been
replaced with using the script.customscripts vector directly.
The script editor has been cleaned up, so the cursor's Y position is
relative to the entire script, rather than what's just displaying on
the screen currently. This simplifies a lot of code, and I don't know
why it was done the other way in the first place.
The script selector and script editor cursors have been sped up, since
both lists can be massive, and waiting 6 frames per line is extremely
slow and boring. This is still slow and boring, but we don't have
proper input repetition yet.
This commit moves everything left out of the previous commit to the
state system. This means a bunch of new functions were added as well,
to avoid the code in each function becoming too huge. A lot of cleanup
was done as well, simplifying logic, merging duplicated code, etc.
This commit does NOT touch "script hooks", script editor logic and
autotiling, as those seem to be their own separate beasts.
While warp lines were being drawn, they also got resized to
automatically fit between collision. In renderfixed, gravity lines are
resized the same way. Doing logic while drawing is very poor practice,
so resizing of these has been moved into logic, and merged together.
Aside from some more cleanup, this commit also removes the very poorly
done right click emulation, when you hold CTRL and click. It never
worked well in the past, and even requires a right click to use, so
there's not really any point to keeping it around.
The drawer could definitely be improved further, however I cleaned up a
little bit of the code duplication. I'll have to take a closer look
some other time, but I'm pretty sure that the duplicated code at the
bottom can be removed with a few tweaks, but I'll do that carefully
in a different commit.
Tools were a mess, spread all over the code with hundreds of `else-if`
statements. Instead of magic numbers denoting tools, an enum has been
created, and logic has been extracted into simple switch/cases, shared
logic being deduplicated.
The base of a state system for the editor has been created as well,
laying a good path for further organization improvements. Because of
this, the entire editor no longer gets drawn underneath the menus,
except for a few pieces which I haven't extracted yet. Either way,
this should be good for performance, if that was a concern.
I have this annoying issue where the game will open on the wrong monitor
in fullscreen mode, because that monitor is considered to be display 0,
whereas the primary monitor I want is display 1.
To mitigate this somewhat, the game now stores the display index that it
was closed on, and will save it to settings. Then the next time the game
opens, it will open on that display index. This should work pretty well
as long as you aren't changing your monitor setup constantly.
Of course, none of this applies if your window manager is busted. For
example, on GNOME Wayland, which is what I use, in windowed mode the
game will always open on the monitor the cursor is on, and it won't even
be centered in the monitor. But it works fine if I use XWayland via
SDL_VIDEODRIVER=x11.
Previously, the game would not store the size of the window itself, and
would always call SDL_GetRendererOutputSize() (via
Screen::GetWindowSize()) to figure out the size of the window. The only
problem is, this would return the size of the whole monitor if the game
was in fullscreen mode. And the only place where the original windowed
mode size was stored would be in SDL itself, but that wouldn't persist
after the game was closed.
So, if you exited the game while in fullscreen mode, then your window
size would get set to the size of your monitor (1920 by 1080 in my
case). Then when you opened the game and toggled fullscreen off, it
would go back to the default window size, which is 640 by 480.
This is made worse, however, if you were in forced fullscreen mode when
you previously exited the game in windowed mode. In that case, the game
saves the size of 1920 by 1080, but doesn't save that you were in
fullscreen mode, so opening the game not in forced fullscreen mode would
result in you having a 1920 by 1080 window, but in windowed mode.
Meaning that not even fullscreening and unfullscreening would put the
game window back to normal size.
The solution, of course, is to just store the window size ourselves,
like any other screen setting, and only use GetWindowSize() if needed.
And just to make things clear, I've also renamed the GetWindowSize()
function to GetScreenSize(), because if it was named "window" it could
lead one to think that it would always return the size of the screen in
windowed mode, when in fact it returns the size of the screen whatever
mode it is in - fullscreen size if in fullscreen mode and window size if
in windowed mode.
And doing this also fixes the FIXME above Screen::isForcedFullscreen().
This fixes the following bug that only occurs on Wayland: If the game is
configured to be fullscreened and in stretch mode, on startup, it won't
be in stretch mode. It will appear to be in letterbox mode, but the game
still thinks it's in stretch mode.
This is because during the ResizeScreen() call on startup, for whatever
reason, the window size will be reported to be the default size (640 by
480) instead of the screen resolution of the monitor, as one would
expect from being in fullscreen. It seems like when the game queries the
window size, the window isn't actually in fullscreen at that time, even
though this is after fullscreen has been set to true.
To fix this, I decided to always update the logical size before
SDL_RenderPresent() is called. To make this neater, I put the scaling
code in its own function named UpdateScaling().
This bug has existed since 2.3 and does not occur on X11. I tested this
on GNOME Wayland, and for testing it on X11, I used Openbox in a Xephyr
session while running VVVVVV with SDL_VIDEODRIVER=x11.
It turns out this texture is only used as a temporary texture to draw
the screen with an offset before rendering it to the output target.
I thought it was used for drawing the map menu animation, but that was
only true of `tempBuffer`, and is no longer true of the new render
system.
When `blackout` is active, the screen (to simplify) stops getting drawn
to. The excecption is textboxes, which draw anyway. But since the
screen isn't being cleared, removed textboxes stay on screen, since
that texture isn't being cleared. In the SDL_Renderer PR, I
accidentally broke this behavior, so this commit fixes it.
Fixes#951.
Dav999 pointed out this potential issue on Discord.
While basically all memcmp implementations will terminate early here if
there's a null byte (because it's mismatched), it doesn't hurt to add
the check here.
This fixes a bug where the trinket collection text boxes, along with the
cutscene bars, would stay on-screen if the player warped to the ship
while they were up.
This only happens during the gamestate 0 anti-softlock checks, and only
if completestop is active in the first place, so text boxes aren't
cleared if the player is doing something that wouldn't lead to a
softlock otherise.
Fixes#921.
This fixes a regression where the behavior of duplicate player entities
is different, causing a gameplay section in Vespera Scientifica to be
impossible, as described in #903.
In the level, you are allowed to flip in mid-air. Vespera accomplishes
this by having two duplicate player entities stuck in a platform. One of
them is responsible for letting the player flip in one direction, and
one of them is responsible for letting them flip in the other.
In 2.3, this works because in order for a player entity to flip,
`game.jumppressed` is checked, and the entity will flip if
`game.jumppressed` is greater than 0, then `game.jumppressed` will be
set to 0. In this way, whenever a player entity flips, it will set
`game.jumppressed` to 0, so whenever the next player entity is
evaluated, `game.jumppressed` is 0 and thus _that_ entity will not flip.
This is because the for-loop surrounds both the `game.jumppressed` check
and flipping the entity. In 2.4 after #609 and subsequent patches,
however, this is not the case. Here, the for-loop only surrounds
flipping the entity. Therefore, the `game.jumppressed` check is
evaluated only once. So, it will end up flipping every player entity if
the entities are eligible. In this case, one of them is eligible twice,
resulting in the game flipping gravitycontrol four times, which is
essentially the same as not flipping at all (except for all the sound
effects).
Hence, the fix here is to make it so the for-loop surrounds the
`game.jumppressed` check.
Now, this doesn't mean that the intent of #609 - that duplicate player
entities have the same initial velocity applied to them when flipping -
has to be removed. We can just put the for-loops back in. But I
carefully implemented them in such a way that the overall function is
not quadratic, i.e. O(n²). Instead, there's a pass done over the
`obj.entities` vector beforehand to store all indexes of a player entity
in a temporary vector, and then that vector is used to update all the
player entities. In this manner, the function is still linear - O(n) -
over the number of entities in the room.
I tested this to make sure that no previous regressions popped up again,
including #839, #855, and #887. And #484 is still resolved.
Fixes#903.
This adds support for OGG files as sound effects (via renaming them to
the wrong .wav file extension), because in previous versions of the
game, SDL_mixer didn't care what the file extension was, and so some
people relied on this, as described in #900.
This is accomplished by copy-pasting the OGG loading code for music
tracks. For a bit of cleanliness, I put the WAV and OGG loading code in
separate functions.
This is mostly the same code, except that because sound effects don't
loop and can't be paused or resumed, there's no reserve buffer, and
there's no data for loop points.
Also, for some reason, the music loading code divided by 20 in the
`size` calculation. I found that this prematurely cut off sound effects,
and that it made more sense to just not do the division. I don't know
why it was there, but removing it works.
Also also, some OGG files don't work with this. Namely, ones produced by
FFmpeg. To test this, I just extracted 0levelcomplete.ogg from
vvvvvvmusic.vvv and replaced terminal.wav with it. And it works, so
hopefully I won't have to touch audio code again, although I might if
someone complains about this. But either way, I'm committing this
because it's better than it was before.
Fixes#900.
I noticed that this call wasn't using VVV_freefunc. I missed it earlier
when going through Music.cpp and checking for instances when
VVV_freefunc should have been used
(a926ce9851).
Sound effects already get recreated if the number of channels
mismatches, but the same could be true if the sample rate mismatches
too, which was the case with music tracks as described in #886.
So, just to be sure - and to be consistent with music tracks - sound
effects now check that the sample rate matches, too, and if not, will be
recreated.
This fixes an issue where sound effects of bit depths that weren't 16,
such as 8, were being played incorrectly, as described in #886.
The problem is that when loading the sound effect, we would always set
the bit depth to 16 no matter what! Instead, we should set the bit depth
to the actual bit depth of the file.
Fixes#886.
As described in #886, if a track was played when an existing track was
already playing, and the sample rates of the two tracks differ, then the
second track would play wrong and distorted.
This is because the second track would play with the sample rate of the
first. To fix this, halt the track if the sample rate is mismatched,
which destroys the voice. This results in the voice being recreated
later in the Play() function. The track is also halted if the number of
channels or bit depth is mismatched.
Fixes#886.
The style we have here is that functions with no arguments are to have
explicit `void` arguments, even though this doesn't apply in C++,
because it does apply in C.
Unfortunately, some have slipped through the cracks. This commit fixes
them.
This removes the `addnull` argument from `FILESYSTEM_loadFileToMemory`
and `FILESYSTEM_loadAssetToMemory`, and makes it so a null terminator is
always appended no matter what.
This simplifies things and removes the need for callers to make the
decision about null termination and what its implications are. Then you
get cases where null termination might not happen when it should be,
such as the one df577c59ef (#947) fixed.
When FIQ added the `addnull` argument in
5862af4445 (#117), I'm guessing he did it
because he wanted to be cautious about adding the null terminator to
every file, so he only did it for XML files, which was the only case
needed at the time. But really, there's no downsides to always appending
a null terminator. In fact, it's already always done whenever the STDIN
buffer is loaded.
Because of how `blackout` works, screen shaking must clear the gameplay
buffer. `blackout` simply pauses rendering, so if the gameplay buffer
gets cleared, then the screen will just be black, otherwise it'll look
like the game is "frozen". VVVVVV only uses `blackout` during screen
shaking, so it works as intended. However, when reimplementing this
behavior in the move to using the SDL_Renderer system, I failed to
notice that since my implementation always clears the gameplay buffer
when shaking, if you open the menu during a shake, instead of seeing
gameplay during the transition animation, you only see black. This has
been fixed with a simple `game.blackout` check before clearing the
gameplay buffer.
For some reason, originally, this function mutated the std::string
passed into it by reference. So calling the function could potentially
mutate whatever got passed in, and callers potentially could have relied
on that behavior.
Now that the surrounding callsites have all been cleaned up, though
(especially scriptclass::startgamemode), it's clear that it's only used
in two places: Loading the level in the editor, and loading the level in
live gameplay. In both cases, the passed-in string isn't used ever again
afterwards.
So, it's safe to delete the mutable reference without any undesirable
effects, making the code cleaner and easier to understand. The function
now mutates a copy instead of mutating whatever the caller has.
An example is Maximally Misleading Miserable Misadventure, which has a
font.txt which includes all ASCII characters starting with a 0x00 byte.
This would accidentally null-terminate the string too early.
Instead, we now use the total length of the file again, and keep
getting the next UTF-8 codepoint until the file ends. We still need to
null-terminate it - it protects against incomplete sequences getting
the UTF-8 decoder to read out of bounds.
This was an oversight when we migrated to the new UTF-8 system - it
expects a null-terminated string, but the utfcpp implementation worked
with a pointer to the end of the file instead.
I also added an assert in FILESYSTEM_loadFileToMemory() so this is less
likely to happen again - because there should be no valid reason to
have a NULL pointer for the total file size, as well as not wanting a
null terminator to be added at the end of the file.
Courtesy of Reese. Mainly the accented characters are updated -
uppercase letters are now mostly a pixel higher to make them higher
than lowercase letters (and only a single pixel lower than uppercase
letters without accents, instead of two pixels lower). Accents for
lowercase letters have also been made thicker overall and changed in
appearance.
Also: this font image is converted to indexed grayscale instead of full
RGBA, which makes the file only 4.5 KB instead of about 10 KB.
This updates all language files to the latest version.
- Some minor errors are also fixed - for example, a small number of
changes were made to the English string instead of the translation.
Alignment of the dimensional stability generator terminal is also
improved in several languages.
- I also discovered that the string "Complete the game" appears twice -
and has, to be consistent with adjacent strings, two separate
translations in Portuguese (PT). So this string now properly has two
different cases so it can be translated separately.
- The limit for TIME/SHINY/LIVES has been bumped from 7 to 8
The following languages are new:
- French
- German
- Italian
- Portuguese (BR)
- Portuguese (PT)
- Russian
- Spanish
- Turkish
Esperanto has also received some updates.
`hardestroom` currently stores the current roomname, but it was missing
a call to get the translated string. This has been added, fixing the
hardest room appearing as untranslated.
See the previous two commits, a lot of the time we don't need
std::string objects to be passed to these functions because we already
have C strings.
Commit 1/3: font::print_wrap
Commit 2/3: font::print
-> Commit 3/3: font::len
Turns out I was overplaying my hand a little when changing font::print
from std::string to const char*, so instead, I'll overload the
function: it can take either a const char* (the main function) or a
std::string (a wrapper). This means any C string that's printed
everywhere else (which is common, especially because loc::gettext gives
them) no longer needs to be converted to a std::string object each call.
Commit 1/3: font::print_wrap
-> Commit 2/3: font::print
Commit 3/3: font::len
We no longer need to pass a std::string object to the print and len
functions - in fact, we often only have a C string that we want to
print or get the visual width of (that C string most often comes from
loc::gettext), and it's a bit wasteful to wrap it in a new std::string
object on every print/len call.
This does mean adding a few more .c_str()s, but there's not many places
where a std::string is being passed to these functions, and we already
use .c_str() sometimes.
-> Commit 1/3: font::print_wrap
Commit 2/3: font::print
Commit 3/3: font::len
This removes memory churn caused by using analogue mode.
The surfaces are only allocated if analogue mode is turned on, and kept
after they are initialized. Otherwise, if analogue mode is never turned
on (which will be the case for the vast majority of the time the game is
played), then no extra memory is used.
Drawing a texture onto itself seems to produce issues on Metal.
To fix this, use a temporary texture instead, that then gets drawn onto
the original texture.
Fixes#927.
These are from a fan translation that was originally made in 2020.
The files were kept around as a possible base for the future Spanish
translators, and now that a first version of the new Spanish
translation is being tested, it's time to remove this one. These files
are making it very annoying for me to test all the new translations and
then jump around between different commits (and they're already not in
the round 2 translator pack).
Actually, this won't really help with the jumping around different
commits part... But the sooner it's removed, the less confusing it will
be when different versions of the language pack are floating around and
the latest version needs to be added, and the less "Changes not staged
for commit" problems you'll get when testing the new language packs.
(Or two different españols being on the language screen)
This is because destroying the renderer causes use-after-frees since the
renderer destroys all textures when it gets destroyed.
This fixes a Valgrind error where an invalid read occurs because the
font textures get destroyed again after the renderer is destroyed.
The hashmap would get populated with the name of each font, as each
font was being added. Unfortunately, adding a font would also realloc
the storage for fonts, in which the names are also stored... Possibly
invalidating the pointers to the names. This is now fixed by populating
the hashmap after all the fonts are added.
For consistency, since they are created in create_buffers as well. I
checked with Valgrind (which is very noisy on Wayland, it turns out),
but I didn't see anything about them not being freed. It doesn't hurt to
use VVV_freefunc here anyway, though, since it does a NULL check and
nulls the pointer afterwards, which should prevent double-freeing and
use-after-frees.
I'm going to soon be creating an actually temporary texture, so having
two textures named "temp" would get confusing. This is also a good
chance to correct the name of this texture, because it's not really
temporary, but it's used for map menu animation rendering.
This commit replaces the old system with the new one, making it much
easier to edit the transforming and glitchy roomnames. Additionally,
this syncs flag 72 to finalstretch.
Co-authored-by: Misa Elizabeth Kai <infoteddy@infoteddy.info>
This commit adds a better system for animated roomnames.
The old system, like many other systems, were very hardcoded, and can be
described as mostly else-if chains, with some fun string comparisons.
The new system uses lists of text for transformations and glitchy names,
making it much easier to add new cases if needeed.
This commit implements the system but does not replace the old system,
where that is done in the next commit.
The settings for special roomnames can be read from level XML, and
`setroomname()` can be used from commands to set a new, static name.
I'm also planning to change the argument types of font::len,
font::print and font::print_wrap from const std::string&s to
const char*s, but I'll do that separately.
This is a small library I wrote to handle UTF-8.
Usage is meant to be as simple as possible - see for example decoding
a UTF-8 string:
const char* str = "asdf";
uint32_t codepoint;
while ((codepoint = UTF8_next(&str)))
{
// you have a codepoint congrats
}
Or encoding a single codepoint to add it to a string:
std::string result;
result.append(UTF8_encode(0x1234).bytes);
There are some other functions (UTF8_total_codepoints() to get the
total number of codepoints in a string, UTF8_backspace() to get the
length of a string after backspacing one character, and
UTF8_peek_next() as a slightly less fancy version of UTF8_next()), but
more functions could always be added if we need them.
This will allow us to replace utfcpp (utf8::unchecked) and also fix
some less-than-ideal code:
- Some places have to resort to ignoring UTF-8 (next_wrap) or using
UCS-4→UTF-8 functions (VFormat had to use PHYSFS ones, and one other
place has four lines of code including a std::back_inserter just for
one character)
- The iterator stuff is kinda confusing and verbose anyway
Originally the changedir command was used here, making
Vitellary look left and then immediately snap back to
looking right. Now the changeai command is used instead
to make him actually look left, and then look back to
the right on his last textbox.
The `-addresses` command-line option added in 64be99d4 helps
autosplitters on platforms where VVVVVV is not built as a
position-independent executable. macOS has made it increasingly
difficult, or impossible, to build binaries without PIE.
Adding an obvious string to search for will help tools that need to deal
with versions of VVVVVV built with PIE. The bytestring to search for is
`[vVvVvV]game`, followed by four null bytes (to avoid finding it in the
program code section). This identifies the beginning of the game object;
addresses to other objects can be figured out by relative offsets
printed by `-addresses`, since ASLR can only change where the globals
begin.
Partially fixes#928; it may still be advisable to figure out how to
explicitly disable PIE on Windows/Linux.
Activity zone prompts have always been limited to a single line,
because the text box had a hardcoded size. A translator requested for
the possibility to add a subtitle under music names for the jukebox,
and the easiest solution is to make it possible to translate a prompt
with multiple lines. This is possible now, and the textbox even
wordwraps automatically.
(I wouldn't really like to see translations using multiple lines for
stuff like "Press ENTER to talk to Vitellary", especially if it wraps
with one name and not with another, but if a string is too long,
wordwrapping will look better than text running out of the box.)
There were two print calls, one for the transparent case, and one for
a regular textbox. The print calls were nearly the same except for the
color, and for some reason the transparent case didn't have PR_CJK_LOW
(that one is on me).
There is no overlap in side effects between this line and the switch
statement after it, but it did result in adding the width of a final
null terminator or newline to the width of the current line, which is
a waste because those widths both 1) require trying to find
non-existent characters in the font and 2) will not be used.
I found this out because I added a debug print in find_glyphinfo(), and
something was requesting lots of codepoint 0 from the font.
In a button glyph font (like buttons_8x8.fontmeta) you can now specify
<type>buttons</type> to indicate that it's a button glyphs font. In a
normal font, you can specify <fallback>buttons_8x8</fallback>. This
will make it such that if a character is not found in the main font,
it will instead be looked for in buttons_8x8. If not found there
either, the main font's U+FFFD or '?' will be used as before.
This makes find_font_by_name() not O(n). It's not really a big deal,
because there won't be many fonts, but it'd make a function in the next
commit (finding the given fallback font for each font by name) O(n^2).
It's easy enough to add the hashmap.
They are not used yet in this commit - this just adds the graphics and
data for the glyphs. It also adds a <fallback> tag to font.fontmeta to
use buttons_8x8.
The icons themselves are made by Reese Rivers - see #859.
This makes them stand out more.
The border around the tool has also been moved to be drawn first.
Otherwise, it would be drawn on top of the outline of the text, which
would look bad.
This prints the address of every global class to the console, and then
exits.
This is useful for autosplitters, which read memory addresses directly.
Any time a new version of the game is shipped, this makes updating the
autosplitters much easier as people don't have to find the addresses of
the global classes themselves.
After discussing with Ally and Dav, we came to the agreement that this
is basically useless since the prompt will always be centered and take
up most of the horizontal space of the screen.
And the x-position was only added as an offset because at some point,
there was a missing space from the side of the "- Press ENTER to
Teleport -" prompt, and the offset was there so people could mimic the
prompt accordingly. But that was fixed at some point, so it's useless
now.
Even though it would be a bizarre combination, declaring no character
set (neither via <chars> nor via font.txt) meant that <special>
couldn't be used because the ASCII fallback charset would be loaded
after special ranges were processed. Now, all the methods of loading
the charset are attempted sequentially, and only afterwards, the
special ranges are loaded.
There used to be two ways of fading in/out text in VVVVVV:
- Local code that modifies the R, G and B values of the text
- Keeping the RGB values the same and using the alpha channel
The latter approach is only used once, for [Press ENTER to return to
editor]. The former approach causes problems with colored (button)
glyphs: there's no way for the print function to tell from the RGB
values whether a color is "full Viridian-cyan" or "Viridian-cyan faded
out 50%", so I added the flag PR_COLORGLYPH_BRI(value) to tell the
print function that the color brightness is reduced to match the
brightness of colored glyphs to the brightness of the rest of the text.
However, there were already plans to make the single use of alpha
consistent with the rest of the game and the style, so PR_ALPHA(value)
could be removed, as well as the bit signifying whether the brightness
or alpha value is used. For the editor text, I simply copied the "Press
{button} to teleport" behavior of hiding the text completely if it
becomes darker than 100/255.
Another simplification is to make the print function handle not just
the brightness of the color glyphs while local code handled the
brightness of the normal text color, but to make the print function
handle both. That way, the callsite can simply pass in the full colors
and the brightness flag, and the flag name can be made a lot simpler as
well: PR_BRIGHTNESS(value).
"by {author}" is a string that will cause a lot of localization-related
problems, which then become much worse when different languages and
levels can also need different fonts:
- If the author name is set to something in English instead of a name,
then it'll come out a bit weird if your VVVVVV is set to a different
language: "de various people", "por various people", etc. It's the
same problem with Discord bots completing "playing" or "watching" in
their statuses.
- Translators can't always fit "by" in two letters, and level creators
have understandably always assumed, and will continue to assume, that
"by" is two letters. So if you have your VVVVVV set to a language that
translates "by" as something long, then:
| by Various People and Others |
...may suddenly show up as something like:
|thorer Various People and Othe|
- "by" and author may need mutually incompatible fonts. For example, a
Japanese level in a Korean VVVVVV needs to be displayed with "by" in
Korean characters and the author name with Japanese characters, which
would need some very special code since languages may want to add
text both before and after the name.
- It's very possible that some languages can't translate "by" without
knowing the gender of the name, and I know some languages even
inflect names in really interesting ways (adding and even replacing
letters in first names, surnames, and anything in between, depending
on gender and what else is in the sentence).
So to solve all of this, the "by" is now replaced by a 10x10 face from
sprites.png, like a :viridian: emote. See it as a kind of avatar next
to a username, to clarify and assert that this line is for the author's
name. It should be a fairly obvious/recognizable icon, it fixes all the
above problems, and it's a bonus that we now have more happy faces in
VVVVVV.
If your language has a bigger font, the max attribute isn't really
helpful to you as a translator, so the sync feature adds a special
max_local attribute which is accurate to the font size. This was
already documented in advance.
If used, we now also write an attribute in the root tag of strings.xml
and strings_plural.xml, that looks like max_local_for="12x12". I
decided to add this attribute after finding out the Excel macros would
be really hard to change to only show a max_local column if it is ever
used (it'd need to look ahead through the strings until it finds a
string with a max, or remove the column if no string has used it), but
it's also a convenience for translators.
If, somehow, a single character is wider than the limit, next_wrap
would get you stuck in an infinite loop by refusing to update the start
index and giving a line length of 0. Now, it just gives you a line with
that single character.
I also made some small readability improvements: I renamed next_wrap_s
to next_wrap_buf, and added comments at the top of both functions
explaining what they do.
By default, when you open the level editor to start a new level, the
level font will now match your VVVVVV language; so if you're, say,
Japanese, then you can make Japanese levels from the get-go. If you
want to make levels for a different target audience, you can change the
font via a new menu (map settings > change description > change font).
The game will remember this choice and it will become the new initial
level font.
If a custom level doesn't specify a font, it should be the 8x8 font.
But the main game can't specify a font, it's just the interface font
because that's for the language that the game is in.
They need to know how wide the text is going to be in a particular
font, so font::string_wordwrap and font::string_wordwrap_balanced now
take a flags argument like all the printing and dimensions-getting
functions. next_wrap and next_wrap_s take a Font* now, they're internal
to Font.cpp so they can take a Font and avoid double flag-parsing. But
if any non-Font.cpp code needs next_wrap/next_wrap_s in the future, I'd
just make a public wrapper that takes a uint32_t flags and passes the
Font* to the internal functions.
This is one of the few places still using 2-space indentation instead
of 4 spaces, and it makes it very annoying to work with when your tab
key inserts four spaces - so I could either just mimic it for the time
being, or I could just fix it while I was at it.
- If the level font is higher than 10 pixels, the third description
line (Desc3) is disabled and unavailable. CJK languages require less
characters to convey the same message (140 characters caused people
to cram tweets in all languages except CJK) and this gives us enough
room in the levels list without having to cram the metadata even more
than it already was or showing less levels per page.
- The "Untitled Level" and "by Unknown" now selectively show up in the
interface font instead of the level font.
None of the structs in the new font system ended up being "publicly"
accessible, they were all treated as implementation details for
Font.cpp to use, so these structs are now fully defined in Font.cpp
only.
The last two deprecated functions are:
- Graphics::Print
- Graphics::PrintWrap
These are used a lot, but they're relatively easy to replace, since the
only flag I probably have to immediately worry about is PR_CEN. I do
often need to add PR_FONT_* flags but I don't need to add any
PR_2X/PR_3X/PR_4X anymore.
Only three deprecated functions remain:
- Graphics::Print
- Graphics::PrintWrap
- Graphics::bigprint
I also fixed multiline transparent textboxes having their outlines
overlap the text itself, and fixed textboxclass::padtowidth assuming
glyph widths of 8 (it made the hints at the start of intermission 1
run offscreen for example)
Only four deprecated functions remain:
- Graphics::Print
- Graphics::PrintWrap
- Graphics::bigprint
- Graphics::bprint
Graphics::bprint is the least-used one of them, and after that, the
other functions are used a LOT, but it'll be a lot faster to go through
them, since I have less and less flags to worry about. I'll probably
start using Vim macros again like I did for loc::gettext()ing strings,
or maybe I'll automate this completely.
I migrated all of them to font::print, so they can now be removed.
Six deprecated print functions left! (Of which some are used a whole
lot, it's simpler if the lesser-used ones are gone first.)
This used some constants counting numbers of characters
(LEN_INTERIM_COMMIT and LEN_BRANCH_NAME) and even an SDL_arraysize,
all multiplied by 8, to get the length of the displayed text.
Now it just uses the new PR_RIGHT flag of font::print.
I did also force the 8x8 font for all the interim information, since
the date kinda overlapped with the menu options... And all of these
lines only show up in interim versions anyway, except for the version
number - which is left in the interface font for consistency with the
rest of the menu in non-interim versions. The inconsistency in interim
versions doesn't really matter all that much I think, it's just some
technical/debug info.
I migrated all graphics.len calls over to font::len (and also migrated
prints mainly surrounding those graphics.len's) so the old len function
is now completely removed.
I especially focused on graphics.len and the print calls around them,
because graphics.len calls appear a bit less often, might be overlooked
when migrating print calls (thus possibly using different fonts by
accident) and are often used for some kind of right-alignment or
centering which can be changed into PR_RIGHT or PR_CEN with a different
X anyway.
Notably, I also added a new function to generate these kinds of
sliders: ....[]............
Different languages means that the slider for analogue stick
sensitivity needs to be longer to fit possibly long words for
Low/Medium/High, and then different font sizes means that the longer
slider won't fit onscreen in a language that needs a 12-wide font. So
slider_get() can take a "target width", which dynamically changes the
number of characters depending on the width of them in the interface
font.
I kinda forgot that I could force the 8x8 font instead of adapting the
characters in the slider to the font, and other ideas (like using
different characters or a more graphical progress bar) have been
brought up on Discord, so this might all change again sooner or later.
If you for example have your VVVVVV set to English, have the option for
Chinese selected, and then the window loses focus, the English pause
screen would show up in the Chinese font. This is now fixed.
meta.xml can now have a <font> tag, which gives the name of the font
that the language needs. This will directly control the interface font
when the language is active, and will soon also control the font used
for each option on the language screen.
Also added some borders to more of the text in room name translator
mode, fixed a positioning issue if the interface font is not 8x8, and
migrated the trophy texts to font::print_wrap (including
PR_COLORGLYPH_BRI that still needed to be done)
Activity zones need to be in the interface font if the message is from
the system (like Press ENTER to activate terminal, which may be in a
different language) and in the level font if it's a customized message
(setactivitytext).
Graphics::drawtextbox was counting the textbox width and height in
8x8 characters, even including the borders as characters, so it'd need
to be told what the font for the textbox is, and then probably only the
height needs to be adapted to the font and not the width because that's
adapted to the screen width... So just call Graphics::drawpixeltextbox
directly instead. It was already weird enough how actual cutscene
textboxes called Graphics::drawtextbox with x/8, y/8 before the
previous commit, (when you already have the pixel width and height!)
only to have that be a wrapper for drawpixeltextbox by doing x*8, y*8.
Some textboxes need to be in the level font (like room names, cutscene
dialogue, etc - even in the main game), and some need to be in the
interface font (like when you collect a shiny trinket or crewmate). So
most of these textboxes now have graphics.textboxprintflags(font_flag)
as appropriate.
RoomnameTranslator.cpp is now also migrated to the new print system -
in room name translator mode, the room name is now displayed in the 8x8
font if it's untranslated and the level font if it is.
Level text such as room names, text box content, and the contents of
the script editor need to be displayed in the level-specific font, and
tweaked to look right. This involves displaying less lines in the
script editor, making text boxes bigger, displaying some text higher
and some text lower. This is still unfinished, but it's the real start
of a migration to font::print functions!
find_font_by_name() just finds the index of a given font name. This
index is supposed to be stored and reused, because the font (for a
language/level) won't be changed very often. So this function would
only run when getting the language metadata, when loading a level, etc.
All global fonts and all custom fonts in a level are now loaded, and
added to their respective "vectors". The selected font is still always
as the global font.png, and the custom level font also isn't selected
yet, but it's now easier to implement that.
Also, I added FILESYSTEM_enumerateAssets, which #902 already has but I
needed it now. I also rewrote it to not use std::vector<std::string>.
That was my idea, it's also how FILESYSTEM_getLanguageCodes worked,
so for symmetry, that function is getting changed as well.
The font::len function now handles the printing scale, so it can
immediately return the scaled length instead of having the caller
calculate it. The print function now handles CJK low/high flags and
vertically centers CJK text by default (instead of letting it stick
out on the bottom).
The following functions were moved directly:
- next_wrap
- next_wrap_s
- string_wordwrap
- string_wordwrap_balanced
- string_unwordwrap
These ones will probably still need get a flags argument, except for
string_unwordwrap (since they need to know what font we're talking
about.
The implementation of graphics.len has also been moved to Font.cpp,
but graphics.len still exists for now and is deprecated.
graphics.PrintWrap is now also deprecated. An advantage of the new
version (with flags) is that it'll be possible to do things like put
a border around wrapped text, wrap text at larger scales, etc, but
these things don't work perfectly yet.
This commit also has some other fixes, like the default advance of
6 pixels for characters 0x00-0x1F in 8x8 fonts.
There has always been a mess of different print functions that all had
slightly different specifics and called each other:
Print(x, y, text, r, g, b, cen)
nothing special here, just does what the arguments say
PrintAlpha(x, y, text, r, g, b, a, cen)
just Print but with an alpha argument
PrintWrap(x, y, text, r, g, b, cen, linespacing, maxwidth)
added for wordwrapping, heavily used now
bprint(x, y, text, r, g, b, cen)
prints an outline, then just PrintAlpha
bprintalpha(x, y, text, r, g, b, a, cen)
just bprint but with an alpha argument
bigprint(x, y, text, r, g, b, cen, sc)
nothing special here, just does what the arguments say
bigbprint(x, y, text, r, g, b, cen, sc)
prints an outline, then just bigprint
bigrprint(x, y, text, r, g, b, cen, sc)
right-aligns text, unless cen is given in which case it just
centers text like other functions already do?
bigbrprint(x, y, text, r, g, b, cen, sc)
prints an outline, then just bigrprint
We need even more specifics with the new font system: we need to be
able to specify whether CJK characters should be vertically centered or
stick out on the top/bottom, and we sometimes need to pass in
brightness variables for colored glyphs. And text printing functions
now fit better in Font.cpp anyway. So there's now a big overhaul of
print functions: all these functions will be replaced by font::print
and font::print_wrap (the former of which now exists). These take flags
as their first argument, which can be 0 for a basic left-aligned print,
PR_CEN for centered text (set X to -1!!!) PR_BOR for a border (instead
of functions like bprint and bigbprint), PR_2X, PR_3X etc for scaling,
and these can be combined with |.
Some text, for example [Press ESC to return to editor], fades in/out
using the alpha value, which is passed to the print function. In some
other places (like Press ENTER to teleport, textboxes, trophy text...)
text can fade in or out by direct changes to the RGB values. This means
regular color-adjusted white text can change color, but colored button
glyphs can't, since there's no way to know in the print system what the
maximum RGB values of a specific textbox are supposed to be, so the
only thing it can do is draw the button glyphs at full brightness,
which looks bad. Therefore, you can now also pass in the brightness
value via the flags, with PR_COLORGLYPH_BRI(255).
Since there's now a new XML-based font metadata file format to obsolete
the .txt file with all the glyphs, this commit switches the built-in
font to that new format.
This is still a work in progress, but the existing font system has been
removed and replaced by a new one, in Font.cpp.
Design goals of the new font system include supporting colored button
glyphs, different fonts for different languages, and larger fonts than
8x8 for Chinese, Japanese and Korean, while being able to support their
30000+ characters without hiccups, slowdowns or high memory usage. And
to have more flexibility with fonts in general. Plus, Graphics.cpp was
long enough as-is, so it's good to have a dedicated file for font
storage.
The old font system worked with a std::vector<SDL_Surface*> to store
8x8 surfaces for each character, and a std::map<int,int> to store
mappings between codepoints and vector indexes.
The new system has a per-font collection of pages for every block of
0x1000 (4096) codepoints, that may be allocated as needed. A glyph on
a page contains the index of the glyph in the image (giving its
coordinates), the advance (how much the cursor should advance, so the
width of that glyph) and some flags which would be at least whether the
glyph exists and whether it is colored.
Most of the *new* features aren't implemented yet; it's currently
hardcoded to the regular 8x8 font.png, but it should be functionally
equivalent to the previous behavior. The only thing that doesn't really
work yet is level-specific font.png, but that'll be supported again
soon enough.
This commit also adds fontmeta (xml) support.
Since the fonts folder is mounted at graphics/, there are two main
options for recognizing non-font.png fonts: the font files have to be
prefixed with font (or font_) or some special file extension is
involved to signal what files are fonts. I always had a font.xml in
mind (so font_cn.xml, font_ja.xml, etc) but if there's ever gonna be
a need for further xml files inside the graphics folder, we have a
problem. So I named them .fontmeta instead.
A .fontmeta file looks somewhat like this:
<?xml version="1.0" encoding="UTF-8"?>
<font_metadata>
<width>12</width>
<height>12</height>
<white_teeth>1</white_teeth>
<chars>
<range start="0x20" end="0x7E"/>
<range start="0x80" end="0x80"/>
<range start="0xA0" end="0xDF"/>
<range start="0x250" end="0x2A8"/>
<range start="0x2AD" end="0x2AD"/>
<range start="0x2C7" end="0x2C7"/>
<range start="0x2C9" end="0x2CB"/>
...
</chars>
<special>
<range start="0x00" end="0x1F" advance="6"/>
<range start="0x61" end="0x66" color="1"/>
<range start="0x63" end="0x63" color="0"/>
</special>
</font_metadata>
The <chars> tag can be used to specify characters instead of in a .txt.
The original idea was to just always use the existing .txt system for
specifying the font charset, and only use the XML for the other stuff
that the .txt doesn't cover. However, it's probably better to keep it
simple if possible - having to only have a .png and a .fontmeta seems
simpler than having the data spread out over three files. And a major
advantage: Chinese fonts can have about 30000 characters! It's more
efficient to be able to have a tag saying "now there's 20902 characters
starting at U+4E00" than to include them all in a text file and having
to UTF-8 decode every single one of them.
If a font.txt exists, it takes priority over the <chars> tag, and in
that case, there's no reason to include the <chars> tag in the XML.
But font.txt has to be in the same directory as font.png, otherwise it
is rejected. Same for font.fontmeta. If neither font.txt nor <chars>
exist, then the font is seen as a 2.2-and-below-style ASCII font.
In <special>: advance is the number of pixels the cursor advances after
drawing the character (so the width of the character, without affecting
the grid in the source image), color is whether the character should
have its original colors retained when printed (for button glyphs).
As for <white_teeth>:
The renderer PR has replaced draw-time whitening of sprites/etc
(using BlitSurfaceColoured) by load-time whitening of entire images
(using LoadImage with TEX_WHITE as an argument).
This means we have a problem: fonts have always had their glyphs
whitened at printing time, and since I'm adding support for colored
button glyphs, I changed it so glyphs would sometimes not be whitened.
But if we can't whiten at print time, then we'd need to whiten at load
time, and if we whiten the entire font, any colored glyphs will get
destroyed too. If you whiten the image selectively, well, we need more
code to target specific squares in the image, and it's kind of a waste
when you need to whiten 30000 12x12 Chinese characters when you're only
going to need a handful, but you don't know which ones.
The solution: Whitening fonts is useless if all the non-colored glyphs
are already white, so we don't need to do it anyway! However, any
existing fonts that have non-white glyphs (and I know of at least one
level like that) will still need to be whitened. So there is now a
font property <white_teeth> that can be specified in the fontmeta,
which indicates that the font is already pre-whitened. If not
specified, traditional whitening behavior will be used, and the font
cannot use colored glyphs.
The MAKEANDPLAY, NO_CUSTOM_LEVELS, and NO_EDITOR defines remove content
or features. However, they then raise several warnings because of some
cases, functions, or variables that end up not being used.
This silences them by using the UNUSED macro, or by adding a default
catch-all case if the define is defined (so unhandled cases will still
raise warnings in a build that doesn't have these defines).
We get these warnings because of the typedefs for 64-bit integers in
PhysFS's header files. The compiler will treat them as extensions and
will still compile it fine but it does mean we aren't strictly standards
conforming. Which really isn't a problem anyway. Probably.
This adds the build type in brackets in `-version` output after e.g.
"VVVVVV v2.4". The build type is MAKEANDPLAY, NO_CUSTOM_LEVELS, or
NO_EDITOR (which are not necessarily mutually exclusive).
This is appended on to the end of the first line so as to not break
Ved's existing `-version` check which only checks if the beginning of
STDOUT is "VVVVVV" followed by any version number.
This makes it so that whenever the game loads a script as directed by a
script command, it will first try to load the script from the processed
argument, and if that fails only then will it try to load the script
from the raw argument.
This fixes a regression reported by Dav999 in the custom level "Vungeon"
created by Dynaboom, where a script `ifflag`s to `aselectP1.1` even
though the actual script name is `aselectp1.1`. In 2.3, it would
lowercase `aselectP1.1` and load the script properly, but previous to
this commit it would try to load the script with a capital name and then
fail.
This aborts and prints the error from SDL_GetError() if
SDL_CreateWindow() or SDL_CreateRenderer() fails.
We abort because there's not much point in continuing if the window or
renderer can't be created. There might be a use case for running the
game in headless mode, but let's code that in explicitly if we ever want
it.
This sets the minimum window size (to 320 x 240), so that the window
cannot be resized to lower than that.
This is because there's no utility in having a game window smaller than
that, and it provides a bonus convenience of being able to resize the
game to exactly 320x240 without needing to be exactly precise about it.
This idea was suggested by Dav999.
The `point` struct was a relic of ActionScript and was added because of
the Flash 'point' object. However, it seems like Simon Roth didn't
realize that SDL has its own point struct.
With this, `Maths.h` can be un-included from a couple headers, which
exposes the fact that `preloader.cpp` was relying on `Maths.h` being
transitively included from `Graphics.h`.
Dav999 added `#include "Localization.h"` before `#include "KeyPoll.h"`,
when it should go after instead, because the letter L comes after the
letter K in the English alphabet.
Ever since VVVVVV was initially ported to C++ in 2.0, it has used surfaces from SDL. The downside is, that's all software rendering. This commit moves most things off of surfaces, and all into GPU, by using textures and SDL_Renderer.
Pixel-perfect collision has been kept by keeping a copy of sprites as surfaces. There's plans for pixel-perfect collision to use masks instead of reading pixel data directly, but that's out of scope for this commit.
- `graphics.reloadresources()` is now called later in `main`, because textures cannot be created without a renderer.
- This commit also removes a bunch of surface functions which are no longer needed.
- This also recaches target textures in certain places for d3d9.
- graphics.images was converted to a fixed-size array.
- fillbox and fillboxabs use SDL_RenderDrawRect instead of drawing an outline using four filled rectangles
- Update my name in the credits
The pixel border around the map fits to map size normally. However, when
the map is off, it's always the dimensions of the full size map, and the
border didn't reflect that, so if the custom minimap was off, and the
map wasn't the full size, it wouldn't fit correctly.
This bug was introduced in PR #898.
The branch name will be added to the window title if it is an interim
version, e.g. "VVVVVV [master]".
This makes it easier for developers to tell at a glance which build of
the game they're running.
While the window is initialized with 640x480, the screen settings
defaulted to 320x240, which is a tiny window. The screen settings take
priority over the initialized window, so people with no previous
settings file will get 320x240. This makes it so they get 640x480
instead.
The window is still initialized to 640x480 (constants used for clarity)
just in case there's some weirdness if it's initialized to a potentially
odd resolution from the user's settings file.
This is useful for developers who may have multiple builds of the game
from various different branches and may easily forget which build of the
game is what.
This shows up in the bottom-right corner of the title screen and also
with the `-version` command-line option, and in the status message
printed when building the game.
Unfortunately there needs to be an intermediate surface for proper alpha
color blending to happen via SDL_BlitSurface. The exact SDL blending
logic seems complicated and unclear for me to implement at the moment,
and my attempts kind of failed, so this is just a stopgap measure to at
least get the game rendering how it was before I screwed it up.
This refactors them to not allocate a temporary surface by instead
simply drawing directly to the destination surface.
This means re-implementing the original semantics of SDL_BlitSurface in
them, which is the function signature they (and BlitSurfaceStandard)
were based off of. So now if src_rect or dest_rect are NULL, it
automatically uses a rect of the entirety of the corresponding surface,
and other things like that. And also some other optimizations like
no-opping if the alpha is 0 (because then nothing will be drawn), and
critical checks (not drawing if the destination pixel is out of bounds,
because then otherwise it would wrap around, or at least that's what it
did when I tested it).
This resulted in a bunch of code that would really suck to copy-paste
because then you'd have to remember to update the other copy, so I
refactored them further and put the common code into one function, while
separating the different code (the exact transformation they do) into
different functions that get passed in through function pointers.
In general, "temp" is a bad name because it could mean anything. In this
case the buffer isn't really temporary and it's only used for drawing
the menu with a certain offset, so I made it use a better name. But also
because I'm going to be adding temporary buffers so I don't want the
names to be confused.
Honestly not too sure why we ever wrote the mask handling logic
ourselves instead of using SDL functions. Hell, we even used SDL_MapRGB
for Graphics::getRGB before.
`ct` was used to be a variable that a color was temporarily stored in
before being passed to a draw function. But this is unnecessary and you
might as well just have a temporary of the color directly. I guess this
was the practice used because temporaries were apparently really bad in
Flash.
setcolreal() was added in 2.3 to do basically the same thing (set it
directly from entities' realcol attributes). But it's no longer needed.
Correspondingly, Graphics::setcol has been renamed to Graphics::getcol
and now returns an SDL_Color, and Graphics::huetilesetcol has been
renamed to Graphics::huetilegetcol for the same reason.
Some functions (notably Graphics::drawimagecol and
Graphics::drawhuetile) were relying on the `ct` to be implicitly set and
weren't ever having it passed in directly. They have been corrected
accordingly.
colourTransform is a struct with only one member, a Uint32. The issue
with `Uint32`s is that it requires a bunch of bit shifting logic to edit
the colors. The issue with bit shifting logic is that people have a
tendency to hardcode the shift amounts instead of using the shift amount
variables of the SDL_PixelFormat, which makes it annoying to change the
color masks of surfaces.
This commit fixes both issues by unhardcoding the bit shift amounts in
DrawPixel and ReadPixel, and by axing the `Uint32`s in favor of using
SDL_Color.
According to the SDL_PixelFormat documentation (
https://wiki.libsdl.org/SDL2/SDL_PixelFormat ), the logic to read and
draw to pixels from colors below 32-bit was just wrong. Specifically,
for 8-bit, there's a color palette used instead of some intrinsic color
information stored in the pixel itself. But we shouldn't need that logic
anyways because we don't use colors below 32-bit. So I axed that too.
This makes it so temporary variables have their scopes reduced (if
possible). I also didn't hesitate to fix style issues, such as their
names ("temp" is such a bad name), making them const if possible, and
any code it touched too.
It previously duplicated the for-loop twice, once for tiles.png and
tiles2.png, which just made me sad. Now it doesn't do that.
Also it previously had an alternate tileset == 10 condition for
tiles.png, which didn't seem to do anything because there's no such
thing as tileset 10, and anyways it's useless in-game because when
playing in the actual game it won't draw tiles.png, so I removed it. I
don't know why it was there in the first place.
Since I removed the temp variable from the outer scope, the other usage
of it has to be updated.
For some reason they all have their executable bits set. Presumably this
is because they were made on an NTFS system where every file is
executable (which doesn't sound secure at all but that's another story).
This allows translators to test all text boxes in the scripts. It
doesn't run the scripts themselves - it only shows the basic appearance
of each text box individually, so context may be lost but it's good to
have a way to see any text boxes that might otherwise not be easily
seen because they require specific circumstances to appear.
Did another complete proofread of all the non-roomnames (hadn't looked
through *everything* in a while), and it's just three little things
that I felt would be important enough to tweak.
The strings "Vitellary"/"Vermilion"/"Verdigris"/"Victoria" now have two
cases to support changing them for the intermission replay menu options
(like "with Vitellary").
Also, the string "< and > keys change tool" is now "{button1} and
{button2} keys change tool", so it can be changed dynamically without
having to retranslate the string.
I wanted to not complicate the system with different string cases (like
cgettext) if possible, and I have been able to keep the main strings a
simple English=Translation mapping thus far, but apparently strings
like "Rescued!" (which are one string in English), have to be
translated for the correct gender in some languages. So this was a good
time to add support for string cases anyway.
It's a number that can be given to a string to specify the specific
case it's used, to disambiguate identical English keys. In the case of
"Rescued!" and "Missing...", male versions of the string are case 1,
female versions are case 2, and Viridian being missing is case 3. Of
course, if a language doesn't need to use different variants, it can
simply fill in the same string for the different cases.
If any other string needs to switch to different cases: distinguish
them in the English strings.xml with the case="N" attribute (N=1 and
higher), sync language files from the translator menu (existing
translations for the uncased string will simply be copied to all cases)
and change loc::gettext("...") to loc::gettext_case("...", 1),
loc::gettext_case("...", 2), etc.
With a single README.txt for both translators and maintainers, both
have to read through info that's not relevant for them, because
translators don't need to worry about the specifics of adding new
English strings and recompiling the game, and programmers don't need to
worry about the specifics of how to translate things. Now it's split
into README-translators.txt and README-programmers.txt.
I would, of course, recommend translators to translate the roomnames
while playing the full game (optionally in invincibility) so they can
immediately get all the context and maybe the most inspiration. And if
you want to go back into a specific level, then there's always the time
trials and intermission replays which will give you full coverage of
all the room names.
However, the time trials weren't really made for room name translation.
They have some annoying features like the instant restart when you
press ENTER at the wrong time, they remove context clues like
teleporters and companions, but the worst problem is that the last room
in a level is often completely untranslatable inside the time trials
because you immediately get sent to the results screen...
So, I added a new menu in the translator options, "explore game", which
gives you access to all the time trials and the two intermissions, from
the same menu. All these time trials (which they're still based off of,
under the hood) are stripped of the annoying features that come with
time trials. These are the changes I made to time trial behavior in
translator exploring mode:
- No 3-2-1-Go! countdown
- No on-screen time/death/shiny/par
- ENTER doesn't restart, and the map menu works. The entire map is also
revealed.
- Prize for the Reckless is in its normal form
- The teleporters in Entanglement Generator, Wheeler's Wormhole and
Level Complete are restored as context for room names (actually, we
should probably restore them in time trials anyway? Their "press to
teleport" prompt is already blocked out in time trials and they do
nothing other than being a checkpoint. I guess the reason they were
removed was to stop people from opening the teleporter menu when that
was not specifically blocked out in time trials yet.)
- The companions are there at the end of levels, and behave like in no
death mode (become happy and follow you to the teleporter). Also for
context.
- At the end of each level, you're not suddenly sent to the menu, but
you can use the teleporter at your leisure just like in the
intermission replays. In the Final Level, you do get sent to the menu
automatically, but after a longer delay.
I made another mark on VVVVVV: don't be startled, I added gamestates.
I wanted all teleporters at the end of levels to behave like the ones
at the end of the intermission replays, and all handling for
teleporting with specific companions is already done in gamestates, so
rather than adding conditional blocks across 5 or so different
gamestates, it made more sense to make a single gamestate for
"teleporting in translator exploring mode" (3090). I also added an
alternative to having to use gamestate 3500 or 82 for the end of the
final level: 3091-3092.
One other thing I want to add to the "explore game" menu: a per-level
count of how many room names are left to translate. That shouldn't be
too difficult, and I'm planning that for the next commit.
This is a single block of code so it was easy to split from the
foundation commit.
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
This involves loc::gettext_roomname and loc::gettext_roomname_special.
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
This mainly adds loc::gettext calls and replaces SDL_snprintf by
VFormat for the percentage.
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
This replaces SDL_snprintf by VFormat for the time strings, and makes
number_words get translated numbers.
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
This mainly adds loc::gettext calls for all the menu titles and
explanations. It also redesigns the time trial screen.
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
This mainly adds loc::gettext calls for menu option labels.
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
setLevelDirError was changed from snprintf-style to VFormat, but it's
only used in that file so...
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
This adds loc::gettext for all "Press {button} to explode" and friends,
and also changes the interact_prompt function in Render.cpp to expect
{button} instead of %s.
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
The affected functions are:
- editormenuactionpress
- editorinput
- editorclass::switch_tileset
- editorclass::switch_tilecol
- editorclass::switch_enemy
- editorclass::switch_warpdir
This mainly adds loc::gettext calls.
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
This commit adds most of the code changes necessary for making the game
translatable, but does not yet "unhardcode" nearly all of the strings
(except in a few cases where it was hard to separate added
loc::gettexts from foundational code changes, or all the localization-
related menus which were also added by this commit.)
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
This is needed for the limits check in the translator menu.
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
This just adds booleans roomname_special to the level classes in
preparation for the localization system to use them.
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
A relevant paragraph copied from the original commit history:
The idea is that we store all strings somewhere managed, and then the
hashmap only needs pointers to those strings. For storing strings, I
created a `Textbook` structure, which consists of one or more 50 KB
"pages" (allocated as needed) on which you can simply write strings in
both languages back-to-back with `textbook_store(textbook, text)` and
get pointers to each of them. (I was originally going to just use one
big buffer and realloc to double the size when filled up, but then the
hashmap would be full of dangling pointers...) When needed, like when
switching to a different language, an entire textbook can be freed at
once.
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
This has a lot of characters that different languages need.
This commit is part of rewritten history of the localization branch.
The original (unsquashed) commit history can be found here:
https://github.com/Dav999-v/VVVVVV/tree/localization-orig
I think this is because if you both check that __has_builtin is defined
and use it in the same 'if' preprocessor statement, it can error because
there's no equivalent to short-circuiting in preprocessor statements.
_SDL_HAS_BUILTIN should be safer.
This creates the game over screen for dying in No Death Mode. It's three
lines long and it's only called once. There's no reason it has to be a
separate function. From the name it sounds like it was meant to be a
generic function but it's anything but that. So just inline it in to
where it's called.
This fixes a bug where players could flip in mid-air at the start of
custom levels whose start points were in mid-air, because
onground/onroof were not being reset. This could also be done when
loading in to a save with a checkpoint in mid-air. All that's required
is to exit the game while standing on a surface (or otherwise having a
nonzero onground/onroof).
This reminded me that there were other variables on the player entity
persisting through that made loading in to a level not have a totally
clean slate, such as walkingframe (which could affect sequencing
individual TASes together into one TAS), so it's better to just destroy
the player entity and recreate it.
Except some attributes still have to be persisted for 2.2 and 2.0
glitchrunner mode. But it's better that this is done via a whitelist
rather than a blacklist.
The player duplicate removal code in hardreset is mostly redundant now
(except for some very unlikely corner cases), but there's nothing wrong
with having redundant code as long as it's not harmful. I had a
paragraph here justifying why it could be removed but decided it was
simpler to just keep it.
This print is useful to know if an achievement (one that's not already
unlocked) would actually be unlocked in an end user environment, while
running the game in a dev environment.
Also fixed up the style of the function because it was definitely
inconsistent with the surrounding code.
This overhauls scriptclass::gamemode massively.
The first change is that it now uses an enum, and enforces using that
enum via using its type instead of an int. This is because whenever
you're reading any calls to startgamemode, you have no idea what magic
number actually corresponds to what unless you read startgamemode
itself. And when you do read it, not every case is commented adequately,
so you'd have to do more work to figure out what each case is. With the
enum, it's obvious and self-evident, and that also removes the need for
all the comments in the function too. Some math is still done on mode
variables (to simplify time trial code), but it's okay, we can just cast
between int and the enum as needed.
The second is that common code is now de-duplicated. There was a lot of
code that every case does, such as calling hardreset, setting Flip Mode,
resetting the player, calling gotoroom and so on.
Now some code may be duplicated between cases, so I've tried to group up
similar cases where possible (most notable example is grouping up the
main game and No Death Mode cases together). But some code still might
be duplicated in the end. Which is okay - I could've tried to
de-duplicate it further but that just results in logic relevant to a
specific case that's located far from the actual case itself. It's much
better to leave things like setting fademode or loading scripts in the
case itself.
This also fixes a bug since 2.3 where playing No Death Mode (and never
opening and closing the options menu) and beating it would also give you
the Flip Mode trophy, since turning on the flag to invalidate Flip Mode
in startgamemode only happened for the main game cases and in previous
versions the game relied upon this flag being set when using a
teleporter for some reason (which I removed in 2.3). Now instead of
specifying it per case, I just do a !map.custommode check instead so it
covers every single case at once.
While refactoring scriptclass::startgamemode, I noticed that these
variables weren't being reset. Fortunately, this doesn't seem to have
affected anything because they get overwritten one way or another in
startgamemode. But it's good just to be defensive and reset them anyway.
They are not reset in 2.2 or 2.0 glitchrunner mode because dying during
exiting to the menu is needed for credits warp, and that means the
checkpoint position needs to be maintained through.
This is to indicate when a code path is absolutely, for certain, 100%
unreachable. Useful as the default case inside a case-switch that is for
sure 100% exhaustive because it's inside the case of another case-switch
(and the default case is there to suppress compiler warnings about the
case-switch not being exhaustive), which is a situation coming up in my
scriptclass::startgamemode refactor.
It does this by deliberately invoking undefined behavior, either using a
compiler builtin that does the same thing or being a noreturn function
that returns. (And undefined behavior is not undefined behavior if it is
not executed in a code path, otherwise all NULL checks would be useless
because it'd dereference something that could be NULL in another code
path.)
I have no idea why they were floats in the first place. They are
coordinates, and coordinates are integer positions in this game. They
get converted to float only to be explicitly `static_cast`ed back to
ints in `testwallsy`.
This variable passes along the rule of the entity, which is an int. No
idea why it was converted to a float. Thankfully this is only used for
an unused block type, so it doesn't really matter.
This makes it so room names are no longer pointers to someone else's
memory, and instead to set them you use `mapclass::setroomname`. If the
string is short enough to fit in a static, no-alloc buffer, then it gets
copied there. Otherwise, a new heap allocation is made that duplicates
the string, and the new pointer is used instead.
This makes it possible for room names to contain arbitrary data whose
origin is temporary (e.g. from a script command that could be added in
the future).
This replaces all calls to SDL_free with a new macro, VVV_free, that
nulls the pointer afterwards. This mitigates any use-after-frees and
also completely eliminates double-frees. The same is done for any
function to free specific objects such as SDL_FreeSurface, with the
VVV_freefunc macro.
No exceptions for any of these calls, even if the pointer is discarded
or zeroed afterwards anyway. Better safe than sorry.
This is a macro rather than a function that takes in a
pointer-to-pointer because such a function would have type issues that
require casting and that's just not safe.
Even though SDL_free and other SDL functions already check for NULL, the
macro has a NULL check for other functions that don't. For example,
FAudioVoice_DestroyVoice does not check for NULL.
FILESYSTEM_freeMemory has been axed in favor of VVV_free because it
functionally does the same thing except for `unsigned char*` only.
Trinket and teleporter legends would be drawn even if they were out of
bounds. Trinket legends in particular were easy to do because you can
just place a trinket in a custom level and resize the map to not include
the room of the trinket.
Now, there are checks added so they won't be added if they are out of
bounds. This is in line with the fact that, since 2.3, if a trinket
exists outside of the map in custom levels, it won't count towards the
number of trinkets.
It's becoming pretty clear that the size of the map is important enough
to be queried a lot, but each time it's something like `map.custommode ?
map.customwidth : 20` and `map.custommode ? map.customheight : 20` which
is not ideal because of copy-pasting.
Furthermore, even `map.customwidth` and `map.customheight` are just
duplicates of `cl.mapwidth` and `cl.mapheight`, which are only set in
`customlevelclass::generatecustomminimap`. This is a bit annoying if you
want to, say, add checks that depend on the width and height of the
custom map in `mapclass::initcustommapdata`, but `map.customwidth` and
`map.customheight` are out of date because `generatecustomminimap`
hasn't been called yet. And doing the ternary there requires a `#ifndef
NO_CUSTOM_LEVELS` to reference `cl.mapwidth` and `cl.mapheight` which is
just awful.
So I'm axing `map.customwidth` and `map.customheight`, and I'm axing all
the ternaries that are duplicating the source of truth in
`MapRenderData`. Instead, there will just be one function to call for
the width and height, `mapclass::getwidth` and `mapclass::getheight`,
and everyone can simply call those without needing to do ternaries or
duplication.
The existing code was allergic to putting spaces between tokens, and had
some minor code duplication that I took the time to clean up as well.
The logic should be easier to follow now that the for-loops are no
longer duplicated for each of the map zoom levels.
I tested this by temporarily disabling map fog entirely and going
through a couple different custom levels to compare their minimap with
the existing code. These were A New Dimension and 333333 for 1x, Golden
Spiral and VVVV 4k for 2x, and VVVVVV is NP-Hard for 4x. There was no
difference in the output, not even a single pixel.
This adds color support to the output of the console on Windows. Now if
you're using Windows 10 build 1511 or later (I think it's build 1511
anyway; they added more VT sequence support in later versions), you will
see colors by default. This isn't due to Windows helping in any way;
this commit has to specifically enable it with SetConsoleMode() because
by default, Windows won't enable color support unless we enable it. (Or
if it's enabled in the registry, but having to go through the registry
to enable basic shit like that is completely fucking stupid.)
I tested this in my Windows 10 virtual machine and it's completely
working.
Previously, we were using `color_enabled` to mean both that the color
was supported and that it was enabled by the user (which it is enabled
by default). But this logic doesn't work well if the color check
function is called again and ends up enabling color after the user
disabled it. To fix this, just separate the two so the user controls one
`color_supported` variable and the `color_enabled` variable is separate.
Check both of them in order to print color, of course.
This adds the `-console` command-line option (for Win32 only) so the
game can spawn an attached console window which will contain all console
output.
This is to make it easier for people to debug on Windows systems.
Otherwise, the only way to get console output would be to either compile
the application as a console app (i.e. switch the subsystem to console)
- which is undesirable for regular users as this makes it so a console
is always spawned even when unwanted - or launch the game with shell
arguments that make it so output is redirected to a file.
As a result, color checking support is factored out of vlog_init() into
its own function, even though we don't support colors on Windows.
Using SDL_GetTicks() to seed the Gravitron RNG caused many
reproducibility issues while syncing https://tasvideos.org/7575S . To
fix this, add a frame counter, which is a number that is incremented
every frame and never resets, and use it instead.
If someone needs to switch back to SDL_GetTicks() for old TASes, then
provide the -seed-use-sdl-getticks command-line option for them.
In its previous location, it would only print the value of `s` after it
had been mutated by `splitmix32` four times, and it doesn't get used
after that, so the print isn't very useful.
Mixing code and declarations here is fine because starting from a few
months ago, we compile with C99 and if we ever need to compile with C90
then it's trivial to add braces surrounding the declarations.
If a music track has a loop comment with a negative value, ignore all
comments of the track. This is just to prevent any weirdness from
happening as it's safer to just let the track loop improperly. Also log
to the console to let users know.
This is the same thing that SDL_mixer does now:
libsdl-org/SDL_mixer@e819489459
This commit happened as a result of discussion on the VVVVVV Discord
server about SDL_mixer 2.0.4 behavior with weird loop comment values
(e.g. octal input with leading zeroes). This is simply updating the code
to be in line with what newer versions of SDL_mixer do.
Just in case it happens. Comments aren't really important to the game
(at worst a track will just loop in the wrong place) so it's fine to
carry on here and ignore all comments if this happens.
This does the following:
- Const-qualify variables if they are not modified
- Place each statement onto their own separate lines
- Place the asterisk with the type instead of the variable name
- Combine declarations and initializations where possible
VVVVVV uses submodules now, so you need to know how to initialize them.
I'm explicitly not including `git clone --recurse-submodules`. Usage of
submodules in git projects is kinda rare in my experience, so people
are used to doing simple clones, and that instruction would just result
in people being annoyed thinking they have to delete the repo they
already cloned, and clone it again except slightly differently.
It also doesn't help you if you need submodules that aren't in the
master branch (for example, if you clone my fork recursively and then
checkout the localization branch, you won't have C-HashMap and you'll
need the update command anyway). And you also need it whenever VVVVVV
updates its submodules. So teaching people just the update command is
better.
They weren't ever being used, and nobody really ever uses the return
value from the printf family of functions anyway. They return how many
bytes were actually printed, but if it's less than you expected then
there's not much you can really do about them. Also the vlog_* functions
were computing them inaccurately because I only set the return value to
the return value of vprintf when there's other print functions being
called, but regardless there's no reason to have a return value here
anyway.
The hilariously-named WIN32_LEAN_AND_MEAN slims down the number of
header files included in the already-massive `windows.h`. I know people
say Moore's law and precompiled headers and all that (well, we don't use
precompiled headers), but they kinda forgot about virtual machines, and
there's no reason not to define this and slim down the number of headers
anyway.
This started when I saw the warning that GetVersionExW was deprecated,
then looked it up and found StackOverflow answers saying that you should
basically detect the feature directly instead of checking the version,
which makes sense to me. Then I found that I could probably detect color
support by using GetConsoleMode and GetStdHandle. But then I asked
myself what the point was unless you could get color output directly in
the terminal, which it seems like you really can't if your app is a GUI
app. (I have no idea why Windows makes this pointless distinction
between console and GUI apps...)
I tested Command Prompt, PowerShell, Windows Terminal (which is just
PowerShell again), and even Git Bash (MINGW64), but none of them will
ever give the console output of a GUI app such as VVVVVV. The closest I
got is that Git Bash doesn't seem to detach the process, but it will
simply produce no output.
At this point I feel like it's not worth it keeping this code around if
it didn't even work in the first place, so I'm removing it. People can
always enable color by using the -forcecolor command-line argument
anyway.
I'm fine with putting the release version in a header file, thus
necessitating the need to recompile every file that includes it if it's
changed, simply because it's not supposed to be changed that often.
The SDL_arraysize is necessary because sometimes we'll have subreleases
(e.g. 2.4.1, 2.4.2, 2.4.3), and who knows, maybe we'll get to 2.10
someday.
This reworks how the commit hash and date are compiled so that if
they're changed (and they're changed often), only one source file needs
to be recompiled in order to update it everywhere in the game, no matter
how many source files use the hash or date.
The commit hash and date are now declared in InterimVersion.h (and they
need `extern "C"` guards because otherwise it results in a link fail on
MSVC because MSVC is stupid).
To do this, what now happens is that upon every rebuild,
InterimVersion.in.c is processed to create InterimVersion.out.c, then
InterimVersion.out.c is compiled into its own static library that is
then linked with VVVVVV.
(Why didn't I just simply add it to the list of VVVVVV source files?
Well, doing it _now_ does nothing because at that point the horse is
already out of the barn, and the VVVVVV executable has already been
declared, so I can't modify its sources. And I can't do it before
either, because we depend on the VVVVVV executable existing to do the
interim version logic. I could probably work around this by cleverly
moving around lines, but that'd separate related logic from each other.)
And yes, the naming convention has changed. Not only did I rename
Version to InterimVersion (to clearly differentiate it from
ReleaseVersion, which I'll be adding later), I also named the files
InterimVersion.in.c and InterimVersion.out.c instead of
InterimVersion.c.in and InterimVersion.c.out. I needed to put the file
extension on the end because otherwise CMake wouldn't recognize what
kind of language it is, and I mean like yeah duh of course it doesn't,
my text editor doesn't recognize it either.
I thought all of these were removed earlier but apparently not. Anyways,
add_definitions is bad because it pollutes the definitions of every
single target, we should be using target_compile_definitions instead.
This updates all references to SDL 2.0.22 to SDL 2.24.0, including the
Docker container that I maintain for Linux CI.
Be warned, this release of SDL updates the versioning scheme to be less
dumb. The previous version is 2.0.22, this release is 2.24.0 (so the
last number can be properly used for patch-level version releases).
This option is enabled by default and will replace absolute paths of all
source directory file paths with relative paths in the compiled binary,
if the compiler supports it. Of course, this isn't needed if you compile
with all paths removed anyways (e.g. in Release mode).
The purpose is to help make builds reproducible and to remove any
potentially sensitive information about the user or the user's system
from the compiled binary.
Both Clang and GCC support -fdebug-prefix-map, -fmacro-prefix-map, and
-ffile-prefix-map. In particular, -ffile-prefix-map is just a flag that
does both -fdebug-prefix-map and -fmacro-prefix-map.
According to https://reproducible-builds.org/docs/build-path/ ,
-fdebug-prefix-map is available in all GCC versions but only available
starting from Clang 3.8, and -fmacro-prefix-map and -ffile-prefix-map
are available since GCC 8 and Clang 10. So we check the compiler version
and use the available flags depending on if the compiler supports it or
not.
This does make debugging a bit more annoying, but there are a couple
ways to rectify this. Either disable it with
-DREMOVE_ABSOLUTE_PATHS=OFF, or add a `.gdbinit` that consists of
set substitute-path . ../..
so that `.` is considered to be `../..`. Of course, if you need to,
replace `../..` with the actual source directory path (in my case it's
`../../..` because I place my build folders in another subdirectory to
have multiple build folders in one directory).
This doesn't need to be a global `.gdbinit`, it can be in a
directory-specific `.gdbinit` (similar to how `.gitignore`s can also be
directory-specific). But then you need to add `add-auto-load-safe-path`
to your `.gdbinit` to load any directory-specific `.gdbinit`s.
The above is for GDB; I don't know what (if anything) needs to be done
for LLDB; I don't use LLDB.
Fixes#889.
Whereas all `SDL_assert`s will go away when compiling with optimization
flags and all plain `assert` calls (used in PhysFS) will go away when
compiling in Release mode, FAudio has a bunch of debug stuff that needs
to be explicitly disabled with its own `FAUDIO`-prefixed flag.
To do this in Release mode, we need to use generator expressions for
dumb CMake reasons. Basically, if checking the CMAKE_BUILD_TYPE variable
will not work for certain generators (Ninja, Visual Studio) because they
only specify the build type at build time, not generation/configuration
time.
This is so flags that apply globally (i.e. to the game and all static
libraries it's compiled with), such as /MT on MSVC, can be put in a
list, and along with putting all static libraries in a list, we remove
the need for each flag to be repeated for each static library and we can
just use a foreach loop instead.
(Global compile flags of course don't apply to us meddling with
CMAKE_C_FLAGS and CMAKE_CXX_FLAGS directly, because we need to do that
in order to make sure the C and C++ standards are set properly.)
This fixes a regression where the game ignored the amount of frames you
held down a direction if you released the direction during death.
Previously, the game only checked the amount of frames you held down a
direction if you were able to control the player. If you weren't able to
control the player (e.g. during the death animation), then the number of
frames it counted didn't change. This also meant that if you were
holding a direction before you died, but released it during death, the
game wouldn't zero out the number of frames you held it.
This behavior was useful because it meant you could keep the
deceleration momentum that you normally get by holding a direction for 5
frames just by holding a direction for less than 5 frames after dying,
if you had the rest of the hold frames before you died. This behavior is
what's used in https://tasvideos.org/7575S at around frame 7200.
Unfortunately, #609 made it so that the direction hold processing
happened even if the player didn't have control, meaning that it would
zero the hold frames during the death animation in the TAS, thus
desyncing it when it performed the maneuver it relied on the extra
momentum for after Viridian respawns.
The solution here is to just add the check back in again.
Fixes#887.
While fixing #885, I noticed that I had a bunch of
`special/stdin.vvvvvv` entries saved in my `levelstats.vvv`. At once I
knew that the dumb `special/stdin` hack that actually checks if the
filename passed is `special/stdin` was to blame.
STDIN playtesting was first merged, I knew in the back of my mind that
it was a bit of a dumb hack, but I didn't know it would cause
consequences like showing up in `levelstats.vvv`. For now, I'll just
have to patch it, but hopefully in the future I'll remove the dumb hack
entirely. Commenting both instances of the dumb hack with instructions
to grep for it should help maintainers out.
This is useful to investigate any TAS desync/reproducibility issues
relating to RNG, because even though I specifically separated the
Gravitron RNG away from other RNG and made it not dependent on the
system libc rand() function, there's still apparently some differences
in RNG execution between systems, resulting in TASVideos submission 7575
( https://tasvideos.org/7575S ) not syncing for everyone except the
author.
It seems that SDL_GetTicks(), which is used to seed the xoshiro RNG, is
not reliably consistent between systems, so in the future I will
probably replace it with a counter that is incremented each frame
starting from game startup, which is probably better.
This fixes the following warnings:
desktop_version/src/Music.cpp: At global scope:
desktop_version/src/Music.cpp:240:23: warning: non-static data member initializers only available with ‘-std=c++11’ or ‘-std=gnu++11’ [-Wc++11-extensions]
240 | Uint8 *wav_buffer = NULL;
| ^
desktop_version/src/Music.cpp:414:32: warning: non-static data member initializers only available with ‘-std=c++11’ or ‘-std=gnu++11’ [-Wc++11-extensions]
414 | Uint8* decoded_buf_playing = NULL;
| ^
desktop_version/src/Music.cpp:415:32: warning: non-static data member initializers only available with ‘-std=c++11’ or ‘-std=gnu++11’ [-Wc++11-extensions]
415 | Uint8* decoded_buf_reserve = NULL;
| ^
desktop_version/src/Music.cpp:416:21: warning: non-static data member initializers only available with ‘-std=c++11’ or ‘-std=gnu++11’ [-Wc++11-extensions]
416 | Uint8* read_buf = NULL;
| ^
These warnings are because the non-static data members (i.e. data
members that will be different for each instance of a class) are being
initialized at the same time as they're being declared, which means
that's what their value will be when the class instance is initialized.
However, this is only a C++11 feature, and we don't use C++11. Luckily,
we don't need to do this, and this is in fact redundant because we
already zero out the class instance in its constructor using
SDL_zerop(). Therefore, we should remove these initializers to fix
compliance with C++03.
Instead, for any number that isn't in the list of number words, just
return the regular Arabic numerical representation (i.e. just convert it
to string). It's better than having "Lots" or "???", neither of which
really tell you what the number actually is.
This flag makes it so the MSVC runtime libraries are statically linked.
This avoids needing Windows users to have these libraries installed.
Apparently /MT stands for "MultiThreaded", and there's a bit of a
history there where originally by default you could only have a
single-threaded library, and then the multi-threaded flags were added in
later.
First I tried doing target_compile_options on VVVVVV, but then got a
linker error. Then I tried doing add_compile_options because I figured
/MT had to be applied everywhere, and it seemed to work, but it still
linked to the runtime libraries. Apparently it was being overridden.
Then I tried target_compile_options again but this time did it to
everything, and that linked correctly and also removed the runtime
dependency. I would've tried using the MSVC_RUNTIME_LIBRARY property
- along with the CMP0091 policy - but those were only introduced in
CMake 3.15.
You can verify that a binary is built without dependencies by installing
LLVM and running llvm-readobj --needed-libs path/to/binary. This is the
output for a binary with runtime dependencies:
infoteddy@fedorarune ~/d llvm-readobj --needed-libs VVVVVV.exe
File: VVVVVV.exe
Format: COFF-i386
Arch: i386
AddressSize: 32bit
NeededLibraries [
ADVAPI32.dll
KERNEL32.dll
MSVCP140.dll
SDL2.dll
SHELL32.dll
USER32.dll
VCRUNTIME140.dll
api-ms-win-crt-heap-l1-1-0.dll
api-ms-win-crt-locale-l1-1-0.dll
api-ms-win-crt-math-l1-1-0.dll
api-ms-win-crt-runtime-l1-1-0.dll
api-ms-win-crt-stdio-l1-1-0.dll
api-ms-win-crt-string-l1-1-0.dll
api-ms-win-crt-time-l1-1-0.dll
api-ms-win-crt-utility-l1-1-0.dll
]
And this is the output for a binary with those dependencies having been
statically-linked in:
infoteddy@fedorarune ~/d llvm-readobj --needed-libs VVVVVV.exe
File: VVVVVV.exe
Format: COFF-i386
Arch: i386
AddressSize: 32bit
NeededLibraries [
ADVAPI32.dll
KERNEL32.dll
SDL2.dll
SHELL32.dll
USER32.dll
]
As already described in cc61194bed, as
well as Ved's commits from the last almost two weeks, starting VVVVVV
from Ved for playtesting could be made a lot faster by "preloading" the
game - letting it do all its asset loading in the background without
creating a window - and then waiting until the level is passed in via
stdin. There's only one problem left with this approach: VVVVVV
currently expects the starting position to be passed via command line
arguments, which isn't known yet at the time we'd like to start VVVVVV.
Therefore, this commit allows passing the starting position via the
level XML, instead of via arguments.
The extra XML looks like this, and is added next to the <Data> tag:
<Playtest>
<playx>214</playx>
<playy>112</playy>
<playrx>100</playrx>
<playry>100</playry>
<playgc>0</playgc>
<playmusic>4</playmusic>
</Playtest>
This is handled similarly to how the equivalent arguments are handled:
when the level metadata is loaded for CLI playtesting, we also try to
find this tag, and if it exists, it sets the same variables that the
arguments would have otherwise set.
There's always been a bit of an inconsistency in the game where enabling
invincibility would make spikes so solid that enemies and moving
platforms would treat spikes as solid and bounces off of them.
This fixes that by adding an `invincible` parameter to collision
functions, so the functions will only treat spikes as solid if that
parameter is true, and the parameter passed will only be true if it's
called for an entity that is a humanoid and invincibility mode is
enabled.
Also, for clarity, `spikecollide` is renamed to `towerspikecollide`
because it's only used for tower spikes. And as a small optimization,
`checktowerspikes` returns early if invincibility mode is enabled.
This is a minor optimization to streamline the experience of Ved
playtesting. Previously, the user would have to wait for all the assets
to load when launching playtesting (most of the time, I suspect, is
taken up by loading music from a vvvvvvmusic blob). With this
optimization, however, the game can be launched in the background and
its assets can be loaded, while it blocks on STDIN input. During this
time, the user in Ved will be choosing where to start playtesting. After
Ved provides STDIN input, then the window will be created and appears
instantaneously.
This also fixes a related issue in which providing an invalid
playtesting level name would result in a brief window flash that gets
instantly destroyed. With this, if the level is invalid then no window
is ever shown at all.
Probably should have done this earlier in 2.3, but better late than
never.
This makes it easier for third-party programs like Ved to detect what
version of the game this is.
Slightly quick-n-dirty for now, I'll de-duplicate the version number
later, and add commit hash and date if applicable.
Without this, entering in-game and opening the map with missing graphics
will result in a segfault. This is because even if the image doesn't
exist, it's still pushed into the `images` std::vector as a NULL
pointer. And it segfaults because we dereference it (to get things like
their width and height). In contrast, passing them to SDL is fine
because SDL always checks for NULL first.
There are three different places where we call PHYSFS_openRead. This
commit makes sure all of them print a statement upon failure along with
the PhysFS reason for failure, and assigns the log level of each print
as so:
- FILESYSTEM_loadFileToMemory: Debug print (previously no
print existed in the first place), because some files (such as
font.txt) may or may not be needed, but if it isn't then no need to
print and worry the user. The game will error anyway if a critical
file like a graphics file is missing.
- FILESYSTEM_loadBinaryBlob: Debug print (previously info print),
because sometimes it's not needed, such as mmmmmm.vvv. I remember one
user being worried that the game printed "Unable to open file
mmmmmm.vvv" when it's not critical unlike vvvvvvmusic.vvv (and that
file is assumed to exist if data.zip exists anyways). Though maybe we
should move to loose-leaf files to save on memory usage (and so we
don't have to use special tools to modify a binary blob)...
- FILESYSTEM_loadZip: Error print. If we're calling this function, we
really do expect the zip to be loaded, and if it fails because we
can't open the file in the first place, then it would be good to know
why.
This commit adds a new string formatting system to replace uses of
`SDL_snprintf` and string concatenation.
Making our own string formatting system has been briefly discussed
during the review of the localization branch, and on the VVVVVV
Discord. It's inspired by Python's format strings, but simpler.
This is primarily to benefit localization - strings will be easier to
understand (`Now using %s Tileset` → `Now using {area} Tileset`,
`"%s remain"` → `"{n_crewmates|wordy} remain"`), translators can change
the word order for their language's grammar (`%1$s` is a POSIX
extension), and this system is also less error-prone (making the format
string not align with the actual arguments won't result in a crash or
UB).
It also integrates our needs better - particularly the "wordy" numbers
without having to have a `help.number_words(n).c_str()` at the
callsite, translators can opt in and out of wordy numbers per string,
and this should also make it easier to solve #859.
This commit adds the formatting system itself, and changes one
`SDL_snprintf` in the code to use it as a small demo (the rest should
probably be done in the localization branch to avoid more unneeded
work).
The system is described in full detail in VFormat.h and in the pull
request description.
2.0.22 just released 40 minutes ago.
This also updates the `Dockerfile` to use the URL from the GitHub
releases page, instead of SDL's servers.
I've also pushed a new Docker container to
`ghcr.io/infoteddy/vvvvvv-build`.
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.
It's quite rare, though possible, that during finalstretch you could see
a glitchy tileset that looked like this:
https://i.imgur.com/V7cYKDW.png
This happened because final_mapcol, the variable that controls which
color of finalstretch is rendered, could end up being 7. Normally, it's
in the range of 1..6, which perfectly correlates with the Warp Zone
tilesets in tiles2.png, and the higher the number the farther back in
the tileset it goes from the gray Warp Zone tileset. However, if it's 7,
then it'll start grabbing tiles from the Ship plus some unused blank
tiles, which does not look pretty in the slightest.
This happened because it's possible, though exceedingly unlikely, that
fRandom(), a function which returns a float between 0..1, could return
exactly 1. fRandom() calls rand(), which returns a result between 0 and
RAND_MAX, and divides it by RAND_MAX. This value is implementation
dependent, but required to be at least 32767, and on most systems is
2147483647. Even taking the value of 32767, that means there's a 0.003%
chance that you could get this glitchy tileset when the game cycled the
color in finalstretch. But of course, playing the game for long periods
of time will eventually increase this chance - cycling the color 1,000
times (around 17 minutes of playing) will result in the chance being 3%.
Then as the calculations in the finalstretch color cycling logic calls
fRandom(), then multiplies by 6 and adds 1, it was possible for
fRandom() to return exactly 1, then have
6 added to it, resulting in final_mapcol being 7.
To fix this, just decrement the multiplication by fRandom() to multiply
by 5 instead of 6. Now the only possible numbers that calculation can
produce would be 1..6.
This fixes a limitation where the level filename had to be the exact
same name as the name of the zip, because the game used the name of the
level to identify the zip of which to load assets, and this also made it
impossible to use assets for more than one level in a zip.
Instead, we just look up where the level came from, so we can always
load its assets regardless of its filename.
Additionally, the zip structure checks can go away too, simplifying the
code further.
This WOULD be a huge breaking change, if it weren't for the fact that no
one uses them. Which is why I'm removing them, to simplify the code.
I asked on the VVVVVV Discord whether anyone used them or was even aware
of them and basically the answer was no. I go on Distractionware and no
one uses them. And why would they, when they'd have to distribute the
level .vvvvvv file separately? Better to just distribute everything in
one zip. And it's quite a bit obscure that you have to suffix the file
with .data.zip anyway.
During review of #869, I looked at this part of the codebase again. I
have no idea how or why, but during the course of 2.4 this whole area
just became a mess.
The issues I fixed (in no particular order):
- Copy-pasting the code that loads from the binary blobs
- Making sure SDL_RWFromConstMem is used over SDL_RWFromMem wherever
possible
- Adding checks to make sure the index from the binary blob is valid
(it's possible it could not exist)
- Adding checks to make sure we gracefully handle
SDL_RWFromConstMem/PHYSFSRWOPS_openRead returning NULL
- Moving the pointer asterisk to the type instead of the name :)
So, it turns out we weren't quite done fighting CMake yet...
To accommodate #869 (and actually also #272), the C standard was raised
from C90 to C99. This turned out to require a bit of a fight with the
CentOS CI's CMake version to get it to set the flags we wanted (and to
not overwrite them later). Eventually the fix was to move the block
that sets the standards to later in the file, which was done in
24353a54bb.
As it apparently turns out, if your CMake is at least 3.1.3 and
`CMAKE_<LANG>_STANDARD` is used instead of the workaround, the standard
setting now has an effect on the third party libraries, but not on
VVVVVV itself. The cause is (probably) the phrase "if it is set when a
target is created" in the CMake documentation - the
`CMAKE_<LANG>_STANDARD` values have to come before the VVVVVV target is
defined. In other words, the compiler's default C/C++ standard will be
used, probably something like C17 and C++17. As I can confirm with
`__cplusplus` and `__STDC_VERSION__` with my recent-enough CMake. If I
force the pre-3.1.3 workaround to be used, everything is compiled with
C99/C++98 as expected; and the `-fno-exceptions` `-fno-rtti` flags
appear everywhere regardless of version.
So my fix is to make the CMakeLists a little less complex by
simplifying away the `CMAKE_<LANG>_STANDARD` and
`CMAKE_<LANG>_EXTENSIONS`, and always using the workaround regardless
of CMake version. There's nothing wrong with the workaround, the same
thing is also done for `-fno-exceptions` `-fno-rtti`, and it's good to
have a less complicated CMakeLists that doesn't do different and
unexpected things for different versions.
The previous commit f6d7a214f8 ended up
breaking CI because the workaround ended up breaking the PhysFS build
too, which was previously relying on extensions to compile.
Since #869 is going to require C99 anyways, I might as well just up the
standard now. That way the PR won't have to fight it too.
Previously, if the user had a CMake version below 3.1.3, we told them to
set `-std` themselves.
However, we are going to go to C99 soon (because of FAudio, see #869),
and CentOS 7's CMake is too old to set `-std=` automatically, defaulting
to C90. This is bad because it fails the CI.
To work around this, we set `-std=` ourselves, but first we have to
clear any existing `-std=` flag in C_FLAGS or CXX_FLAGS. Amusingly
enough, MSVC does not have `/std:` switches for either C90 or C++98, so
we just get to do nothing.
This isn't necessary, but it does silence these annoying logs if you
pass an invalid argument or don't have data.zip:
[ERROR] Could not get window size: Invalid renderer
[WARN] Stats not loaded! Not writing unlock.vvv.
[ERROR] Could not get window size: Invalid renderer
[WARN] Settings not loaded! Not writing settings.vvv.
To do this, I've added FILESYSTEM_isInit().
This means we are no longer copy-pasting PhysFS source files directly.
Since the source files reside in a src/ subdirectory, the paths in the
CMakeLists.txt have to be adjusted.
We are no longer copy-pasting LodePNG source files directly.
As we can't rename lodepng.cpp to lodepng.c in the submodule itself, we
need to make a wrapper file, lodepng_wrapper.c, that #includes
lodepng.cpp, but gets compiled as C.
This prevents writing to unlock.vvv or settings.vvv if the game hasn't
made an attempt to load them yet. Otherwise, if the game aborted via
VVV_exit() because of, say, failure to parse a graphics file, it would
overwrite perfectly existing valid save data since it hasn't loaded it
yet.
Fixes#870.
Another cause of #870 is d0ffafe117, as a
bisect tells me. What that commit did is remove screenbuffer as a
pointer, since it's a statically-allocated object that _should_ always
exist, and it removed the `screenbuffer == NULL` guards in savestats()
and savesettings(). Unfortunately, those guards did something very
important - namely, they prevented writing to the save files when the
filesystem wasn't initialized. But that wasn't made clear, because it
seemed like the point of those guards was to prevent dereferencing NULL.
So instead, explicitly make it clear that
FILESYSTEM_saveTiXml2Document() needs to fail if the filesystem isn't
initialized. I've done this by adding an isInit bool to
FileSystemUtils.cpp.
Issue #870 showed one of the problems that this game has, namely that it
only sometimes checks SDL return values, and did not do so in this case.
Part of the cause of #870 is that Screen::GetWindowSize does not check
the return value of SDL_GetRendererOutputSize, so when that function
fails (as in the case where m_renderer is NULL and does not exist), it
does not initialize the out values, so it ends up writing uninitialized
values to the save files.
We need to make sure every function's return value is checked, not just
SDL functions, but that will have to be done later.
While reviewing #272, I noticed that the PR was passing these two
arguments through a helper function, even though they really shouldn't
ever change. To obviate the need to pass these through, I'm making them
global variables.
pathSep is just a string literal from PhysFS, while basePath is a whole
complicated calculation from SDL and needs to be freed. It will be freed
upon filesystem deinit (as is done with PhysFS and the STDIN buffer).
Additionally the logic in FILESYSTEM_init is simplified by no longer
needing to keep a retval variable or use gotos to free basePath in
there.
This lets any script name use capitals and spaces all they want, while
still being able to jump to them via iftrinkets() or similar.
The issue is that whenever tokenize() is ran, all spaces are stripped
and every argument is lowercased before being put into `words`. So, the
solution here is to create a raw_words array that doesn't perform space
stripping or lowercasing, and to refer to that whenever there's a script
command that loads a script. We keep the lowercasing and space removal
elsewhere to be more forgiving to newcomers.
This is technically a forwards compatibility break, but it's only a
minor one, and all levels that utilize it can still be easily modified
to work on older versions anyway.
As reported by Dav999, Victoria and Vermilion's trophy colors are
swapped again in 2.4. He points to
37b7615b71, the commit where I fixed the
color masks of every single surface to always be RGB or RGBA.
It sounded plausible to me, because it did have to do with colors, after
all. However, it didn't make sense to me, because I was like, I didn't
touch the trophy colors at all after I originally fixed them.
After I ruled out the RGBf() function as a confounder, I decided to see
whether intentionally reversing the color order in RGBf() to be BGR
would do anything, and to my surprise it actually swapped the colors
back around and it didn't actually look bad.
And then I realized: Swapping the trophy colors between RGB and BGR
ordering results in similar colors that still look good, but are simply
wrong, but not so wrong that they take on a color that no crewmate uses,
so it'd appear as if the crewmates were swapped, when in reality the
only thing that was swapped was actually the color order of the colors.
Trying to fix this by swapping the colors again, I actively confused
colors 33 and 35 (Vermilion and Victoria) with colors 32 and 34
(Vitellary and Viridian), so I was confused when Vermilion and Victoria
weren't swapping. Then as a debugging step, I only changed 34 to 32
without swapping 32 as well, and then finally noticed that I was
swapping Vitellary and Viridian, because there were now two Vitellarys.
And then I was reminded that Vitellary and Viridian were also wrongly
swapped since 2.0 as well.
And so then I finally realized: The original comments accompanying the
colors were correct after all. The only problem was that they were fed
into a function, RGBf(), that read the colors backwards, because the
codebase habitually changed the color order on a whim and it was really
hard to reason out which color order should be used at a given time, so
it ended up reading RGB colors as BGR, while it looked like it was
passing them through as-is.
So what happened was that in the first place, RGBf() was swapping RGB to
BGR. Then I came and swapped Vermilion and Victoria, and Vitellary and
Viridian around. Then later I fixed all the color masks, so RGBf()
stopped swapping RGB and BGR around. But then this ended up swapping the
colors of Vermilion and Victoria, and Vitellary and Viridian once again!
Therefore, swapping Vermilion and Victoria, and Vitellary and Viridian
was incorrect. Or at least, not the fix to the root cause. The root
cause would be to swap the colors in RGBf(), but this would be sort of
confusing to reason about - at least if I didn't bother to just type the
RGB values into an image editor. But that doesn't fix the real issue,
which is that the game kept swapping RGB and BGR around in every corner
of the codebase.
I further confirmed that there was no more RGB or BGR swapping by
deleting the plus-one-divide-by-three transformation in RGBf() and
seeing if the colors looked okay. Now with the colors being brighter, I
could see that passing it straight through looked fine, but
intentionally reversing it to be BGR resulted in colors that at a
distance looked okay, but were either washed out or too bright. At least
finally I could use my 8 years of playing this game for something.
So in conclusion, actually, 37b7615b71
("Fix surface color masks") was the real fix, and
d271907f8c ("Fix Secret Lab Time Trial
trophies having wrong colors") was the real regression. It's just that
the regression came first, but it wasn't really a regression until I did
the other fix, so the fix isn't the regression, the regression is...
this is hurting my brain. Or the real regression was the friends we made
along the way, or something like that.
This is the most trivial bug ever caused by the technical debt of those
god-awful reversed color masks.
---
This reverts commit d271907f8c.
Fixes#862.
In hindsight, the FAudio pointer will likely be in SoundTrack since we will
want to keep the mastering voice closer to the sounds and their source voice
arrays, while the MusicTrack will likely just be one source voice that gets
PCM from different streams.
This looks redundant but will actually help in the transition to FAudio; we
mostly want to keep the game logic the same while reimplementing the current
mixer, weirdness and all. Once that's done and confirmed to be stable and
consistent we can start cutting out the workarounds and quirks.
This meant making the track vectors static, but that's kind of what we do with musicclass anyway?
In any case, this will make the transition to FAudio MUCH less invasive.
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++?
Issue #849 suggested making integer be the default on Big Picture and
Steam Deck, but after thinking about it more, I think it's better and
more simple to just default to integer mode in general.
Reason being that people in Big Picture shouldn't expect the picture to
look different if they're out of Big Picture but still in fullscreen, or
have the picture look different in fullscreen depending on if they
launched the game for the first time in Big Picture or not. And besides,
the less lines of code, the better. So I'm just making integer mode the
default.
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)
This de-duplicates the code, simplifying the codebase and reducing the
number of code paths that needs to be maintained. It also adds
robustness checks to LoadIcon that weren't there before (checking that
loading the file succeeded and that decoding the file also succeeded).
Now, you might think that loading the image with alpha will change
things in some way. But actually, I tested it, and I'm pretty sure it
doesn't. Since my window manager, i3, doesn't display icons, I've had to
resort to this hacky multi-liner
( https://unix.stackexchange.com/a/48866 ) to dump the icon to a PAM
file. I don't know what a PAM file is and all my various attempts to
convert it into something readable failed. But what I did instead was
just grab the icon of the game before this commit (on 2.3, just to be
extra sure), and `diff`ed it with the grabbed icon now, and they end up
being the exact same file. So there's literally no difference.
The only other consideration is that LoadImage needs to be exported,
since it's implemented in GraphicsResources.cpp. I just opted to
forward-declare it right before LoadIcon in Screen.cpp, since it's
really the only other time it's used. No need to create a new header
file for it or anything.
This is just to simplify the function. I really don't see any point in
taking away the alpha for some images, other than to disappoint people
who mod the game assets. It just complicates loading the image with no
real gain. To reduce maintenance costs, let's remove this alternate code
path.
Also it's a default argument and I don't like default arguments.
This argument... doesn't do anything.
First off, setting it to true explicitly enables blending on the
resulting surface, which is kind of the exact opposite of the variable
name and is misleading to say the least? And secondly, SDL surfaces have
blending enabled by default anyways, so it still doesn't even do
anything.
It's also a default argument, and I'm not one to shy away from removing
such default arguments.
This includes:
- Removing the constructor in favor of actually being able to see that
there's an actual function called being made initializing the struct
- Removing the use of a reference in Screen::init() in favor of using a
pointer
- Adding the struct qualifier everywhere (it's not much typing),
although technically you could typedef it in C, but I'd rather much
not typedef just to remove a tag qualifier
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.
In aa7b63fa5f, I didn't notice that the
result was implicitly being converted to int by the min/max from before.
I instead added it to the existing char, but that resulted in a char
overflow (it's unsigned, so thankfully not undefined behavior).
But of course the entire point of that commit is to make it explicitly
clear when you are converting between types, intentionally or otherwise,
in min/max comparisons. So despite causing a regression (which I have
now fixed), at least it did its job.
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.
The reason why the wall stuck flipping behavior happened in the first
place was because the code went like this:
if (jumppressed)
{
if (onground && gravitycontrol == 0)
{
gravitycontrol = 1;
}
if (onroof && gravitycontrol == 1)
{
gravitycontrol = 0;
}
}
Basically, if you were both on ground and on a roof (i.e. stuck in a
wall), you would flip, but then due to code order and the fact that the
statement is not connected to the previous one, you would immediately
unflip afterwards. But if you were already flipped then the only path
that can be taken is to unflip you, since it's the statement that
appears last.
52fceb3f69 replaces the onground/onroof
conditionals with any_onground/any_onroof, so any player entity would
allow you to flip. But otherwise the code is the same. So is that the
problem?
No; tracing it through with GDB reveals that when you flip,
gravitycontrol is being set to 1, but never being set to 0. And it turns
out that's because any_onroof is not getting set. And that happens
because of another thing that 52fceb3f69
did - which was to set any_onground/any_onroof to true if indeed any
player entity was on ground or on a roof.
Unfortunately, the way Leo did it was to make the two statements
mutually exclusive - an 'if'-'else if' instead of two separate
statements. So a single entity could not mark both any_onground and
any_onroof as true (and the majority of the time, you will be a single
entity).
Thus, the solution is to just drop that 'else'.
Fixes#855.
I noticed when going frame-by-frame in Vertigo that sometimes the
wrapping enemies at the top sometimes just "popped" in frame. This is
because the sprite warp code only draws the warping sprite of sprites at
the bottom of the screen if they're below y=210. However, the warp point
starts at y=232, and warp sprites can be at most 32x32, which is exactly
the case with the Vertigo sprites, which are exactly 32x32. So the warp
code should start warping sprites if they're below y=200 (232 - 32)
instead.
Horizontal warping also has this problem; it warps at x=320 and
starts drawing warp sprites at x=300, even though it should start
drawing at x=288 (320 - 32). I've gone ahead and fixed that as well.
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.
Whew, look at all those copy-pasted print statements!
Doing this because of the in-game timer feature. The text would
otherwise clash harshly with the timer otherwise. Even with the outline
it still clashes, but at least there's an outline so it's not as harsh.
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.
Currently, all game-gamestate variables are just ints. This is not
particularly type-safe, in case the number of enums changes. To verify
that all current uses of the game-gamestate variables actually use the
enums, change them to be typed with the enum instead.
(As an aside, we should probably rename this so that it can't be
confused with Terry's state machine that has several different ways to
exploit to warp you to the credits, but that's something to do later.)
You'll note that getting in to the glitchy state of the game (the state
where you could play the game after it had hardreset() called on it)
required the player to quit to menu with ingame_titlemode set to true.
Well, quitting to menu calls hardreset(). So if hardreset() is called
when quitting, then you can no longer preserve ingame_titlemode that
way. This is a bit overkill, but I'm just taking precautions.
The game will now assert if the main menu is created while
ingame_titlemode is true, or if we attempt to load into a mode while
it's true. And if assertions are disabled then it just stops doing it
anyway.
I don't think there's any way to get a glitched ingame_titlemode again,
ever since I removed save data deletion taking you back to the main
menu. But I've had enough bugs with the fact that we more-or-less use
the same state for main menu options and in-game options, and that
glitched ingame_titlemode bug DID just happen, so I'm taking
precautions.
The next commit will add logic that more-or-less quits the whole block
if ingame_titlemode, and instead of adding another layer of indentation
I will just pull this into its own function so we can use a return
statement.
While I was testing deleting data while you were in-game, I noticed that
deleting data gave you all the "Win with less than X deaths" trophies,
even if you never got any of them before deleting data. Well, it turns
out that if you have the best game death count of 0, then you win every
trophy, and if you have the best game death count of -1 then that means
you haven't completed the game yet.
This reset was added in e3bfc79d4a, so at
least it's not in 2.3, but I only have myself to blame for making this
mistake. Whoops.
Going back to the main menu allowed for glitchiness to occur if you
deleted your save data while in in-game options. This meant you could
then load back in to the game, and then quit to the menu, then open the
options and then jump back in-game, exploring the state of the game
after hardreset() had been called on it. Which is: pretty glitchy.
For example, this meant having your room coordinates be 0,0 (which is
different from 100,100, which is the actual 0,0, thanks for the
100-indexing Terry), which caused some of the room transitions to be
disabled because room transitions were disabled if the
game.door_up/down/left/right variables were -2 or less, and they were
computed based on room coordinates, which meant some of them went
negative if you were 0,0 and not 100,100. At least this was the case
until I removed those variables for, at best, doing nothing, and at
worst, being actively harmful.
Anyways, so deleting your save data now just takes you back to the
previous menu, much like deleting custom level data does. I don't know
why deleting save data put you back on the main menu in the first place.
It's not like the options menu needed to be reloaded or anything. I
checked and this was the behavior in 2.0 as well, so it was probably
added for a dumb reason.
I considered prohibiting data deletion if you were ingame_titlemode, but
as of the moment it seems to be okay (if albeit weird, e.g. returning to
menu while in Secret Lab doesn't place your cursor on the "play"
button), and I can always add such a prohibition later if it was really
causing problems. Can't think of anything bad off of the top of my head,
though.
Btw thanks to Elomavi for discovering that you could do this glitch.
Okay, so, this is the elephant sprite, right?
https://i.imgur.com/dtS70zk.png
This is how it looks in the actual game, when you stitch all the rooms
together:
https://i.imgur.com/aztVnFT.png
Looks kind of messed-up, doesn't it?
Okay, so, in the bottom two rooms (11,9) and (12,9), the elephant is
placed at y-position -152. But in (11,8) and (12,8), it's placed at
y-position 96. This is despite the fact that -152 plus 240 is 88, not
96.
Similarly, in the left two rooms (11,8) and (11,9), the elephant is
placed at x-position 64, but in the right two rooms (12,8) and (12,9),
the elephant is placed at -264. This is despite the fact that 64 minus
320 is -256, not -264.
All of this stems from the calculations in Otherlevel.cpp using offsets
of -248 and -328 instead of -240 and -320.
So there's an 8-pixel offset that causes the elephant to be chopped off
when viewed with all the rooms stitched together. Simple enough to fix.
For the y-position fixes, I decremented the initial 8-pixel multiplier
as well, else the elephant would sink into the floor.
And this is what the elephant looks like now after stitching:
https://i.imgur.com/27ePLm1.png
Thanks to Tzann for pointing this out.
These warnings are kinda spammy, and they make sense in principle.
vlog_error takes a format string, so passing it an arbitrary string
(even error messages from libraries) isn't a good idea.
Dvoid from Discord just reported a crash when trying to load a
custom tiles2.png that was encoded weirdly.
The problem is that we don't check the return value from LodePNG, so
LodePNG gives us a null pointer, and then
SDL_CreateRGBSurfaceWithFormatFrom doesn't check this null pointer,
which then propagates until we crash in SDL_ConvertSurfaceFormat (or
rather, one of its sub-functions), and we would probably crash somewhere
else anyway if it continued.
After properly checking LodePNG's return value, along with printing the
error, it turns out that Dvoid's custom tiles2.png had an "invalid CRC".
I don't know what this means but it sounds worrying. `feh` can read the
file correctly but it also reports a "CRC error".
While we can't fix Canonical, we can at least work around them, and help
people on Ubuntu out by linking them to my comment listing the
currently-known workarounds.
SDL_GetTicks64() is a function that got added in SDL 2.0.18, which is
just an SDL_GetTicks() without a value that wraps every ~49 days,
instead wrapping after the sun explodes and kills us all. Oh sorry,
didn't mean to get existential.
For now, put this behind an SDL_VERSION_ATLEAST guard, which will be
removed when SDL 2.0.18 officially releases and we can update to it.
