I ran the game through Valgrind to catch various issues.
One thing caught my attention -- map.resumedelay. This is
used by The Tower/etc to add a delay before the game is
resumed (probably for camera controls delaying it) for
spawning the player. This variable was uninitialized. Notably,
if I explicitly set it to 592851 or similar, it reproduces the
bug, even if I cannot reproduce it with my typical compilation
flags. So fixing this by initializing it to 0, alongside some
other Valgrind warnings, should fix the problem entirely.
* Add a null terminator to loaded TinyXML files
The TinyXML parse() function expect a C-like string, including terminator.
When xml loading was changed, it loaded the file, but included no such thing.
Thus, we load the file, then reallocate the memory so that we can insert a
null terminator to it, before passing it to parse().
* Tweak TinyXML file loading
Instead of first loading the file content into memory, then reallocate it
to add a null pointer, add an argument to the file load function for whether
to append a null terminator or not, defaulting to false. It still returns the
length without the null pointer in case a length ptr is passed.
An earlier change caused TinyXml to prettyprint specifically the
contents of entities (script names and roomtext) a bit more than
before in level files, and added an unusually high amount of whitespace
(particularly, it added an empty line to every entity). This happens
because, for some reason, an empty string is explicitly being added
when creating an entity XML element. The line is so mysterious it feels
like it probably somehow solved a nasty bug long ago and shouldn't be
touched, but it was probably just a mistake, and with all that
whitespace it doesn't look good.
Previously, if it was called in a custom level, it would say "out of
Twenty" no matter what, even if the level had zero trinkets. This commit
fixes that.
Previously, it was a hardcoded list going up to fifty.
This time, it's less hardcoded. Most of the time, it will take a number,
find out its tens-place word, find out its ones-place word, and combine
the two together. There are special cases for the teens, and the numbers
zero through nine, and one hundred.
Also, now if it is given a negative value, it will just return "???"
instead.
If there are more than one hundred it will still say "Lots".
2.2 will handle this just fine, because there are 100 slots allocated
for trinkets and crewmates, and it will save and load all 100 slots just
fine. Except, it only resets the first 20 slots when starting a level
from the beginning, but that's minor and already fixed in 2.3 anyway.
This commit makes it so you can now place up to one hundred trinkets and
crewmates in the editor.
When editorclass::reset() was resetting the contents of the level
previously, it was mixing up the X and Y bounds. The Y bound was
supposed to be 30*maxheight, and the X bound was supposed to be
40*maxwidth. Instead, it took 30*maxwidth as its Y bound and
40*maxheight as its X bound.
Then, when it actually indexes the contents vector to set each tile to
0, it used 30*maxwidth instead of 40*maxwidth.
The difference between width and height is a bit hard to spot, but one
thing you can do to remember the difference is to remember the fact that
X corresponds with width, and Y corresponds with height. Also, rooms are
40 by 30 tiles, and so X (and therefore width) should correspond with
40, and Y (and therefore height) should correspond with 30.
As a result of mixing up the variables, whenever you played a 20x20 map,
quit the level and then started making a new 20x20 map, the tiles of the
last four rows of the previous map would persist, from y=16 (1-indexed)
all the way to y=20 (1-indexed).
I don't recall anyone ever running into this bug before, which is a bit
strange. But if no one truly has ever ran into this bug before, then I'm
genuinely surprised.
While working on the patch to fix the enemy type room property of each
room not getting reset, and testing the fix, I noticed that for some
reason some contents of the previous level I played in order to test the
enemy type property persisting was ALSO persisting alongside the enemy
type property.
Then I read the code and when I realized that the X and Y bounds were
getting mixed up I groaned. Very loudly.
This fixes a bug where if you loaded a level, then started making a new
level in the editor, the enemy types from the previous level would
persist.
While working on VVVVVV: Community Edition and adding a new room
property for enemy speed, I noticed that enemy type was not getting
reset at all. After some testing, I confirmed that this was the case. So
this bug is fixed now.
This fixes a bug where you could get mismatched text and background
colors on the menu screen by being in the Tower and exiting at a certain
time. The background when mismatched will always be red, which seems to
have something to do with the fact that when you enter the Tower the
game always sets the background to be red. I could get a quick repro
setup going by starting in the checkpoint in Teleporter Divot, then
quickly entering the Tower, exiting, then re-entering, then pressing Esc
and quitting - all done very quickly.
The important thing about this mismatch is that it's only temporary, and
will be corrected when you press ACTION and the color of the text and
background in the menu both change at the same time, which is what
map.nexttowercolour() does. So all I do is simply call that function
when exiting to the menu, and it will fix the color mismatch.
Earlier in 53950e14de65a54d9369c5183a16337782d3dc4e, I made playing a
song while a song was already fading quickly fade out the current song,
but then added an exception for if the fade came from the musicfadeout()
command. This commit adds another exception for if the fade came from
niceplay(), since otherwise the music transitions between areas in the
game would go too quick.
This fixes an issue where you could softlock the game if you exited
playtesting mode by pressing Esc while a "- Press ACTION to advance text
-" prompt was onscreen, then re-entered playtesting and then started any
script.
The "- Press ACTION to advance text -" prompt is most directly
controlled by game.advancetext, but when it appears game.pausescript is
usually set as well. You need to have both variables on at the same time
in order to press ACTION. If only advancetext is on, then you won't be
able to flip but can still move around, but if only pausescript is on,
then the game softlocks because the only way you can turn pausescript
off is by pressing ACTION, but the game will only let you press ACTION
if *both* advancetext and pausescript are on.
This fixes a bug where bringing up the pause screen while in a tower
would draw the room you entered BEFORE the tower during the time it took
for the pause screen to be brought up or down.
This fixes a bug in the editor where if you had cutscene bars active
while exiting playtesting, when you re-entered playtesting, the bars
would do their closing animation even though they should be already
gone.
Out-of-bounds array access is Undefined Behavior, which means Bad
Things.
In this particular case, it was indexing an array by using the
`testeditor` variable. Which is fine, except it was indexing that array
*in a conditional that only happens if `testeditor` is -1*. So it was
indexing an array at position -1, which is Out of Bounds and is Not
Good.
For a long time, VVVVVV has had an issue where if you used custom
graphics with translucent pixels (i.e. pixels whose opacity was neither
0% nor 100%, but somewhere between 0% and 100%), those pixels would end
up looking very ugly. They seemed to be overlaid on top of some weird
blue color, instead of actually being translucent.
This bug only occurred when in-game and not in towermode. So, most of
the time, basically. The translucent pixels only displayed properly
inside the Tower, both minitowers, and the editor (when not
playtesting).
I traced this down to the use of the magic number 0xDEADBEEF in
Graphics::drawmap() and Graphics::drawfinalmap(). Looks like someone had
fun finding a color. Was it you, simoroth? Anyway, I've changed it so it
simply uses 0 instead. Note that this magic number needs to be changed
for BOTH FillRect() and OverlaySurfaceKeyed(), or else the game's
background will be some random solid color instead of actually being
drawn properly.
There's a long-standing issue where the Direct Mode status of the loaded
custom map in memory never resets properly. So if you load a level with
a certain layout of Direct Mode rooms, that same layout will be
preserved if you start making a new level in the editor. This commit
fixes that issue.
Similar to 2ebccbc3e9, there's also a
long-standing bug where if you backspace an empty line (and this time,
the line IS actually already empty, not merely
emptied-earlier-in-the-frame), the game will quickly delete more blank
lines if there are any above the blank line you deleted. Again, this is
annoying too, if you so happen to need to use lots of blank lines.
To fix this, it's simple - just set ed.keydelay to 6 when the game
backspaces an empty line. Then it won't be so trigger-happy in deleting
blank lines.
There is a long-standing bug with the script editor where if you delete
the last character of a line, it IMMEDIATELY deletes the line you're on,
and then moves your cursor back to the previous line. This is annoying,
to say the least.
The reason for this is that, in the sequence of events that happens in
one frame (known as frame ordering), the code that backspaces one
character from the line when you press Backspace is ran BEFORE the code
to remove an empty line if you backspace it is ran. The former is
located in key.Poll(), and the latter is located in editorinput().
Thus, when you press Backspace, the game first runs key.Poll(), sees
that you've pressed Backspace, and dutifully removes the last character
from a line. The line is now empty. Then, when the game gets around to
the "Are you pressing Backspace on an empty line?" check in
editorinput(), it thinks that you're pressing Backspace on an empty
line, and then does the usual line-removing stuff.
And actually, when it does the check in editorinput(), it ACTUALLY asks
"Are you pressing Backspace on THIS frame and was the line empty LAST
frame?" because it's checking against its own copy of the input buffer,
before copying the input buffer to its own local copy. So the problem
only happens if you press and hold Backspace for more than 1 frame.
It's a small consolation prize for this annoyance, getting to
tap-tap-tap Backspace in the hopes that you only press it for 1 frame,
while in the middle of something more important to do like, oh I don't
know, writing a script.
So there are two potential solutions here:
(1) Just change the frame ordering around.
This is risky to say the least, because I'm not sure what behavior
depends on exactly which frame order. It's not like it's key.Poll()
and then IMMEDIATELY afterwards editorinput() is run, it's more
like key.Poll(), some things that obviously depend on key.Poll()
running before them, and THEN editorinput(). Also, editorinput() is
only one possible thing that could be ran afterwards, on the next
frame we could be running something else entirely instead.
(2) Add a kludge variable to signal when the line is ALREADY empty so
the game doesn't re-check the already-empty line and conclude that
you're already immediately backspacing an empty line.
I went with (2) for this commit, and I've added the kludge variable
key.linealreadyemptykludge.
However, that by itself isn't enough to fix it. It only adds about a
frame or so of delay before the game goes right back to saying "Oh,
you're ALREADY somehow pressing backspace again? I'll just delete this
line real quick" and the behavior is basically the same as before,
except now you have to hit Backspace for TWO frames or less instead of
one in order to not have it happen.
What we need is to have a delay set as well, when the game deletes the
last line of a char. So I set ed.keydelay to 6 as well if editorinput()
sses that key.linealreadyemptykludge is on.
For a long time, the script editor has had a bug where it would let you
put the cursor on a nonexistent script line, which would APPEAR to be
the last line of the script... but in reality, it WASN'T the last line
of the script, and in fact, the ACTUAL last line was the line ABOVE the
script.
So, if you typed anything on this nonexistent line, it would appear to
get erased when exiting the script. Thus, people have (erroneously)
misdiagnosed this as the script editor somehow being trigger-happy and
erasing lines when it shouldn't be, when in reality it should've have
let you gone onto that line in the first place!
Hiding the mouse cursor does makes sense in a lot of situations I agree, but it's very much a preference thing, and I don't see a good reason to change the original behavour. Some people (i.e. me) don't like cursors disappearing in windowed mode when you move the cursor over the screen, but most importantly, it makes the editor much less pleasant to use (since you're relying on the 30fps editor cursor box).
I'd be happy to support, say, a setting that allowed you to disable the mouse cursor, or even a 15 second time-out which makes the cursor disappear if not moved (and reappear once moved), but just having it off by default feels wrong to me.
Yikes. Somebody brought this to my attention, I didn't even remember that I'd written it. "Spa" or "Spastic" is kind of a south park esque slang term that used to be pretty common in Ireland, which I used without even thinking about it. It's definitely not something I would say anymore, 10 years on, and it's something I shouldn't have said at the time either. I'm sorry :(
(somebody on twitter was asking me about how much cleaning up of the source code I did before launching this. I think this commit kinda answers that)
* Add entity type attribute checks to getcrewman()
This means that the game is no longer able to target other non-crewmate
entities when it looks for a crewmate.
This has actually happened in practice. A command like
position(cyan,above) could position the text box above a horizontal warp
line instead of the intended crewmate.
This is because getcrewman() previously only checked the rule attributes
of entities, and if their rule happened to be 6 or 7. Usually this
corresponds with crewmates being unflipped or flipped, respectively.
But warp lines' rules overlap with rules 6 and 7. If a warp line is
vertical, its rule is 5, and if it is horizontal, its rule is 7.
Now, usually, this wouldn't be an issue, but getcrewman() does a color
check as well. And for cyan, that is color 0. However, if an entity
doesn't use a color, it just defaults to color 0. So, getcrewman() found
an entity with a rule of 7 and a color of 0. This must mean it's a cyan
crewmate! But, well, it's actually just a horizontal warp line.
This commit prevents the above from happening.
If you're in the room being targeted when a `warpdir()` command is run,
and it sets the warp direction to none (warp dir 0), and if you're in a
Lab or Warp Zone room, the background will be set to the wrong one,
because it will always set the background to the stars-going-left
background, instead of setting it to the Lab background or the
stars-going-up background.
However, this is only a visual glitch, and is a temporary one, because
if you manage to trigger a `gotoroom()` on the room, it will get set to
its proper background.
This commit makes it so if you're in a Lab room, the background gets set
to the Lab background, and if you're in a Warp Zone room, the background
gets set to the stars-going-up background.
There are three map-related vectors that need to be reset in hardreset:
`map.roomdeaths`, `map.roomdeathsfinal`, and `map.explored`. All three
of these vectors use the concatenated rows system, whereby each room is
given a room number, calculated by doing X + Y*20, and this becomes
their index in each vector.
There's a double-nested for-loop to handle resetting all of these, where
the outer for-loop iterates over the Y-axis and the inner for-loop
iterates over the X-axis, but only `map.explored` is properly reset.
That's because it's the only vector that properly indexes using X +
Y*20. The other vectors reset using X, so previously, only the first row
of `map.roomdeaths` and `map.roomdeathsfinal` got reset, corresponding
with only the first row of rooms in both Dimension VVVVVV and Outside
Dimension VVVVVV getting reset.
This commit makes sure that both get reset properly.
There are two main problems with flipgravity():
1. It doesn't work for an already-flipped crewmate.
2. It doesn't work on the player.
This commit addresses both of those issues.
For reference, here is the list of scores for a custom level:
0 - not played
1 - finished
2 - all trinkets
3 - finished, all trinkets
Previously, you could've played a level and finished it and set a score
of 3. Then you could re-play the entire level, but manage to only get a
score of 1, i.e. only finished but not with all trinkets. Then your
score for that level would get set back to 1, which basically nullifies
the time you spent playing that level and getting all of the trinkets.
This commit makes it so your new score will get set only if it is higher
than the previous one.
This disables being able to press M to mute while editing a script in
the script editor. It also disables it in the script list, too, but I'm
hoping no one thinks that's an issue, because I personally don't think
it is.
This fixes a bug where if you died after activating a teleporter prompt
and then respawned in the same room as the teleporter, the teleporter
prompt would go away. Which would be very annoying if you wanted to, you
know, teleport.
You don't even have to R-press to make this happen. "Level Complete!"
and Energize are two teleporter rooms in the main game that have hazards
in them.
I see no reason to set game.activetele to false when dying. For one, it
gets set to false during a gotorom anyway. For another, I couldn't get
the game to activate the teleport prompt in a room without a teleporter
in my testing, mostly because when you touch a teleporter, your
checkpoint is set to that teleporter, so you can't R-press to go to
another room and carry over the teleport prompt.
This uncomments the code for the font outline. Now, you will see the
font outline on text that had it in the Flash version, most notably "-
Press ENTER to Teleport -" and the time trial overlay text.
This also fixes a problem with the commented code, where un-centered
text didn't outline properly.
The game makes sure that the player entity is never destroyed, but in
doing so, it doesn't destroy any duplicate player entities that might
have been created via strange means e.g. a custom level doing a
createentity with t=0.
Duplicate player entities are, in a sense, not the "real" player entity.
For one, they can take damage and die, but when they do they'll still be
stuck inside the hazard, which can result in a softlock. For another,
their position isn't updated when going between rooms. It's better to
just destroy them when we can.
This is similar to what was reverted in 3011911 but this time should use
syntax available even before C++11, tested on Clang with
`-std=c++98 -Wall -pedantic`.
The TinyXml functions to load and save files don't properly support
unicode file paths on Windows, so in order to support that properly, I
saw no other option than to do the actual loading and saving via PHYSFS
(or to use the Windows API on Windows and retain doc.LoadFile and
doc.SaveFile on other OSes, but that'd be more complicated and
unnecessary, we already have PHYSFS, right?).
There are two new functions in FileSystemUtils:
bool FILESYSTEM_saveTiXmlDocument(const char *name, TiXmlDocument *doc)
bool FILESYSTEM_loadTiXmlDocument(const char *name, TiXmlDocument *doc)
Any instances of doc.SaveFile(<FULL_PATH>) have been replaced by
FILESYSTEM_saveTiXmlDocument(<VVVVVV_FOLDER_PATH>, &doc), where
<FULL_PATH> included the full path to the saves or levels directory,
and <VVVVVV_FOLDER_PATH> only includes the path relative to the VVVVVV
directory.
When loading a document, a TiXmlDocument used to be created with a full
path in its constructor and doc.LoadFile() would then be called, now a
TiXmlDocument is constructed with no path name and
FILESYSTEM_loadTiXmlDocument(<VVVVVV_FOLDER_PATH>, &doc) is called.
There's now a thin layer of UTF-16 around the WinAPI functions to get
the path to the Documents folder and to create a new directory, so that
account usernames with non-ASCII characters do not result in no VVVVVV
folder being created or used.
Resizing the vector does the same thing that the loops did, it changes
the size for the vector and initializes it with default-constructed
elements (or 0 or its equivalent for POD types).
Where a specific value is needed, it is set with the second
parameter of resize().
The next official VVVVVV build removes 32-bit Linux (like all my other games),
and I need to get rid of the shell script on macOS at some point, so this
basically brings it up to what my other games are doing. Plus, this probably
fixes a bug where someone tries to run their executable away from the root...
- Return in Unlock Play Modes goes to Game Options instead of the main menu
- changing some Graphics Options settings no longer plays two menu select sounds, which was really loud :(
- Return in Game Pad Options plays the menu select sound
- turning off Screen Effects plays the menu select sound
Follow-up to #19 which did the change for macOS.
It appears to work as expected on Linux too.
Tested on a distro where XDG_DATA_HOME is undefined by default,
and `PHYSFS_getPrefDir` also resolves to `~/.local/share/VVVVVV/`.
The first organization parameter is unused on Linux and macOS.
Instead use PHYSFS_getPrefDir which does the same than the manual
concatenation done before. The organization name argument is required
but is not used on macOS.
