As part of fixing #464, I'll need to move these pieces of code around
easily. In #220 I just kind of shoved them awkwardly in whatever
fixed function would be last called in the gamestate loop, which I
shouldn't have done as I've now had to make formal fixed-render
functions anyway. Because these fixed functions need to be called
directly before a render function, and I'm fixing the order to put
render functions in their proper place, so I need to be able to move
these around easily, and making them function calls instead of inlined
makes them easier to manipulate.
My previous custom level forwards compatibility would only work if you
saved to the same filename as you read from. But what if you saved to a
new filename? Well, your extra XML is lost.
Not to worry, I've introduced a variable that remembers the filepath of
the currently-loaded level (the existing `filename` attribute is kind of
weird and not really suited for this purpose, so). I tried to make it a
simple `const char*`, but it turns out that's bad when you take the
`c_str()` of an `std::string` that then gets destroyed, so it's best to
use `std::string` in an `std::string` world.
So now when you save a level, it'll attempt to open the original file,
and if that doesn't exist then your extra XML gets lost anyway, but I
shouldn't be expected to keep your XML if you delete the original file.
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.
After looking at pull request #446, I got a bit annoyed that we have TWO
variables, key.textentrymode and ed.textentry, that we rolled ourselves
to track the state of something SDL already provides us a function to
easily query: SDL_IsTextInputActive(). We don't need to have either of
these two variables, and we shouldn't.
So that's what I do in this patch. Both variables have been axed in
favor of using this function, and I just made a wrapper out of it, named
key.textentry().
For bonus points, this gets rid of the ugly NO_CUSTOM_LEVELS and
NO_EDITOR ifdef in main.cpp, since text entry is enabled when entering
the script list and disabled when exiting it. This makes the code there
easier to read, too.
Furthermore, apparently key.textentrymode was initialized to *true*
instead of false... for whatever reason. But that's gone now, too.
Now, you'd think there wouldn't be any downside to using
SDL_IsTextInputActive(). After all, it's a function that SDL itself
provides, right?
Wrong. For whatever reason, it seems like text input is active *from the
start of the program*, meaning that what would happen is I would go into
the editor, and find that I can't move around nor place tiles nor
anything else. Then I would press Esc, and then suddenly become able to
do those things I wanted to do before.
I have no idea why the above happens, but all I can do is to just insert
an SDL_StopTextInput() immediately after the SDL_Init() in main.cpp. Of
course, I have to surround it with an SDL_IsTextInputActive() check to
make sure I don't do anything extraneous by stopping input when it's
already stopped.
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).
It was never used and didn't do anything. It looks like it was intended
to be used but I guess time ran out or something, but it's too late now
and level files don't have timestamps or anything, so might as well just
remove it.
Good thing too, because asctime() is apparently deprecated.
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.
This removes around megabyte from the binary, so a stripped -Og binary
went from 4.0 megabytes to 2.9 megabytes, and an unstripped -O0 binary
went from 8.1 megabytes to 7.1 megabytes, which means I can now finally
upload an unstripped -O0 binary to Discord without having to give money
to Discord for their dumb Nitro thing or whatever.
They're always the same size, so there's no need for them to be vectors.
Also made the number of elements in ed.level/kludgewarpdir controllable
by maxwidth/maxheight.
I removed editorclass::saveconvertor() because I didn't want to convert
it to treat ed.contents like an array, because it's unused so I'd have
no way of testing it, plus it's also unused so it doesn't matter. Might
as well get rid of it.
map.contents always has 1200 tiles in it, there's no reason it should be
a vector.
This is a big commit because it requires changing all the level classes
to return a pointer to an array instead of returning a vector. Which
took a while for me to figure out, but eventually I did it. I tested to
make sure and there's no problems.
Again, basically no reason for it to exist on the class itself.
The usage of the variable was replaced with temp2 instead of temp
because there was already a temp variable in the function it was used
in.
Ved has this useful feature where instead of having to manually travel
to a room whose coordinates you know, you can just press G and type in
coordinates to go there.
VCE added this, but I changed the text to be "x,y" instead of "(x,y)"
because otherwise it could confuse someone into thinking they need to
type parentheses when in reality they don't need to and typing them will
just make it not work.
Also I made sure to add an error message if the user types in an invalid
format. Failing silently would just confuse people, and maybe they'll
start thinking the feature doesn't work or something like that. VCE
doesn't have this helpful error message.
Lastly, VCE has a bug where if you use the shortcut to go from one
horizontally/vertically warping room to another, the background of the
previous room will still be there and scroll off with the background of
the room you went to, instead of just having the new background only.
This is because they forgot a 'graphics.backgrounddrawn = false;'. But
don't worry, *I* didn't forget about it.
This is basically FIQ's patch from VCE, except he never upstreamed it
because he said something along the lines of it seeming to not fit the
purpose of upstream.
But anyway, it basically just de-duplicates all the text input and text
finishing handling code and cuts down on the large amount of copy-paste
in the editor functions. It makes things way more maintainable.
Interesting note, it seems like FIQ had the intent to refactor the text
input in editor settings (i.e. the level metadata details), but never
got around to it in VCE. Maybe we'll finish that job for him later.
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.
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.
Like actual entities, editor ghost colors were updating every render
frame instead of logic frame. So just like actual entities, I added a
realcol attribute to them that editorrender() uses instead, and added
code to update said realcol attribute in editorlogic(). That way the
colors don't go by too quickly (especially the death color).
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.
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.
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)
Some of the file was indented with two spaces and the rest indented with
tabs. It feels like two different people worked on the file, one more
than the other. Since most of it uses two spaces, I'll just replace the
tabs with two spaces.
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.
Previously, the editor would always say it saved or loaded a level,
even if it was not successful. For example, because a file to load does
not exist, a file to save has illegal characters in its name or the
name is too long to be stored. Now failure is reported. Also, when
quitting the editor and saving before quitting is unsuccessful, the
editor will abort quitting.
I have the feeling that none of the devs understood what extern did, and
they kind of just sprinkled it everywhere until things started working.
But like all other classes, it should just be one line in the class's
respective header file, and shouldn't be so messy.
When the game loads a room in a custom level, previously it would load
the tilemap of that room into ed.swapmap, and then mapclass::loadlevel()
would manually go through each element in ed.swapmap to set each tile in
`contents`. Why do that, when you can just return the vector from
editorclass::loadlevel() and set it directly? ed.swapmap is really
unnecessary.
Previously, it existed solely to count the number of trinkets and
crewmates when loading a level, because we were keeping track of the
amount of them manually, incrementing and decrementing every time a
trinket or crewmate was added or removed, but loading a new level
represented a case that could potentially not be an increment or
decrement.
However, since the amount tracking is now handled automatically, this
function now does nothing, and can be safely removed.
Same principle as removing the separate variable to track number of
collected trinkets. This means it's less error-prone as we're no longer
tracking number of trinkets separately.
In the function that counts the number of trinkets, I would've liked to
have used std::count_if(). However, the most optimal way would require
using a lambda, and lambdas are too new for the C++ standard we're
using. So I just bit the bullet and counted them manually.
Looks like this function was created because editorclass::load() takes
in a string by reference, not by value, and thus mutates it afterwards,
so if you passed a string in when you didn't want it to be mutated, bad
things would happen.
However, a better workaround for the above issue would simply to
duplicate the string and pass that string instead, thus the original
string wouldn't be affected.
This removes global arg passing from all functions on editorclass.
Callers have been updated correspondingly. Additionally, all 'dwgfx' has
been replaced with 'graphics' in editor.cpp.
This commit removes all global args from the parameters of each function
on the scriptclass object, and updates all places they are called
accordingly. It also changes all instances of 'dwgfx' to 'graphics' in
Script.cpp.
Interestingly enough, it looks like editor.h depended on Script.h's
class define of the musicclass. I've temporarily placed the class define
in editor.h, but by the end of this patchset it'll be gone.
This is a refactor that turns the script-related arrays `ed.sb`, and
`ed.hooklist` into C++ vectors (`script.commands` was already a vector, it was
just misused). The code handling these vectors now looks more like idiomatic
C++ than sloppily-pasted pseudo-ActionScript. This removes the variables
`script.scriptlength`, `ed.sblength`, and `ed.numhooks`, too.
This reduces the amount of code needed to e.g. simply remove something from
any of these vectors. Previously the code had to manually shift the rest of
the elements down one-by-one, and doing it manually is definitely error-prone
and tedious.
But now we can just use fancy functions like `std::vector::erase()` and
`std::remove()` to do it all in one line!
Don't worry, I checked and `std::remove()` is in the C++ standard since at least
1998.
This patch makes it so the `commands` vector gets cleared when
`scriptclass::load()` is ran. Previously, the `commands` vector never actually
properly got cleared, so there could potentially be glitches that rely on the
game indexing past the bounds set by `scriptlength` but still in-bounds in the
eyes of C++, and people could potentially rely on such an exploit...
However, I checked, and I'm pretty sure that no such glitch previously existed
at all, because the only times the vector gets indexed are when `scriptlength`
is either being incremented after starting from 0 (`add()`) or when it's
underneath a `position < scriptlength` conditional.
Furthermore, I'm unaware of anyone who has actually found or used such an
exploit, and I've been in the custom level community for 6 years.
So I think it's fine.
This turns the array 'edentity' into a proper vector, and removes the need to
use a separate length-tracking variable and manually keep track of the actual
amount of edentities in the level by using the long-winded
'EditorData::GetInstance().numedentities'. This manual tracking was more
error-prone and much less maintainable.
editorclass::naddedentity() has been removed due to now functionally being the
same as editorclass::addedentity() (there's no more
'EditorData::GetInstance().numedentities' to not increment) and for also being
unused in the first place.
editorclass::copyedentity() has been removed because it was only used to shift
the rest of the edentities up manually, but now that we let C++ do all the
hard work it's no longer necessary.
This makes the "[Press ENTER to return to editor]" fade out after a few frames, allowing screenshots of custom levels to be cleaner and to make sure nothing is obscured while the user is editing their level.
This commit also adds alpha support in BlitSurfaceColoured, where it takes into account the alpha of the pixel *and* the alpha of the color.
`graphics::getRGBA(r,g,b,a)` was added to help with this.
This fixes a bug where if warpdir() was used during in-editor
playtesting, the changed warp direction would persist even when leaving
playtesting.
This would be very annoying to correct back every time you playtested
and warpdir() was used, so I've added some kludge to store the actual
warp direction of each room when entering playtesting, and then set the
warp directions back when leaving playtesting.
First, two bug fixes. Room text input mode wasn't properly unset
upon pressing Esc, making the prompt get stuck, requiring you to
add roomtext again and finish it to make it go away. Secondly,
escaping script text input would remove the wrong entity.
I also tweaked the handling slightly so that instead of deleting
the entity if it already existed if escaping from text input,
it merely reverts the change in script name/roomtext to what it
was previously.
I considered refactoring the editor text input handler entirely,
but figured such a change would be a bit too extensive for the
purpose of this repository.
Misa
AllyTally
Fussmatte
Dav999-v
Matt Penny
Terry Cavanagh
Fredrik Ljungdahl