Just like all the other fixes, the variable that controls the amount of
ghosts to show was being updated every render frame instead of every
logic frame.
This is because due to the game loop changes in this over-30-FPS patch,
editorrender() can be called and undo graphics.backgrounddrawn being set
to false once again. Solution here is to make it so it keeps being set
to false until game.shouldreturntoeditor is turned off, which has also
been moved to editorlogic().
This is to make sure no lerping occurs in 30-FPS mode, otherwise things
might look weird. A good case that this fixes is how entities look in a
double-gotoroom (they should be completely frozen in 30-FPS mode).
There is now an option in "graphic options" named "toggle fps", which
toggles whether the game visually runs at 1000/34 FPS or over 1000/34
FPS. It is off by default.
I've had to put the entire game loop in yet another set of braces, I'll
indent it next commit.
What happens here is that the entity gets created and then gets
immediately updated on the next frame, but there's no time for their
walkingframe of 0 to be rendered, so it'll look like they have just
started with walkingframe 1. However in the delta-timestep rendering
it'll render with walkingframe 0. So we need to fix their drawframe and
increment it when creating them.
Looks like mapclass::changefinalcol() is called whenever you enter a
room in Outside Dimension VVVVVV.
mapclass::changefinalcol() changes the tile, but it doesn't update their
drawframe. So after that function is called, update their drawframe.
If you update their drawframe while inside that function, then when the
platform actually cycles color it'll cycle backwards quickly sometimes,
which is not ideal.
When I loaded the room where Vitellary is in Space Station 2, I saw this
1-frame glitch happen despite my previous efforts to prevent it. So now
it's fixed.
This is because otherwise, on my Linux system at least, the game will
take up a lot of CPU that it doesn't really need to (I only have a 60hz
monitor).
On Windows, it looks like Windows already enforces V-sync for
applications anyway, unless they have exclusive fullscreen control.
Linux doesn't enforce V-sync on apps and lets them take up as much CPU
as they want, so I'm putting this here to limit the framerate.
The game is already actually 30 FPS anyway, the nice smooth FPS is just
visual. V-sync won't introduce any more "input lag" than already exists.
By "hidden names", I'm referring to "Dimension VVVVVV" and "The Ship"
popping up on the quit/pause/teleporter screens, even though those rooms
don't have any roomnames.
Apparently my commit to fix roomname re-draw bleed on the
quit/pause/teleporter screens exposed yet another hardreset()-caused
bug. The issue here is that since hardreset() sets game.roomx and
game.roomy to 0, map.area() will no longer work properly, and since the
hidden roomname check is based on map.area(), it will no longer display
"Dimension VVVVVV" or "The Ship" once you press ACTION to quit. It used
to do this due to the re-draw bleed, but now it doesn't.
I saw that roomnames didn't get reset in hardreset(), so the solution
here is to re-factor hidden names to be an actual variable, instead of
being implicit. map.hiddenname is a variable that's set in
mapclass::loadlevel(), and if isn't empty, it will be drawn on the
quit/pause/teleporter screens. That way it will still display "Dimension
VVVVVV" and "The Ship" when you press ACTION to quit to the menu.
EDIT: Since PR #230 got merged, this commit is no longer strictly
necessary, but it's still good to refactor hidden names like this.
As much as it looks cool to have a slowly-scrolling background on the
title screen, it's quite annoying that slowmode applying on the title
screen mean that your keypresses are less responsive.
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.
This is only noticeable if you have a font with translucent pixels, like
I do. But it gets really noticeable really quickly with this over-30-FPS
patch because the render functions are continuously called every
delta-timestep. To prevent this, just fill the backbuffer with black
before rendering anything.
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.
This was especially noticeable in slowmode, where after going to an
adjacent room, it would look like they stopped for a split second. This
commit makes it so they smoothly continue their journey after switching
rooms.
The integer cast is to round off any fractional part of the velocity so
that they don't make a difference and result in the oldxp/oldyp being
one pixel off. Especially since the player's y-velocity fluctuates while
standing unflipped on the floor.
Incidentally enough, this seems to only have been a problem with screen
transitions for some reason. No other uses of gotoroom() (such as the
one where gotoroom() is called every other frame, or every frame) seem
to have resulted in this "pausing" behavior, or at least a reversion
back to 30 FPS movement. I don't know why.
Otherwise it'll go by too quickly.
Also something subtle here - I didn't make it conditional on
game.advancetext, so now it'll still decrement even if you have
advancetext up.
This makes editor notes fade out smoothly. And even though the notedelay
only gets decremented by one every editor-frame (the editor runs at
1000/24 FPS fixed-timestep here), it actually gets multiplied by 4, so a
floating-point interpolated value would make a difference here.
This is because their oldxp wasn't being updated when they move (or
rather, teleport) and wrap around the screen.
These enemies are ZZT centipedes, but they're referred to as ASCII
snakes in comments in the code.
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.
Just a small thing, but if you teleported out of a tower with the
top/bottom screen spikes being onscreen (by dying, for example), they
would retract once you went back in to a tower. Small little thing, but
it's a good thing to polish.
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.
When you pressed and released ACTION to speed up the credits, the
credits position would end up being 1 frame off from the background.
This is due to the fact that we update the tower background after we
update the credits position, so this commit moves the tower background
update before the credits position update.
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.
By "frame" here I'm referring to the fixed-timestep, not a visual
delta-timestep.
Anyway, the problem is because crewmates' drawframes wait for
entityclass::animateentities() to be called in gamelogic(). In the
old, 30-FPS-only system, this entityclass::animateentities() would be
called in gamerender(), before any actual rendering would take place.
However, I've had to move it out of gamerender() because otherwise
entities would animate too fast. As a result, since gamerender() could
be called in between the entity creation and gamelogic(), a
less-than-1-frame visual glitch could happen.
The solution is to set the entity's drawframe as well when fixing facing
the player in Map.cpp.
Incidentally enough, this de-duplicates the amount of times this code
has been copy-pasted from 4 times to 2.
Anyways, this makes it so the crew don't go lightning fast on the
summary screen, the NDM game-over screen, the NDM win screen, and the
pause screen in the main game. It was slightly hilarious seeing them go
really quickly, actually. It was like they were running away from a
giant monster or something.
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().
To make it real smooth, just in case it was noticeable that it only
updated at 1000/34 FPS before (well, except in slowmode, it's really
noticeable THERE).
Also this removes the re-typing out of (game.act_fade/10.0f) for every
single R, G, and B in gamerender().
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.
This has to be done in order to fix rendering when on a conveyor or
moving platform and actively moving with or against it. Pretty sure this
shouldn't break anything, oldxp/oldyp is mostly visual after all (and by
the time it's used for gravity line collision checking,
updateentitylogic() would've already gotten around to it anyway).
Incidentally, this also fixes a jitter that would occur if you were
moving at the time you died or collected a trinket or custom crewmate,
due to the game temporarily freezing and either doing deathsequence or
completestop.
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.)
Since "if (graphics.resumegamemode)" and "if (menuoffset > 0)" both do
the same thing, they've been combined with an "or" conjunction.
As well, the map.extrarow check in maplogic() has been refactored to use
a variable instead of duplicating the entire code block. Not that it
matters anyway, because the difference between 240 and 230 is only 10
pixels, far short of the 25 pixel increment that bringing the menu up
and down uses, and both 240 and 230 integer-divided by 25 have the same
non-remainder value of 9.
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.
This fixes entities being drawframe 0 for 1 frame when being first
created. Incidentally, this also fixes entities created during
completestop being the player sprite, too, which is something not many
people notice.
For some reason, it was put near the start of gamerender(), even though
since it handles edge-flipping it seems like it should be in the logic
function already.
This makes sure entity animations don't animate as fast as possible, and
also fixes edge-flipping on normal surfaces.
This prevents undefined behavior because we use oldxp/oldyp to do linear
interpolation.
It's also initialized in entclass::entclass(), just to be sure. And I've
deduplicated the regular xp/yp initialization in createentity(), too.
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.
Ok, and this is where the fun starts.
In an ideal world, this would be the end of this patch. However, of
course, there are many, MANY places in the game that update
fixed-timestep timers DIRECTLY inside the render function, which is not
ideal because it means those timers go super fast.
I'll have to fix those later.
Ok, NOW indent it. I didn't indent it previously because the diffs are
annoying to read if there's an indent that doesn't otherwise change
anything (and even now it's pretty annoying to read).
Alright, this is the start of the over-30-FPS patch!
First things first, we'll need to make it possible to have a separate
deltatime loop outside of the fixed timestep loop. And for that, we
can't be using SDL_Delay(), as SDL_Delay() (as you might imagine) blocks
the whole program.
Instead we'll be using this thing called an accumulator. It looks at how
long the previous poll took (the raw deltatime), and lets timesteps pass
accordingly.
On a side note, I've had to split the `time` and `timePrev` declaration
each onto their own separate line, otherwise there's undefined behavior
from `time` not being initialized.
I use `accumulator = fmodf(...)` instead of `accumulator -=
timesteplimit` because otherwise it'll fast-forward if it's behind,
which is a jarring thing to see.
Also in preparation for what's going to come down the over-30-FPS road,
I've also added `deltatime` and `alpha`. `deltatime` is going to be used
if the game is in slowdown mode, and `alpha` is going to be used for
linear interpolation of animations.
By the way, what was the main game loop previously (and is now the new
timestep loop) is now in an extra set of curly braces, but I haven't
indented it yet to reduce the noise in this commit.
This prevents being able to "roll over" the amount of minutes to 0 (by
simply waiting for the timer to tick past one hour) and being able to
get a result of 00:13 when your result is really 01:00:13.
By looking only at the minutes, the game would read 01:00:13 as 00:13
instead. So simply add the amount of hours to the time trial result.
This is needed for MinGW when compiling C++98, apparently. I put it in
an if-guard because otherwise there'll be a warning from MY compiler
about redefinitions.
If you exited to the menu normally (i.e. got on a code path that went
through Game::quittomenu()), the menu music wouldn't play. This is
because FILESYSTEM_unmountassets() was put after music.play(6). So the
game would play the custom level's track 6, and then unmount it, which
meant it could no longer play track 6, but there's nothing telling the
game to play track 6 again. So I just changed the frame ordering around.
I also added a comment to make sure anyone reading the code is aware of
the frame order dependency.
If you exited from the editor, custom assets would not be unmounted. But
I made sure to put the FILESYSTEM_unmountassets() before the
music.play(6) because otherwise the menu music wouldn't play.
You could also exit to the menu from a custom level using the
rollcredits() command, so I made sure to put a
FILESYSTEM_unmountassets() when returning to the menu from the credits
as well. I also made sure to put it before the music.playef(18) so
there's no risk of the sound effect not playing properly, or not playing
the non-level-specific one.
I added a comment to both FILESYSTEM_unmountasset()s to make sure anyone
reading the code is aware of the frame order dependency.
This is really awful, but there's not much we can do.
TinyXML-2 no matter what will never stop on newlines, so without
changing the XML parser, this is the best we can do - just remove the
"\n " (that's a linefeed plus exactly 12 spaces) if it
appears at the end of the contents of an edentity tag.
Also a giant comment for good measure.
Looks like duplicate player entities persisting across rooms is a
semi-useful feature used by some levels. Still, though, it's a bit of a
nuisance to have duplicate player entities persisting across game
sessions. And levels can't rely on this persistence anyway, anyone could
just close the game and re-open it to get rid of the duplicate entities
regardless.
If a trinket or crewmate ID is out-of-bounds, it will not be created.
This is not only because the `collect`/`customcollect` check in
entityclass::createentity() would then be out-of-bounds, but also
touching it would also be out-of-bounds, too.
Display trinkets will always be created if the ID is out-of-bounds.
Apparently some people (@AllyTally) have been creating display trinkets
with IDs of -1 in order to get a display trinket that always shows up,
which is rather horrifying. But it makes sense, there's a lot more
nonzero int values than there are the amount of int values that are
zero, so it's fairly likely that the `collect` check will always come up
to be true (nonzero). Also, it's more useful to be able to have a
display trinket that always shows up without having to collect a trinket
beforehand, than it is to have it not be created (because technically by
default, you're already in the world where you don't have it created).
Display trinkets still have their `para` set to their ID, though, and if
they managed to gain an `onentity` of 1, bad things could happen... So
just to be sure, I added INBOUNDS checks to crewmates and trinkets in
entityclass::updateentities() so no UB will happen if you collect a
crewmate/trinket with an out-of-bounds ID. Also, I de-duplicated the
`collect`/`customcollect` setting, too.
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.
This prevents the game from being saved if you manage to trigger a
savetele() during a "special" gamemode (like if you use the Gravitron
out-of-bounds glitch when replaying Intermission 2, then go to Game
Complete that way).
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.
To deal with using a different image file for Flip Mode, it looks like
copy-paste was used. This isn't exactly maintainable code. So I'm
replacing it with a reference that changes depending on if the game is
in Flip Mode or not, instead.
Removing the player entity has all sorts of nasty effects, such as
softlocking the game because many inputs require there to be a player
present, such as opening the quit menu.
The most infamous glitch to remove the player entity is the Gravitron
Fling, where the game doesn't see a gravity line at a specific
y-position in the current room, and when it moves the bottom gravity
line it moves the player instead. When the gravity line gets outside the
room, it gets destroyed, so if the player gets dragged outside the room,
they get destroyed, too. (Don't misinterpret this as saying anytime the
player gets dragged outside the room, they get destroyed - it's only the
Gravitron logic that destroys them.)
Also, there are many places in the code that use entity-getting
functions that have a fallback value of 0. If it was possible to remove
the player, then it's possible for this fallback value of 0 to index
obj.entities out-of-bounds, which is not good.
To fix this, entityclass::removeentity() is now a bool that signifies if
the entity was successfully removed or not. If the entity given is the
player (meaning it first checks if it's rule 0, just so in 99% of cases
it'll short-circuit and won't do the next check, which is if
entityclass::getplayer() says the indice to be removed is the player),
then it'll refuse to remove the entity, and return false.
This is a change in behavior where callers might expect
entityclass::removeentity() to always succeed, so I changed the
removeentity_iter() macro to only decrement if removing the entity
succeeded. I also changed entityclass::updateentities() from
'removeentity(i); return true;' to 'return removeentity(i);'.
Apparently it results in Undefined Behavior if the argument given isn't
representable as an unsigned char. This means that (potentially) plain
char and signed chars could be unsafe to use as well.
It's rare that this could happen in practice, though. std::isdigit() is
only used by is_positive_num() which is only used by find_tag(), so
someone would have to deliberately put something crazy after an `&#` in
a custom level file in order for this to happen. Still, better to be
safe than sorry and all.
This fixes a compile error that could happen where the compiler doesn't
know what std::isdigit() is, but I'm puzzled as to why this wasn't
happening earlier, 'cause I've only been reported that it happens by
only one person.
Continuing from #280, another potential source of out-of-bounds indexing
(and thus, Undefined Behavior badness) comes from script commands. A
majority of them don't do any input validation at all, which means the
potential for out-of-bounds indexing and segfaulting in custom levels.
So it's always good to add bounds checks to them.
Interesting note, the only existing command that has bounds checks is
the flag() command. That means you can't turn out-of-bounds flags on or
off. But there's no bounds checks for ifflag(), or customifflag(), which
means you CAN index out-of-bounds with those commands! That's a bit bad
to do, so.
Also, I decided to add the bounds checks for playef() at the
musicclass::playef() level, instead of just the level of the playef()
command. I don't know of any other cases outside of the command where
musicclass::playef() will index out of bounds, but musicclass is the one
containing the indexed vector anyway, I wanted to cover more cases, and
it's better to be safe than sorry.
And this the function with the least amount of cases where its sentinel
value is used unchecked. Thankfully. obj.getplayer() was a bit of a slug
to get through.
obj.getplayer() can return -1, which can cause out-of-bounds indexing of
obj.entities, which is really bad. This was by far the most changes, as
obj.getplayer() is the most used entity-getting function that returns
-1, as well as the most-used function whose sentinel value goes
unchecked.
To deal with the usage of obj.getplayer() in mapclass::warpto(), I just
added general bounds checks inside that function instead of changing all
the callers.
A few months ago, I added ghosts to the VVVVVV: Community Edition editor. I was told recently I should think
about upstreaming it, and with Terry saying go ahead I finally ported them into VVVVVV. There's one slight
difference however--you can choose whether you have them or not in the editor's settings menu. They're off by
default, and this is saved to the save file.
Anyway, when you're playtesting, the game saves the players position, color, room coordinates and sprite every 3
frames. The max is 100, where if it tries to add more, the oldest one gets removed.
When you exit playtesting, the saved positions appear one at a time, and you can use the Z key to speed it up.
[Here's a video of them in action.](https://o.lol-sa.me/4H21zCv.mp4)
2.2 and earlier had this god-awful thing where it put the closing tag of
an edentity onto the next line, and then kept the indentation the same.
This requires parsing the XML in an extremely specific way (i.e.
ignoring the whitespace) so the newline and indentation isn't taken as
part of the actual contents of the tag.
2.3 removed this awful whitespace entirely to make it easier on parsers.
When I tested #270, I tested against a 2.3 re-save of Dimension Open and
diffed the two, because I thought testing against the original version
of the level would result in a bunch of noise I didn't want due to the
whitespace change. Well, I did exactly what I intended, and ended up
ignoring the whitespace change so much that levels saved in this stupid
format ended up getting broken.
Luckily, we can just tell TinyXML-2 to parse a document exactly like how
TinyXML-1 would've parsed it, by supplying the COLLAPSE_WHITESPACE enum
to it (by default it's on PRESERVE_WHITESPACE).
