Previously, if you had backgrounds disabled in accessibility options,
and went to the editor and opened up the editor menu, it would be drawn
straight on top of what was already there in the editor instead of being
drawn on top of black. So now it's drawn on top of black.
During testing, I made a cursed level that set the flash timer to
precisely 1,000,000 frames. It turns out that if I activated the timer
in playtesting, exited playtesting, and exited the editor without ever
re-entering playtesting, the timer still kept going. So to prevent being
able to do that, we should hardreset() when exiting the editor.
In-game because that's where screen effects are used the most. But on
the title screen, screen effects are used when you press ACTION to start
the game, and when you enable screen effects, too.
Otherwise, we don't need screen effects for any other game-gamestate.
This de-duplicates the screen effects rendering code by putting it
inside a function, Graphics::renderwithscreeneffects(), and using that
instead of copy-pasted code.
The code to decrement the timers for flashing and shaking is now handled
outside the game-gamestate case-switch, instead of having to be
duplicated inside each render function.
As a bonus, I made it so the timer decrements even if screen effects are
disabled. This is to prevent any theoretical situation where the timer
can "pile up" due to disabled screen effects not letting it tick down.
This fixes being able to rack up a large amount of stack frames by
pressing Esc repeatedly in the editor, which would be a problem if you
were to then return to the main menu afterwards.
Instead, if Menu::ed_settings is already in the stack, the game will
simply return to that menu instead of creating it. Else, it will just
create the menu.
Also, as extra attention to detail, I made sure that the menu create or
return only happens if Esc opens the settings menu, and not when Esc is
closes it.
This stabilizes the code that handles the menu that you land on if you
press Esc and quit to the menu.
Instead of using Game::returnmenu(), we now use the new function
Game::returntomenu() to clearly express intent that we want to return to
a specific menu. So I've added another kludge variable
Game::wasinintermission for the was-in-intermission case.
Also, I made it so that if you didn't have a main game telesave or
quicksave, you just get brought back to the main menu. Because you
shouldn't be able to go to the play menu without a quicksave or
telesave.
To find each individual tag quickly, to optimize levels list loading.
I opted to not read the tags <Created>, <Modified>, and <Modifiers> as
they're actually pretty useless.
Also I've added a tag finder for <MetaData> but it's not meant to be
used directly, it's only used to check that the tag exists.
This also replaces some createmenu()s with returnmenu()s as needed even
when said createmenu()s already didn't go to the main menu.
Now when you exit the level editor, you'll be selecting the "level
editor" option in "play levels", and if you exit from a level you'll
still be selecting that level in the levels list.
Furthermore, regardless of what you're exiting, your cursor position
will be remembered.
Much more stylistic, you don't need to repeat "game.currentmenuname" for
each case, and you don't need to deal with the dangling first "if" that
doesn't have an "else".
Stringly-typed things are bad, because if you make a typo when typing
out a string, it's not caught at compile-time. And in the case of this
menu system, you'd have to do an excessive amount of testing to uncover
any bugs caused by a typo. Why do that when you can just use an enum and
catch compile-time errors instead?
Also, you can't use switch-case statements on stringly-typed variables.
So every menu name is now in the enum Menu::MenuName, but you can simply
refer to a menu name by just prefixing it with Menu::.
Unfortunately, I've had to change the "continue" menu name to be
"continuemenu", because "continue" is a keyword in C and C++. Also, it
looks like "timetrialcomplete4" is an unused menu name, even though it
was referenced in Render.cpp.
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().
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.
It is an exact duplicate of musicclass::haltdasmusik(), so use that
function instead and update callers. Looks like
musicclass::haltdasmusik() came first, anyway (musicclass::stopmusic()
was only used in editor.cpp).
The problem is that it would index out-of-bounds if you did this, but
this UB hasn't caused an exception until my change to refactor
script-related vectors by removing their separate length-trackers.
Just a miscellaneous code cleanup.
There's no glitches that take advantage of the previous situation,
namely that 'temp' was a global variable in Logic.cpp and editor.cpp.
Even if there were, it seems like it would easily lead to some undefined
behavior. So it's good to clean this up.
It looks like this may have been used earlier in development, judging
from the name, obviously, but right now it seems like it's used as an
error message if a main game level is asked for an invalid room (well,
only two of them - the Lab and Warp Zone). It should probably be
formalized into an error system, if we want to keep teststring, and also
people would never see it anyway because I don't think there's a
reliable and consistent way to trigger loading a non-existent room.
I have seen someone manage to load a non-existent Warp Zone room only
one time, but even then this teststring didn't pop up. So this
teststring doesn't even trigger in the right circumstances.
Also, when it does pop up, as far as I can tell it will stay onscreen,
which is kinda annoying. So I'm just removing this ancient relic from
the code.
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 changes something like UtilityClass::String to help.String,
basically. It takes less typing this way, and is a neat effect of having
global args actually be global variables.
The 'r', 'g', and 'b' arguments do absolutely nothing. Except unlike
Graphics::drawtile(), there's only one version of Graphics::drawtile2(),
so just remove those args and update callers.
The 'r', 'g', and 'b' arguments do absolutely nothing, even though
they're used in the version of Graphics::drawtile() that's more used. So
delete the other version without those extra arguments, and then remove
the extra arguments from the remaining version. And then update callers.
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 removes global args from all functions on the Graphics class.
Callers of those functions in other files have been updated accordingly.
Of course, since Graphics.cpp is already in the Graphics namespace,
I do not need to change all 'dwgfx' to 'graphics' in Graphics.cpp.
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.
Text outline is not drawn on roomtext when you're actually playing the
game, so don't draw the outline in the editor, either.
FIQ mistakenly added text outline to roomtext in
ca9f577fc4.
There's an if-else chain that first deals with figuring out if there's
an entity where your left-click happened, and to do this it uses
edentat(), which returns a sentinel value of -1 if there is NOT an
entity where your cursor is.
It's very important to check that the value returned ISN'T -1 before you
start indexing the 'edentity' vector, since if you DO index it with that
-1, it'll result in Undefined Behavior because you're doing an
out-of-bounds array access.
Now, here's what the if-else chain looked like before:
if(tmp==-1 && ed.free(ed.tilex,ed.tiley)==0)
{
...
}
else if(edentity[tmp].t==1)
The bug here is very subtle but it was an easy oversight. Basically, if
'ed.free' ended up not being zero, control flow would jump to the next
"else if" over, which then ends up asking for the -1th index of
'edentity', which is Undefined Behavior.
This undefined behavior has now resulted in a crash on my system after
TerryCavanagh/VVVVVV#172, due it shuffling things around juuuuust enough
such that this UB would end up resulting in a segfault instead of
chugging along and working fine. For me and my system, this meant that
if my first left-click in the editor upon opening the game was me
placing down a tile and not placing down an entity, the game would
crash. But, it would be fine if I first placed down an entity and then
afterwards placed down tiles, because it's UB.
And I'm almost certain this was the cause of the very strange bug where
you couldn't hold down left-click for the foreground-placing tool (but
you COULD for the background-placing tool) that seemed to occur most
often on Windows (TerryCavanagh/VVVVVV#25).
The solution to this is to stick in another conditional in the tree
before any indexing occurs, such that there's no way any other
conditionals with the indexing in the conditional tree could end up
being hit. In summary, the if-else chain looks like this now:
if(tmp==-1 && ed.free(ed.tilex,ed.tiley)==0)
{
...
}
else if(tmp == -1)
{
//Important! Do nothing, or else Undefined Behavior will happen
}
else if(edentity[tmp].t==1)
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 fixes another way you could end up typing on a non-existent line in
the script editor.
In a script with only 1 line, which is empty, the game would let you
press backspace on it, removing the line. This results in you typing on
a non-existent line.
You will keep typing on it until you either close the script or press
Up. If you press Up, you will be unable to get back to the non-existent
line, for it doesn't exist - but the text you typed on the non-existent
line will still be there, until you close the script and re-open it.
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.
In `editor.cpp`, there's a few sections of code that try and index stuff using `string.length()-1`.
This causes issues where if the string is empty, the result is -1, causing undefined behavior.
Flibit fixed a few of these cases, like on line `375` of editor.cpp:
`if((int) tstring.length() - 1 >= 0) // FIXME: This is sketchy. -flibit`
It turns out that one of these weren't caught, over at line `471`.
`tstring=tstring[tstring.length()-1];`
This causes builds compiled on Windows to segfault if you load more than one level in the editor.
I added a quick `if` around it, setting `tstring` to an empty string, which seems to fix the problem.
This uses utfcpp combined with a custom font, in the form of a PNG and text file. By default, the game acts exactly as it did before; custom fonts can be provided by third parties.
