I'm going to refactor drawing an entity out to a separate function, and
since I'm going to do that, I might as well make some things const to
clarify intent first and foremost and possibly improve performance or
compiler optimization.
Instead of using the same tower buffer that gets used for towers, use a
separate buffer instead so there's no risk of stepping on the tower
buffer's toes at the wrong point in time.
This commit combined with the previous one fixes#369.
With the previous commit in place, we can now simply move some usages of
the previous towerbg to use a separate object instead. That way, we
don't have to mess with a monolithic state, or some better way to phrase
what I just said, and we instead have two separate objects that can
coexist side-by-side.
Previously, the tower background was controlled by a disparate set of
attributes on Graphics and mapclass, and wasn't really encapsulated. (If
that's what that word means, I don't particularly care about
object-oriented lingo.) But now, all relevant things that a tower
background has has been put into a TowerBG struct, so it will be easy to
make multiple copies without having to duplicate the code that handles
it.
For some reason, this `tl` is a `point`? But the only other time the
name `tl` is used elsewhere in the code is a float on a `textboxclass`.
Regardless, this is unused.
On the deltaframes of the tower background, there would be "pixel bleed"
if the tower background would be scrolling from the top. This is because
there wouldn't be any more pixels from above the screen to grab, because
the background rendering functions didn't draw any pixels above the
screen. But they couldn't draw any pixels above the screen, because that
was simply the end of the buffer. But now that the buffer is expanded,
we can now draw above the screen, and fix this glitchy interpolation
rendering.
In order to fix the weird title screen pixels at the top on deltaframes,
we'll need to have a bit more space at the top. Also to the left, in
case we need a background to scroll from the left in the future.
The window icon is simply another asset that can be customized by level
makers. And in fact, one of my levels changes the window icon. It's
simply named VVVVVV.png, but it doesn't sit in the graphics folder,
rather it sits in the root VVVVVV directory.
I noticed that this asset was missed when per-level assets loading was
added, so I decided to add it in.
There's a NULL check on screenbuffer because reloadresources() gets
called before screenbuffer's init() does.
I was investigating a desync in my Nova TAS, and it turns out that
the gravity line collision functions check for the `oldxp` and `oldyp`
of the player, i.e. their position on the previous frame, along with
their position on the current frame. So, if the player either collided
with the gravity line last frame or this frame, then the player collided
with the gravity line this frame.
Except, that's not actually true. It turns out that `oldxp` and `oldyp`
don't necessarily always correspond to the `xp` and `yp` of the player
on the previous frame. It turns out that your `oldyp` will be updated if
you stand on a vertically moving platform, before the gravity line
collision function gets ran. So, if you were colliding with a gravity
line on the previous frame, but you got moved out of there by a
vertically moving platform, then you just don't collide with the gravity
line at all.
However, this behavior changed in 2.3 after my over-30-FPS patch got
merged (#220). That patch took advantage of the existing `oldxp` and
`oldyp` entity attributes, and uses them to interpolate their positions
during rendering to make everything look real smooth.
Previously, `oldxp` and `oldyp` would both be updated in
`entityclass::updateentitylogic()`. However, I moved it in that patch to
update right before `gameinput()` in `main.cpp`.
As a result, `oldyp` no longer gets updated whenever the player stands
on a vertically moving platform. This ends up desyncing my TAS.
As expected, updating `oldyp` in `entityclass::movingplatformfix()` (the
function responsible for moving the player whenever they stand on a
vertically moving platform) makes it so that my TAS syncs, but the
visuals are glitchy when standing on a vertically moving platform. And
as much as I'd like to get rid of gravity lines checking for whether
you've collided with them on the previous frame, doing that desyncs my
TAS, too.
In the end, it seems like I should just leave `oldxp` and `oldyp` alone,
and switch to using dedicated variables that are never used in the
physics of the game. So I'm introducing `lerpoldxp` and `lerpoldyp`, and
replacing all instances of using `oldxp` and `oldyp` that my over-30-FPS
patch added, with `lerpoldxp` and `lerpoldyp` instead.
After doing this, and applying #503 as well, my Nova TAS syncs after
some minor but acceptable fixes with Viridian's walkingframe.
By "unnecessary qualifiers to self", I mean something like using the
'game.' qualifier for a variable on the Game class when you're inside a
function on the Game class itself. This patch is to enforce consistency
as most of the code doesn't have these unnecessary qualifiers.
To prevent further unnecessary qualifiers to self, I made it so the
extern in each header file can be omitted by using a define. That way,
if someone writes something referring to 'game.' on a Game function,
there will be a compile error.
However, if you really need to have a reference to the global name, and
you're within the same .cpp file as the implementation of that object,
you can just do the extern at the function-level. A good example of this
is editorinput()/editorrender()/editorlogic() in editor.cpp. In my
opinion, they should probably be split off into their own separate file
because editor.cpp is getting way too big, but this will do for now.
This fixes a bug where font_positions wouldn't get cleared after exiting
a custom level that had a font.txt if it didn't exist in the default
graphics, leading to messed-up-looking font rendering.
This bug was originally discovered by Ally.
When this is done, there is potential for a mistake to occur when
writing out the bounds check, which is eliminated when using the macro
instead. Luckily, this doesn't seem to have happened, but what's even
worse is I hardcoded 400 instead of using SDL_arraysize(ed.level), so if
the size of ed.level the bounds checks would all be wrong, which
wouldn't be good. But that's fixed now, too.
Since there's an INBOUNDS_ARR() macro, it's much better to specify the
macro for the vector is a macro for the vector, to avoid confusion.
All usages of this macro have been renamed accordingly.
Due to #464, standing inside a gravity line during a gotoroom that
occurs every frame will end up with the gravity line being gray instead
of being white. To temporarily fix this (until #464 is properly fixed),
I decided to add some kludge that colors it white if its onentity is 1.
I tested this patch with gravity lines in both constant-gotoroom and
normal environments, and it seems to be fine for both.
This patch fixes a regression caused by commit
6b1a7ebce6.
When you spawn a crewmate with an invalid color, by doing something like
`createentity(100,100,18,-1,0)` (here the color is -1, which is
invalid), a white crewmate with the color of solid white (255, 255, 255)
would appear.
That is, until AllyTally came along and committed commit
6b1a7ebce6 (Make "[Press ENTER to
return to editor]" fade out after a bit) (PR #158). Then after that
commit, it would seem like the crewmate didn't appear, but in reality
they were just invisible, because they had an invisible color.
As part of Ally's changes, to properly support drawing text with a
certain amount of alpha, she made BlitSurfaceColoured() account for the
alpha of the color given instead of only caring about the RGB of the
color, discarding the alpha, and using the alpha of the surface it was
drawing instead. This effectively made it so the alpha of whatever it
was drawing would be 255 all the time, except for if you had custom
textures and your custom textures had translucent pixels.
However, the default color set by Graphics::setcol() if you didn't
provide a valid color index was 0xFFFFFF. Which is only (255, 255, 255)
but ends up having an alpha value of 0 (because it's actually
0x00FFFFFF). And all colors drawn with alpha 0 end up being drawn with
alpha 0 after 6b1a7ebce6. So
invalid-colored entities will end up being invisible.
To fix this, I just decided to add alpha to the default value instead.
In addition, I used getRGB() to be consistent with all the other colors
in the function.
For some reason, there's some color-clamping code in this function
directly after already-existing color-clamping code. This code dates
back to 2.2. And also, there's a smaller lower-bound of -1 for the red
channel, despite the fact that this smaller value doesn't matter because
the red would get clamped to 0 by the first code anyway.
So even if this was put here for some strange reason, it doesn't matter
because it doesn't do anything anyway.
When I added the over-30-FPS mode, I kept running into this problem
where the special images of text boxes would render during the
deltaframes of fade-in/fade-out animations, even though they shouldn't
be. So I simply added a flag to the text box that enables drawing these
special images.
However, this doesn't solve the problem fully, and there's still a small
chance that a special-image text box could draw another special image
during its deltaframes. It's really rare and you have to have your
deltaframe luck juuuuuust right (or you could use libTAS, probably), but
it helps to be in 40% slowmode and have a high refresh rate (which, if
it isn't a multiple of 30, you should disable VSync, too, in order to
not have a low framerate).
So instead, special images will only be drawn if the text box has fully
faded in completely. That solves the issue completely.
ScaleSurfaceSlow() uses DrawPixel() instead of SDL_FillRect() to scale a
given surface, which is slow and inefficient, and makes it less likely
that the game could be moved to SDL_Render.
Unfortunately, it has this weird -1,-1 offset, but that will be fixed in
the next commit.
So, earlier in the development of 2.0, Simon Roth (I presume)
encountered a problem: Oh no, all my backgrounds aren't appearing! And
this is because my foregroundBuffer, which contains all the drawn tiles,
is drawing complete black over it!
So he had a solution that seems ingenius, but is actually really really
hacky and super 100% NOT the proper solution. Just, take the
foregroundBuffer, iterate over each pixel, and DON'T draw any pixel
that's 0xDEADBEEF. 0xDEADBEEF is a special signal meaning "don't draw
this pixel". It is called a 'key'.
Unfortunately, this causes a bug where translucent pixels on tiles
(pixels between 0% and 100% opacity) didn't get drawn correctly. They
would be drawn against this weird blue color.
Now, in #103, I came across this weird constant and decided "hey, this
looks awfully like that weird blue color I came across, maybe if I set
it to 0x00000000, i.e. complete and transparent black, the issue will be
fixed". And it DID appear to be fixed. However, I didn't look too
closely, nor did I test it that much, and all that ended up doing was
drawing the pixels against black, which more subtly disguised the
problem with translucent pixels.
So, after some investigation, I noticed that BlitSurfaceColoured() was
drawing translucent pixels just fine. And I thought at the time that
there was something wrong with BlitSurfaceStandard(), or something.
Further along later I realized that all drawn tiles were passing through
this weird OverlaySurfaceKeyed() function. And removing it in favor of a
straight SDL_BlitSurface() produced the bug I mentioned above: Oh no,
all the backgrounds don't show up, because my foregroundBuffer is
drawing pure black over them!
Well... just... set the proper blend mode for foregroundBuffer. It
should be SDL_BLENDMODE_BLEND instead of SDL_BLENDMODE_NONE.
Then you don't have to worry about your transparency at all. If you did
it right, you won't have to resort this hacky color-keying business.
*sigh*
This function checked the intersection of rectangles, but it used floats
for some reason when its only caller used ints. There's already an
intersection function (UtilityClass::intersects()), so we should be
using that one instead in order to minimize code duplication.
Graphics::Hitest() is used for per-pixel collision detection, directly
checking the pixels of both entities' sprites. I checked with one of my
TASes and it still syncs, so I'm pretty sure this won't cause any
issues.
If you have the translucent room name option enabled, you'd always be
seeing the spikes at the bottom of the screen hidden behind the room
name. This patch makes it so that the spikes get carefully cropped so
they only appear above the room name when the player gets close to the
bottom of the screen.
The function was not actually checking the number that would end up
being used to index the tiles3 vector, and as a result there could
potentially be out-of-bounds indexing. But this is fixed now.
Okay, so basically here's the include layout that this game now
consistently uses:
[The "main" header file, if any (e.g. Graphics.h for Graphics.cpp)]
[blank line]
[All system includes, such as tinyxml2/physfs/utfcpp/SDL]
[blank line]
[All project includes, such as Game.h/Entity.h/etc.]
And if applicable, another blank line, and then some special-case
include screwy stuff (take a look at editor.cpp or FileSystemUtils.cpp,
for example, they have ifdefs and defines with their includes).
Including a header file inside another header file means a bunch of
files are going to be unnecessarily recompiled whenever that inner
header file is changed. So I minimized the amount of header files
included in a header file, and only included the ones that were
necessary (system includes don't count, I'm only talking about includes
from within this project). Then the includes are only in the .cpp files
themselves.
This also minimizes problems such as a NO_CUSTOM_LEVELS build failing
because some file depended on an include that got included in editor.h,
which is another benefit of removing unnecessary includes from header
files.
I ran the game through cppcheck and it spat out a bunch of member
attributes that weren't being initialized. So I initialized them.
In the previous version of this commit, I added constructors to
GraphicsResources, otherlevelclass, labclass, warpclass, and finalclass,
but flibit says this changes the code flow enough that it's risky to
merge before 2.4, so I got rid of those constructors, too.
Graphics::drawmenu() no longer has copy-pasted code for each individual
case. Instead, the individual cases have their own adding on to common
code, which is far easier to maintain.
Also, the only difference Graphics::drawlevelmenu() does is in some
y-positioning stuff. There's no reason to make it a whole separate
function and duplicate everything AGAIN. So it's been consolidated into
Graphics::drawmenu() as well, and I've added a boolean to draw a menu
this way if it's the level menu.
Instead, the string in MenuOption is just a buffer of 161 chars, which
is 40 chars (160 bytes if each were the largest possible UTF-8 character
size) plus a null terminator. This is because the maximum length of a
menu option that can be fit on the screen without going past is 40
chars.
There's no need to use a template here. Just manually call SDL_tolower()
or SDL_toupper() as needed.
Oh yeah, and use SDL_tolower() and SDL_toupper() instead of libc
tolower() and toupper().
These unused vars are:
- Graphics::bfontmask_rect
- Graphics::backgrounds
- Graphics::bfontmask
- GraphicsResources::im_bfontmask
While it seems that Graphics::backgrounds was indexed in
Graphics::drawbackground(), in reality there was never anything in that
vector and thus actually using it would cause a segfault.
The problem we're running into is entirely contained in the Screen - we need to
either decouple graphics context init from Screen::init or we need to take out
the screenbuffer interaction from loadstats (which I'm more in favor of since we
can just pull the config values and pass them to Screen::init later).
These functions will only complain once if they receive an out-of-bounds
tile. And it's only once because these functions are called frequently
in rendering code.
A macro WHINE_ONCE() has been added in order to not duplicate code.
Disabling the one-way recolor if assets are mounted is needed to make
existing levels not look bad, but what about levels that want to use the
recolor anyway?
The best solution here is to just introduce another bool into the XML,
and make the re-color opt-in and only present if assets are mounted if
that tag is present.
Some levels (like Unshackled) have decided to manually re-color the
one-way tiles on their own, and us overriding their re-color is not
something they would want. This does mean custom levels with custom
assets don't get to take advantage of the re-color, but it's the exact
same behavior as before, so it shouldn't really matter that much.
I would've liked to specifically detect if a custom tiles.png or
tiles2.png was in play, rather than simply disabling it if any asset was
mounted, but it seems that detecting if a specific file was mounted from
a specific zip isn't really PHYSFS's strong suit.
One-ways have always had this problem where they're always yellow. That
means unless you specifically use yellow, it'll never match the tileset.
The best way to fix this without requiring new graphics or changing
existing ones is to simply re-tint the one-way with the given color of
the room. That way, the black part of the tile is still black, but the
yellow is now some other color.
The game for some reason had this thing where it would not draw the
diagonal background tiles if you had animated backgrounds turned off.
Which is weird, because spikes with that background are still drawn as
spikes with that background. And also, it doesn't do this for any of the
tower hallway rooms, which is inconsistent.
Better to simplify the logic in Render.cpp anyways by removing
graphics.drawtower_nobackground() and making it really clear what
exactly we'll do if backgrounds are turned off. ("Aren't we already not
drawing the background? What's this _nobackground() function for?")
The only reason why gray Warp Zone entities were green originally was
because there is a giant concatenated list of tileset+tilecol
combinations, and by using tileset 3 tilecol 6 you're using the entry
of tileset 4 tilecol 0, which is the green Ship tileset.
So without interfering with the green Ship tileset's entry, I've decided
that the best thing to do is to just add special cases. The enemy color
was easy enough to fix. The platform color was also easy to fix.
However, there exist no existing textures for gray conveyors, so at that
point I decided to just tint the existing green one gray, and then I did
the same for platforms.
All menus had a hardcoded X position (offset to an arbitrary starting
point of 110) and a hardcoded horizontal spacing for the "staircasing"
(mostly 30 pixels, but for some specific menus hardcoded to 15, 20 or
something else). Not all menus were centered, and seem to have been
manually made narrower (with lower horizontal spacing) whenever text
ran offscreen during development.
This system may already be hard to work with in an English-only menu
system, since you may need to adjust horizontal spacing or positioning
when adding an option. The main reason I made this change is that it's
even less optimal when menu options have to be translated, since
maximum string lengths are hard to determine, and it's easy to have
menu options running offscreen, especially when not all menus are
checked for all languages and when options could be added in the middle
of a menu after translations of that menu are already checked.
Now, menus are automatically centered based on their options, and they
are automatically made narrower if they won't fit with the default
horizontal spacing of 30 pixels (with some padding). The game.menuxoff
variable for the menu X position is now also offset to 0 instead of 110
The _default_ horizontal spacing can be changed on a per-menu basis,
and most menus (not all) which already had a narrower spacing set,
retain that as a maximum spacing, simply because they looked odd with
30 pixels of spacing (especially the main menu). They will be made even
narrower automatically if needed. In the most extreme case, the spacing
can go down to 0 and options will be displayed right below each other.
This isn't in the usual style of the game, but at least we did the best
we could to prevent options running offscreen.
The only exception to automatic menu centering and narrowing is the
list of player levels, because it's a special case and existing
behavior would be better than automatic centering there.
The tilesheets in question are font.png, tiles.png, tiles2.png,
tiles3.png, entcolours.png, teleporter.png, sprites.png, and
flipsprites.png.
This patch removes the hardcoded dimensions when scanning the
tilesheets, because it's simpler that way. It also de-duplicates it so
it isn't a bunch of copy-paste, by using macros. (I had to use macros
because it was the easiest way to optionally pass in some extra code in
the innermost for-loop.)
Also, if the dimensions of a scanned tilesheet aren't exactly multiples
of the dimensions of the tile unit for that given tilesheet (e.g. if the
dimensions of a scanned tiles.png are not exact multiples of 8), then an
SDL_SimpleMessageBox will show up with the error message, a puts() of
the error message will be called, and the program will exit.
Similar to Graphics::map_tab(), this ensures that I don't have to
copy-paste printing the map options for every single game.menupage case
I want, and in this case that's a good thing because there'll be 4
game.menupage cases I'll be using.
This function is useful to de-duplicate all the map page names at the
bottom, which are MAP, CREW/SHIP/GRAV, STATS, and SAVE. If selected, it
will surround the text in square brackets and automatically handle the
positioning.
Shamelessly copy-pasted from Dav999's localization branch.
Ugh, this is terrible and stupid and I hate myself for it.
Anyway, since the SDL2 VSync hint only applies when the renderer is
created, we have to re-create the renderer whenever VSync is toggled.
However, this also means we need to re-create m_screenTexture as well,
AND call ResizeScreen() after that or else the letterbox/integer modes
won't be applied.
Unfortunately, this means that in main(), gameScreen.init() will create
a renderer only to be destroyed later by graphics.processVsync().
There's not much we can do about this. Fixing this would require putting
graphics.processVsync() before gameScreen.init(). However, in order to
know whether the user has VSync set, we would have to call
game.loadstats() first, but wait, we can't, because game.loadstats()
mutates gameScreen! Gahhhhhh!!!!
@leo60228 suggested to fix that problem (
https://github.com/TerryCavanagh/VVVVVV/pull/220#issuecomment-624217939
) by adding NULL checks to game.loadstats() and then calling it twice,
but then you're trading wastefully creating a renderer only to be
destroyed, for wastefully opening and parsing unlock.vvv twice instead
of once. In either case, you're doing something twice and wasting work.
This adds Graphics::crewcolourreal(), which is like the
entityclass::crewcolour() that the editor already uses, except for the
real color instead of the color ID. Also, editorclass now has an
attribute `entcolreal` so enemy colors don't update more than 30 frames
a second.
The solution is to draw another row of incoming textures. And also just
draw another row of textures when the background needs to be redrawn,
otherwise it'll flicker when the color changes while you're holding down
ACTION.
To fix this, I draw another row/column of incoming textures. But of
course, I have to extend the size of the towerbuffer, otherwise the
incoming textures will just be gone.
This could happen if you held down ACTION in the credits, looks like the
background doesn't keep up for some reason. That's another bug to fix,
but at least I can fix this overdraw.
There's still a problem in that the flickering that would lead to this
overdraw in the first place still exists. But at least if it'll flicker,
it'll flicker black and not overdraw.
Currently it interpolates it based on the current state of game.swngame,
but when game.swngame changes the interpolation doesn't know that it has
JUST changed or anything. So add a kludge variable to fix this
off-by-one.
These colors were of the colors of each crewmate, the inactive crewmate
color, and the color of the trinket and clock on the quicksave/summary
screens.
These colors all used fRandom() and so kept updating too quickly because
they would be recalculated every time the delta-timestep render function
got called, which isn't ideal. Thus, I've had to add attributes onto the
Graphics class to store these colors and make sure they're only
recalculated in logic functions instead.
Thankfully, the color used for the sprites on the time trial results
screen doesn't use fRandom(), so I don't have to worry about those.
There's a new version of Graphics::drawsprite() that takes in a pre-made
color already, instead of a color ID. As well, I've also added
Graphics::updatetitlecolours() to update these colors on the title
screen.
Okay, so the problem here is that Graphics::setcol() is called right
before a sprite is drawn in a render function, but render functions are
done in deltatime, meaning that the color of a sprite keeps being
recalculated every time. This only affects sprites that use fRandom()
(the other thing that can dynamically determine a color is help.glow,
but that's only updated in the fixed-timestep loop), but is especially
noticeable for sprites that flash wildly, like the teleporter, trinket,
and elephant.
To fix this, we need to make the color be recalculated only in the
fixed-timestep loop. However, this means that we MUST store the color of
the sprite SOMEWHERE for the delta-timesteps to render it, otherwise the
color calculation will just be lost or something.
So each entity now has a new attribute, `realcol`, which is the actual
raw color used to render the sprite in render functions. This is not to
be confused with their `colour` attribute, which is more akin to a color
"ID" of sorts, but which isn't an actual color.
At the end of gamelogic(), as well as when an entity is first created,
the `colour` is given to Graphics::setcol() and then `realcol` gets set
to the actual color. Then when it comes time to render the entity,
`realcol` gets used instead.
Gravitron squares are a somewhat tricky case where there's technically
TWO colors for it - one is the actual sprite itself and the other is the
indicator. However, usually the indicator and the square aren't both
onscreen at the same time, so we can simply switch the realcol between
the two as needed.
However, we can't use this system for the sprite colors used on the
title and map screen, so we'll have to do something else for those.
In order to make sure colors don't update more than 1000/34 frames per
second, I'll have to move the color-setting part of this function
somewhere else.
Otherwise, the tile animations will go too fast. However, the overall
color cycling hasn't been going fast, since it was never in gamerender()
in the first place.
These special images are the crewmates, Level Complete, and Game
Complete images. They flashed depending on if you were lucky and
happened to got your delta-timesteps just right when text boxes were
fading in and out.
Honestly, I'm surprised text box fading in/out hasn't ran into this
issue before. It's insane luck that this issue hasn't occurred before or
anything.
Well, anyways, to fix this, there's now an attribute `allowspecial` on
text boxes, and an optional parameter of the same name for
Graphics::createtextbox(). This attribute is the only thing that will
let these special text box images render. And any createtextbox()es that
utilize these special images have been updated accordingly.
Just to make sure it's extra smooth. Not that it will be noticeable, and
you can't access the Secret Lab in slowmode without modifying the game,
but it's nice to have this.
Otherwise it'll go by really fast and rapidly pulsate. To the point
where it seems like it would be an epilepsy trigger, although I
wouldn't know anything about epilepsy other than that it's bad.
Ok, now THIS takes the cake for "only really noticeable in slowmode",
because it only ever moves at 1 pixel per second. And even then,
slowmode shouldn't apply on the title screen, so it won't even show up
there once I get around to doing that change.
This is so the background doesn't NYOOOOM past at light speed. Although
for a game set in space like VVVVVV, light speed ain't bad.
And this finally requires that editorlogic() have a call to
Graphics::updatebackground().
This makes text boxes fade in and out pretty smoothly.
This requires that the textboxclass::setcol() be in Graphics::drawgui(),
so now it's moved there.
Text box fading is only really noticeable if you're playing in slowmode.
Now it's really, really smooth. Except for like the last frame when it
goes down, which I sometimes didn't notice (but maybe it didn't happen
every time due to being lucky on the delta timesteps or something,
whatevs.)
Otherwise the screen will shake too fast for my liking.
Also I'm planning to add an FPS limiting option later (because right
now, un-capping the FPS is pretty wasteful and eats up lots of
resources, especially since I have only a 60hz monitor), and it'd feel
weird if screen shaking updated every delta timestep.
I've added a function Graphics::lerp() which simply interpolates between
two values given a certain alpha value. It's just like drawing a
straight line between two points.
Also, Graphics now has an `alpha` attribute, and it is set on every
deltatime update to be used in linear interpolation.
The flashy color of the elephant can be hard on people's eyes,
especially if they're the type who want screen effects disabled because
they might have epilepsy. The elephant takes up a good 3/4ths of the
screen, you know. If screen effects are disabled, the elephant will use
color 22, which is a neutral gray.
I'm only adding this because the VVVVVV speedrun mods (@tzann, @mohoc)
invalidate all runs that have the elephant texture removed, even though
many people would be looking at a potentially epilepsy-inducing image
many times a day grinding 100% speedruns. (Imo, their justification for
this is flimsy at best.)
The only class that actually needs its i/j/k kept is scriptclass,
because some custom levels rely on it for creating custom activity
zones. So I haven't touched that.
Other than that, there's no chance that anything important relies on
i/j/k in any other class. For that to be the case, it would have to use
i/j/k without initializing it beforehand, and that can simply be
detected by removing the attribute from the header file and seeing where
the compiler complains. And the compiler complains only about cases
where it's initialized first. (Note that due to this check, I *haven't*
removed Graphics's `m` as it precisely does exactly this, using it
without initializing it first.)
Interestingly enough, otherlevelclass and towerclass have unused i/k
variables for whatever reason.
If you don't have a font.txt, it could happen that a font index is
requested that's out-of-bounds. And that would result in a segfault. So
to fix that I'm adding INBOUNDS checks to all functions that index the
fontmap.
This fixes indexing out-of-bounds in the functions that draw all the
special images such as the elephant and teleporters. Let's make sure the
game doesn't segfault.
I tracked down all the functions that took in an entity's drawframe and
made sure that no matter what value an entity's drawframe was, the game
would never segfault.
This is to respect the fact that the top half of the file is indented
with spaces, while the bottom half is indented with tabs.
Graphics::reloadresources() is on the bottom half.
Main game would retain custom level assets, now fixed. Also, custom fonts load properly. Finally, levels can be stored as a zip and placed in the levels folder, with the .vvvvvv file at the root of the zip and custom asset folders (graphics, sounds etc) also at the root.
Since translucent roomname backgrounds were introduced in
TerryCavanagh/VVVVVV#122, it exposes one glaring flaw with the game that
until now has been kept hidden: in rooms with room names, the game
cheapens out with the tile data and doesn't have a 30th row, because the
room name would hide the missing row. As a result, rooms with room names
have 29 rows instead of 30 to fill up the entire screen. And it looks
really weird when there's nothing but empty space behind the translucent
room name background.
To remedy this, I added one row to each room with a room name in the level.
First, I had to filter out all the rooms with no room names. However, that's
actually all contained in Otherlevel.cpp, the Overworld, which contains 221
rooms (8 of which are the Secret Lab, 6 more of which are the Ship, so 207 are
the actual Overworld, right? Wrong, 2 of those Overworld no-roomname rooms are
in the Lab, so there are actually 205 Overworld rooms). The remaining level
data files all contain rooms with room names.
But the process wasn't that easy. I noticed a while ago that each room
contains 29 `tmap.push_back()`s, one for each row of the room, and each row is
simply a string containing the 40 tiles for that row, concatenated with
commas.
However, I decided to actually check my intuition by doing a grep on each
level file and counting the number of results, for example `grep 'push_back'
Labclass.cpp | wc -l`. Whatever number comes out should be divisible by 29.
That particular grep on Labclass.cpp returns 1306, which divided by 29 is 45
with a remainder of 1.
So what does that mean? Does that mean there's 45 rooms each, and 1 leftover
row? Well, not exactly. The extra row comes from the fact that Outer Space has
30 rows instead of 29. Outer Space is the room that comes up when the game
finds a room is non-existent, which shouldn't happen with a properly-working
game, except in Outside Dimension VVVVVV. In fact, each level file has their
own Outer Space, and every single Outer Space also has 30 rooms. So really,
this means there are 44 rooms in the Lab and one Outer Space room. (Well, in
reality, there are 46 rooms in the Lab, because 2 of them use the Outside
tileset but have no room names, so they're stored in Otherlevel.cpp instead.)
We find the same result for the Warp Zone. `grep 'push_back' WarpClass.cpp |
wc -l` returns 697, which is 24 remainder 1, meaning 23 rooms of 29 rows and 1
room of 30 rows, which corresponds with 23 rooms in the Warp Zone and one
Outer Space room.
However, Outside Dimension VVVVVV + Tower Hallways and Space Station 1 and 2
are both odd curiosities. Finalclass.cpp contains Outside Dimension VVVVVV,
(which is Intermission 1 and 2 and the Final Level), but also the Tower
Hallway rooms, i.e. the auxiliary Tower rooms that are not a part of the main
tower. Spacestation2.cpp contains both Space Station 1 and 2, so don't be
deceived by the name.
`grep 'push_back' Finalclass.cpp | wc -l` returns 1597, which is actually 55
remainder 2. So... are there two rooms with 30 rows? Yes, in fact, The
Gravitron and Outer Space both contain 30 rows. So there are actually 55 rooms
stored in Finalclass.cpp (not including the minitowers Panic Room and The
Final Challenge), 54 rooms of actual level data and one Outer Space room, and
breaking down the 54 rooms even further, 51 of them are actually in Outside
Dimension VVVVVV and 3 of them are Tower Hallways. Of the 51 Outside Dimension
VVVVVV rooms, 14 of those are Intermission 1, 4 of them are Intermission 2,
and the rest of the 33 rooms are the Final Level (again, not including the
minitowers).
`grep 'push_back' Spacestation2.cpp | wc -l` returns 2148, which is 74
remainder 2. Are there two rooms with 30 rows again? No; one of those counted
2148 rows is a false-positive, because there's an if-else in Prize for the
Reckless that replaces the row with spikes with a row without spikes if you
are in a time trial or in No Death Mode. So there's 73 rooms in Space Station
1 and 2, and one Outer Space room.
With all this in mind, I decided to duplicate the current last row of each
room, the 29th row, to add a 30th row. However, I wasn't going to do this
automatically! But neither was I going to write some kludge-y code to parse
each nightmare of a level file and duplicate the rows that way.
Enter: Vim macros! (Er, well, actually, I use Neovim.) I first did
`/push_back`, so that pressing `n` would keep going to the next `push_back` in
the file. Then I went to the 29th row of the first room in the file, did a
`Yp`, and then started my macro with `qq`. The macro went like this: `30nYp`,
which is simply going to the 29th row of the next room over and duplicating
it. And that's all there was to it. However, I had to make sure that (1) my
cursor was before the `push_back` on the line of the 29th row of the room, and
(2) that I didn't skip rooms, both of which were problems I encountered when
pressing Ctrl+Z a given invocation of the macro (the Ctrl+Z is just a
metaphor, you actually undo by typing `u` in Vim). And also I had to make sure
to be careful around the extra lines of `push_back`s in Prize for the Reckless
and The Gravitron, and make sure I didn't run past the end of the file and
loop back around. Thankfully, all Outer Space rooms are at the end of each
file.
But first, I had to increase the number of rows drawn in Graphics.cpp by 1 in
order to compensate for this, and do the same when reading the tile data in
Map.cpp. I had to change fillcontent(), drawmap(), drawfinalmap(),
drawtowermap(), and drawtowermap_nobackground(). Funnily enough, the tower
functions already used 30 rows, but I guess it's an off-by-one due to the
camera scrolling, so they now draw 31 rows each.
Then, I went in-game to make sure that the row behind each room name looked
fine. I checked EVERY single room with a room name. I turned on invincibility
mode and added a temporary line to hardreset() that always turned on
game.nocutscenes for a smoother playtesting experience. And to make sure that
rooms which have entirely empty bottom rows actually still have 30 rows,
instead of having 29 and the game assuming that the 30th row was empty
(because that sounds like it could lead to Undefined Behavior), I added this
temporary debugging line to the start of mapclass::fillcontent():
printf("(%i,%i) has %i rows\n", game.roomx, game.roomy, (int) tmap.size());
Everywhere I checked - and I made sure to check all rooms - every room had 30
rows and not 29 rows.
Unfortunately, some rooms simply couldn't be left alone with their 29th row
duplicated and had to be manually edited. This was because the 29th row would
contain some edge tiles because the player would be able to walk somewhere on
the 28th, 27th, and 26th rows, and if you duplicated said edge tiles behind
the room name, it would look bad.
Here's a list of rooms whose 30th rows I had to manually edit:
- Comms Relay
- The Yes Men
- Stop and Reflect
- They Call Him Flipper
- Double-slit Experiment
- Square Root
- Brought to you by the letter G
- The Bernoulli Principle
- Purest Unobtainium
- I Smell Ozone
- Conveying a New Idea
- Upstream Downstream
- Give Me A V
- $eeing Dollar $ign$
- Doing Things The Hard Way
- Very Good
- Must I Do Everything For You?
- Now Stay Close To Me...
- ...But Not Too Close
- ...Not as I Do
- Do Try To Keep Up
- Whee Sports
- As you like it
This is actually a strange case where it looked bad because of the 29th
row, instead of the 30th row, and I had to change the 29th row instead of
the 30th row to fix it.
- Maze With No Entrance
- Ascending and Descending
- Mind The Gap
Same strange case as "As you like it" (it's the 29th row I had to change
that was the problem, not the 30th).
- 1950 Silverstone Grand V
- The Villi People
I found that Panic Room and The Final Challenge also looked strange behind the
roomname background, but I can't do much about either because towers' tile
data wrap around at the top and bottom, and if I added another row to either
it would be visible above the room name.
I've considered updating the development editors with these new level tiles,
but I decided against it as the development editors are already pretty
outdated anyway.
While I was working on my over-30-FPS patch, I found out that the tower
background in the credits scroll was being completely re-drawn every
single frame, which was a bit wasteful and expensive. It's also harder
to interpolate for my over-30-FPS patch. I'm guessing this constant
re-draw was done because the math to get the surface scroll properly
working is a bit subtle, but I've figured the precise math out!
The first changes of this patch is just removing the unconditional
`map.tdrawback = true;`, and having to set `map.scrolldir` everywhere to
get the credits scrolling in the right direction but make sure the title
screen doesn't start scrolling like a descending tower, too.
After that, the first problem is that it looks like the ACTION press to
speed up the credits scrolling doesn't speed up the background, too. No
problem, just shove a `!game.press_action` check in
`gamecompletelogic()`.
However, this introduces a mini-problem, which is that NOW when you hold
down ACTION, the background appears to be slowly getting out of sync
with the credits text by a one-pixel-per-second difference. This is
actually due to the fact that, as a result of me adding the conditional,
`map.bscroll` is no longer always unconditionally getting set to 1,
while `game.creditposition` IS always unconditionally getting
decremented by 1. And when you hold down ACTION, `game.creditposition`
gets decremented by 6.
Thus, I need to set `map.bscroll` when holding down ACTION to be 7,
which is 6 plus 1.
Then we have another problem, which is that the incoming textures desync
when you press ACTION, and when you release ACTION. They desync by
precisely 6 pixels, which should be a familiar number. I (eventually)
tracked this down to `map.bypos` being updated at the same time
`map.bscroll` is, even though `map.bypos` should be updated a frame
later AFTER updating `map.bscroll`.
So I had to change the `map.bypos` update in `gamecompleteinput()` and
`gamecompletelogic()` to be `map.bypos += map.bscroll;` and then place
it before any `map.bscroll` update, thus ensuring that `map.bscroll`
updates exactly one frame before `map.ypos` does. I had to move the
`map.bypos += map.bscroll;` to be in `gamecompleteinput()`, because
`gamecompleteinput()` comes first before `gamecompletelogic()` in the
`main.cpp` game loop, otherwise the `map.bypos` update won't be delayed
by one frame for when you press ACTION to make it go faster, and thus
cause a desync when you press ACTION.
Oh and then after that, I had to make the descending tower background
draw a THIRD row of incoming tiles, otherwise you could see some black
flickering at the bottom of the screen when you held down ACTION.
All of this took me way too long to figure out, but now the credits
scroll works perfectly while being more optimized.
It's less code being copied and pasted, especially since for my
over-30-FPS patch I would have to make a separate function for each if
both of them were still there, but if they're unified into one then I
will only have to make one more function.
And since map.scrolldir is now used outside of GAMEMODE, we'll need to
reset it in hardreset() and when exiting playtesting.
Due to the previous commit, the descending tower background now has to
account for map.bscroll, or else it will be off by one pixel from the
incoming textures. But ascending tower backgrounds work fine, so no need
to do anything with those.
Looks like this was done as a quick fix instead of taking the time to
figure out the math needed to actually draw the incoming textures, which
is fair enough - it only makes one room, Panic Room, slightly laggier.
While I was working on my over-30-FPS patch, though, I came across the
fact that this background kept getting entirely redrawn every frame, and
it seems like it would be easier to interpolate descending tower
backgrounds if we scrolled what was already there instead.
Here, we have to draw two rows of incoming textures, otherwise the
scrolling surface will produce black lines.
This de-duplicates the screen effects rendering code by putting it
inside a function, Graphics::renderwithscreeneffects(), and using that
instead of copy-pasted code.
This removes lots of duplicated code that drawtowerentities() did,
because all that really changed was accounting for map.ypos (which can
be done conditionally) and where and when the room wrapped (which can
also be done conditionally).
This fixes an oddity that's only visual, which could only happen in
custom levels by using the createentity() internal command.
For the same reason that the second through fourth tiles of moving
platforms on the top and left was buggily rendered, SDL_BlitSurface()
strikes again to mutate the SDL_Rect we pass it and render the next
SDL_BlitSurface() call inbounds, even though we don't need it to.
Previously, the game could end up rendering a warping sprite twice due
to the fact that it could run "if entity is on the right side of the
screen" right after "if entity is on the left side of the screen" (but
not the other way around). This is most noticeable if you have a custom
player sprite with translucent pixels and stand on the left side of a
warping screen, but the code suggests it happens when warping through
the top of the screen, too.
Instead of doing
if (!obj.entities[i].invis)
{
...
}
It's better to do
if (obj.entities[i].invis)
{
continue;
}
...
It reduces the indentation by one level, which is always a good thing.
In the last commit, I removed having the flip mode conditional directly
inside the sprite-drawing code for each size type, which would reduce
the indentation one level. However, I opted to hold off un-indenting
until this commit, otherwise it would've produced too much noise.
The game uses flipsprites.png instead of sprites.png when in flip mode,
mostly to add exceptions for sprites that SHOULDN'T be flipped in flip
mode.
Looks like to achieve this, the routines for sprite drawing got
copy-and-pasted every single time flipsprites.png needed to be
conditionally used, resulting in large amounts of copy-pasted code. And
this copy-pasted code resulted in copy-paste errors, with relating to
VVVVVV-Man, because apparently due to two copy-pasting errors, the
combined giant crewmate in the epilogue uses flipsprites.png even if you
aren't in flip mode, and it also uses the width instead of the height of
sprites_rect when in flip mode (although, this doesn't end up mattering,
but still).
The solution here is to simply change the referenced sprites vector to a
pointer that can conditionally change based on the game being in flip
mode or not.
This commit fixes a bug where the second, third, and fourth tiles of
moving platforms would render offset from the first tile if the moving
platform hit the top or left edge of the screen.
This is due to the fact that SDL_BlitSurface() will end up changing the
coordinates of the rectangle we pass to it to be 0 if they're negative,
but only after it's already been drawn. Previously, we kept re-using the
same rectangle each time we drew each segment of the moving platform,
but since it only changes the draw rectangle after it's already been
drawn, the first tile shows up fine, but not the rest of the tiles,
hence resulting in an offset.
To fix this, we do the same thing as we did for drawing the "really big
sprite" (size-type 9): just reset the rectangle we use every time we
draw a segment of the moving platform.
They're literally the exact same thing, except one is 4 and the other
has an 8. No need to have all that copy-pasted code.
Actually, the only other difference was that size-type 8 set the
drawRect to sprites_rect instead of tiles_rect for some reason? Even
though it doesn't matter, anyway, because SDL_BlitSurface() only cares
about the X and Y you pass it, not the width and height, which is the
only difference between tiles_rect and sprites_rect.
To make sure that people wouldn't wonder where size-type 8 went if they
saw a blank space between size-type 7 and size-type 9, I kept the
size-type 8 conditional, but inside it is just a comment telling you to
go to size-type 2.
Instead of copy-pasting all the BlitSurfaceStandard()s all over again,
just make the referenced vector a pointer that changes depending on
map.custommode.
There's a noticeable seam in the horizontal and warp backgrounds
whenever you enter a new room. Entering a new room triggers the game to
re-draw the entire warp background instead of simply scrolling what it
already has. This seam is the result of the initial background draw
being misaligned with the rest of the scrolling.
If you get out your measuring tools, you'll see that it's misaligned by
exactly 3 pixels (this applies to both horizontal and vertical warping).
If you look at the part of the code where the game draws fresh warping
textures after scrolling the existing ones offscreen, you'll see it
starts with an offset of 317, which is exactly 320 minus 3. And for
vertical, it uses 237, which is exactly 240 minus 3.
This is where the misalignment comes from. Since the incoming textures
are drawn 3 pixels to the left, but the initial draw isn't, this results
in a misalignment and causes the seam.
To fix this, draw the initial draw of the horizontal and vertical warp
backgrounds 3 pixels to the left.
This makes it more convenient if you have a large levels directory, as
some people in the VVVVVV custom levels community do.
On the first page, this option will change to be "last page" instead.
Since the addition of another menu option pushes up the list of levels
too close to the selected level data itself, I've had to move the list
of levels down by 4 pixels (but "next page"/"previous page"/"return to
menu" are still in their same position).
This feature was already added to VCE but hasn't been upstreamed until
now.
Firstly, menu options are no longer ad-hoc objects, and are added by
using Game::option() (this is the biggest change). This removes the
vector Game::menuoptionsactive, and Game::menuoptions is now a vector of
MenuOption instead of std::string.
Secondly, the manual tracker variable of the amount of menu options,
Game::nummenuoptions, has been removed, in favor of using vectors
properly and using Game::menuoptions::size().
As a result, a lot of copy-pasted code has been removed from
Game::createmenu(), mostly due to having to have different versions of
menus depending on whether or not we have certain defines, or having an
mmmmmm.vvv file inside the VVVVVV directory. In the old days, you
couldn't just add or remove a menu option conveniently, you had to
shuffle around the position of every other menu option too, which
resulted in lots of copy-pasted code. But now this copy-pasted code has
been de-duplicated, at least in Game::createmenu().
This removes the variables graphics.ntextbox, as well as removing
'active' from each text box object. Thus, all text boxes are really
real, and you don't have to check its 'active' variable.
Something that's slightly annoying is that in order to make the vector
of text boxes be properly used, the text box cannot remove itself.
Because the text box does not know it's in a vector. So move the removal
of the text box to drawgui() instead.
This removes the variables obj.nentity and obj.nlinecrosskludge, as well
as removing the 'active' attribute from the entity class object. Now
every entity you access is guaranteed to be real and you don't have to
check the 'active' variable.
The biggest part of this is changing createentity() to modify a
newly-created entity object and push it back instead of already
modifying an indice in obj.entities.
As well, removing an entity now uses the new obj.removeentity() function
and removeentity_iter() macro.
This removes a bunch of commented-out code that was clearly kept from
the Flash version, even though the Flash graphics API is much different
than SDL's. Also removes a bunch of TODOs that either say nothing, or
say something whose meaning has been totally lost to time due to being
completely vague, or something that's already been done and someone
forgot to remove the TODO.
Unlike, say, the scriptclass i/j/k stuff, these tempstrings are only
purely visual, and there's no known glitches to manipulate them anyway.
Thus I think it's safe to make this cleanup.
