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.
