Turn (super)patrons/githubfriends into arrays & move them to new file
So, originally, I wanted to keep them on Game, but it turns out that if
I initialize it in Game.cpp, the compiler will complain that other files
won't know what's actually inside the array. To do that, I'd have to
initialize it in Game.h. But I don't want to initialize it in Game.h
because that'd mean recompiling a lot of unnecessary files whenever
someone gets added to the credits.
So, I moved all the patrons, superpatrons, and GitHub contributors to a
new file, Credits.h, which only contains the list (and the credits max
position calculation). That way, whenever someone gets added, only the
minimal amount of files need to be recompiled.
2020-07-03 12:13:15 +02:00
|
|
|
#include "Credits.h"
|
2020-07-19 21:05:41 +02:00
|
|
|
#include "Entity.h"
|
|
|
|
|
#include "Enums.h"
|
2020-07-19 21:43:29 +02:00
|
|
|
#include "FileSystemUtils.h"
|
2020-07-19 21:05:41 +02:00
|
|
|
#include "Game.h"
|
Split glitchrunner mode into multiple versions
Previously, turning glitchrunner mode on essentially locked you to
emulating 2.0, and turning it off just meant normal 2.3 behavior. But
what if you wanted 2.2 behavior instead? Well, that's what I had to ask
when a TAS of mine would desync in 2.3 because of the two-frame delay
fix (glitchrunner off), but would also desync because of 2.0 warp lines
(glitchrunner on).
What I've done is made it so there are three states to glitchrunner mode
now: 2.0 (previously just the "on" state), 2.2 (previously a state you
couldn't use), and "off". Furthermore, I made it an enum, so in case
future versions of the game patch out more glitches, we can add them to
the enum (and the only other thing we have to update is a lookup table
in GlitchrunnerMode.c). Also, 2.2 glitches exist in 2.0, so you'll want
to use GlitchrunnerMode_less_than_or_equal() to check glitchrunner
version.
2021-08-05 02:09:49 +02:00
|
|
|
#include "GlitchrunnerMode.h"
|
2020-07-19 21:05:41 +02:00
|
|
|
#include "Graphics.h"
|
|
|
|
|
#include "Map.h"
|
|
|
|
|
#include "Music.h"
|
2020-07-19 21:43:29 +02:00
|
|
|
#include "Network.h"
|
|
|
|
|
#include "Script.h"
|
2020-07-19 21:05:41 +02:00
|
|
|
#include "UtilityClass.h"
|
|
|
|
|
|
Explicitly declare void for all void parameter functions (#628)
Apparently in C, if you have `void test();`, it's completely okay to do
`test(2);`. The function will take in the argument, but just discard it
and throw it away. It's like a trash can, and a rude one at that. If you
declare it like `void test(void);`, this is prevented.
This is not a problem in C++ - doing `void test();` and `test(2);` is
guaranteed to result in a compile error (this also means that right now,
at least in all `.cpp` files, nobody is ever calling a void parameter
function with arguments and having their arguments be thrown away).
However, we may not be using C++ in the future, so I just want to lay
down the precedent that if a function takes in no arguments, you must
explicitly declare it as such.
I would've added `-Wstrict-prototypes`, but it produces an annoying
warning message saying it doesn't work in C++ mode if you're compiling
in C++ mode. So it can be added later.
2021-02-25 23:23:59 +01:00
|
|
|
void titlelogic(void)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
//Misc
|
2020-11-03 00:23:53 +01:00
|
|
|
//map.updatetowerglow(graphics.titlebg);
|
2020-01-01 21:29:24 +01:00
|
|
|
help.updateglow();
|
|
|
|
|
|
2020-11-03 00:23:53 +01:00
|
|
|
graphics.titlebg.bypos -= 2;
|
|
|
|
|
graphics.titlebg.bscroll = -2;
|
2020-01-01 21:29:24 +01:00
|
|
|
|
|
|
|
|
if (game.menucountdown > 0)
|
|
|
|
|
{
|
|
|
|
|
game.menucountdown--;
|
|
|
|
|
if (game.menucountdown == 0)
|
|
|
|
|
{
|
2020-04-16 06:53:36 +02:00
|
|
|
if (game.menudest == Menu::mainmenu)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
music.play(6);
|
|
|
|
|
}
|
2020-04-16 06:53:36 +02:00
|
|
|
else if (game.menudest == Menu::gameover2)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-02 01:36:35 +02:00
|
|
|
music.playef(11);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2020-04-16 06:53:36 +02:00
|
|
|
else if (game.menudest == Menu::timetrialcomplete3)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-02 01:36:35 +02:00
|
|
|
music.playef(3);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2020-04-17 06:25:30 +02:00
|
|
|
game.createmenu(game.menudest, true);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
Explicitly declare void for all void parameter functions (#628)
Apparently in C, if you have `void test();`, it's completely okay to do
`test(2);`. The function will take in the argument, but just discard it
and throw it away. It's like a trash can, and a rude one at that. If you
declare it like `void test(void);`, this is prevented.
This is not a problem in C++ - doing `void test();` and `test(2);` is
guaranteed to result in a compile error (this also means that right now,
at least in all `.cpp` files, nobody is ever calling a void parameter
function with arguments and having their arguments be thrown away).
However, we may not be using C++ in the future, so I just want to lay
down the precedent that if a function takes in no arguments, you must
explicitly declare it as such.
I would've added `-Wstrict-prototypes`, but it produces an annoying
warning message saying it doesn't work in C++ mode if you're compiling
in C++ mode. So it can be added later.
2021-02-25 23:23:59 +01:00
|
|
|
void maplogic(void)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
//Misc
|
|
|
|
|
help.updateglow();
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
Explicitly declare void for all void parameter functions (#628)
Apparently in C, if you have `void test();`, it's completely okay to do
`test(2);`. The function will take in the argument, but just discard it
and throw it away. It's like a trash can, and a rude one at that. If you
declare it like `void test(void);`, this is prevented.
This is not a problem in C++ - doing `void test();` and `test(2);` is
guaranteed to result in a compile error (this also means that right now,
at least in all `.cpp` files, nobody is ever calling a void parameter
function with arguments and having their arguments be thrown away).
However, we may not be using C++ in the future, so I just want to lay
down the precedent that if a function takes in no arguments, you must
explicitly declare it as such.
I would've added `-Wstrict-prototypes`, but it produces an annoying
warning message saying it doesn't work in C++ mode if you're compiling
in C++ mode. So it can be added later.
2021-02-25 23:23:59 +01:00
|
|
|
void gamecompletelogic(void)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
//Misc
|
2020-11-03 00:23:53 +01:00
|
|
|
map.updatetowerglow(graphics.titlebg);
|
2020-01-01 21:29:24 +01:00
|
|
|
help.updateglow();
|
2020-04-01 23:48:11 +02:00
|
|
|
graphics.crewframe = 0;
|
2020-11-03 00:23:53 +01:00
|
|
|
graphics.titlebg.scrolldir = 1;
|
2020-01-01 21:29:24 +01:00
|
|
|
|
|
|
|
|
game.creditposition--;
|
Turn (super)patrons/githubfriends into arrays & move them to new file
So, originally, I wanted to keep them on Game, but it turns out that if
I initialize it in Game.cpp, the compiler will complain that other files
won't know what's actually inside the array. To do that, I'd have to
initialize it in Game.h. But I don't want to initialize it in Game.h
because that'd mean recompiling a lot of unnecessary files whenever
someone gets added to the credits.
So, I moved all the patrons, superpatrons, and GitHub contributors to a
new file, Credits.h, which only contains the list (and the credits max
position calculation). That way, whenever someone gets added, only the
minimal amount of files need to be recompiled.
2020-07-03 12:13:15 +02:00
|
|
|
if (game.creditposition <= -Credits::creditmaxposition)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
Turn (super)patrons/githubfriends into arrays & move them to new file
So, originally, I wanted to keep them on Game, but it turns out that if
I initialize it in Game.cpp, the compiler will complain that other files
won't know what's actually inside the array. To do that, I'd have to
initialize it in Game.h. But I don't want to initialize it in Game.h
because that'd mean recompiling a lot of unnecessary files whenever
someone gets added to the credits.
So, I moved all the patrons, superpatrons, and GitHub contributors to a
new file, Credits.h, which only contains the list (and the credits max
position calculation). That way, whenever someone gets added, only the
minimal amount of files need to be recompiled.
2020-07-03 12:13:15 +02:00
|
|
|
game.creditposition = -Credits::creditmaxposition;
|
2020-11-03 00:23:53 +01:00
|
|
|
graphics.titlebg.bscroll = 0;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
Don't re-draw credits scroll background every frame
While I was working on my over-30-FPS patch, I found out that the tower
background in the credits scroll was being completely re-drawn every
single frame, which was a bit wasteful and expensive. It's also harder
to interpolate for my over-30-FPS patch. I'm guessing this constant
re-draw was done because the math to get the surface scroll properly
working is a bit subtle, but I've figured the precise math out!
The first changes of this patch is just removing the unconditional
`map.tdrawback = true;`, and having to set `map.scrolldir` everywhere to
get the credits scrolling in the right direction but make sure the title
screen doesn't start scrolling like a descending tower, too.
After that, the first problem is that it looks like the ACTION press to
speed up the credits scrolling doesn't speed up the background, too. No
problem, just shove a `!game.press_action` check in
`gamecompletelogic()`.
However, this introduces a mini-problem, which is that NOW when you hold
down ACTION, the background appears to be slowly getting out of sync
with the credits text by a one-pixel-per-second difference. This is
actually due to the fact that, as a result of me adding the conditional,
`map.bscroll` is no longer always unconditionally getting set to 1,
while `game.creditposition` IS always unconditionally getting
decremented by 1. And when you hold down ACTION, `game.creditposition`
gets decremented by 6.
Thus, I need to set `map.bscroll` when holding down ACTION to be 7,
which is 6 plus 1.
Then we have another problem, which is that the incoming textures desync
when you press ACTION, and when you release ACTION. They desync by
precisely 6 pixels, which should be a familiar number. I (eventually)
tracked this down to `map.bypos` being updated at the same time
`map.bscroll` is, even though `map.bypos` should be updated a frame
later AFTER updating `map.bscroll`.
So I had to change the `map.bypos` update in `gamecompleteinput()` and
`gamecompletelogic()` to be `map.bypos += map.bscroll;` and then place
it before any `map.bscroll` update, thus ensuring that `map.bscroll`
updates exactly one frame before `map.ypos` does. I had to move the
`map.bypos += map.bscroll;` to be in `gamecompleteinput()`, because
`gamecompleteinput()` comes first before `gamecompletelogic()` in the
`main.cpp` game loop, otherwise the `map.bypos` update won't be delayed
by one frame for when you press ACTION to make it go faster, and thus
cause a desync when you press ACTION.
Oh and then after that, I had to make the descending tower background
draw a THIRD row of incoming tiles, otherwise you could see some black
flickering at the bottom of the screen when you held down ACTION.
All of this took me way too long to figure out, but now the credits
scroll works perfectly while being more optimized.
2020-04-30 05:52:33 +02:00
|
|
|
else if (!game.press_action)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-11-03 00:23:53 +01:00
|
|
|
graphics.titlebg.bscroll = +1;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
|
2020-04-01 23:48:11 +02:00
|
|
|
if (graphics.fademode == 1)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
//Fix some graphical things
|
2020-04-01 23:48:11 +02:00
|
|
|
graphics.showcutscenebars = false;
|
2021-03-20 07:08:35 +01:00
|
|
|
graphics.setbars(0);
|
2020-11-03 00:23:53 +01:00
|
|
|
graphics.titlebg.scrolldir = 0;
|
|
|
|
|
graphics.titlebg.bypos = 0;
|
2020-01-01 21:29:24 +01:00
|
|
|
//Return to game
|
2020-04-17 05:15:53 +02:00
|
|
|
game.gamestate = GAMECOMPLETE2;
|
2020-04-01 23:48:11 +02:00
|
|
|
graphics.fademode = 4;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
Explicitly declare void for all void parameter functions (#628)
Apparently in C, if you have `void test();`, it's completely okay to do
`test(2);`. The function will take in the argument, but just discard it
and throw it away. It's like a trash can, and a rude one at that. If you
declare it like `void test(void);`, this is prevented.
This is not a problem in C++ - doing `void test();` and `test(2);` is
guaranteed to result in a compile error (this also means that right now,
at least in all `.cpp` files, nobody is ever calling a void parameter
function with arguments and having their arguments be thrown away).
However, we may not be using C++ in the future, so I just want to lay
down the precedent that if a function takes in no arguments, you must
explicitly declare it as such.
I would've added `-Wstrict-prototypes`, but it produces an annoying
warning message saying it doesn't work in C++ mode if you're compiling
in C++ mode. So it can be added later.
2021-02-25 23:23:59 +01:00
|
|
|
void gamecompletelogic2(void)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
//Misc
|
2020-11-03 00:23:53 +01:00
|
|
|
map.updatetowerglow(graphics.titlebg);
|
2020-01-01 21:29:24 +01:00
|
|
|
help.updateglow();
|
|
|
|
|
|
|
|
|
|
game.creditposdelay--;
|
|
|
|
|
if (game.creditposdelay <= 0)
|
|
|
|
|
{
|
|
|
|
|
game.creditposdelay = 1;
|
|
|
|
|
game.creditposx++;
|
|
|
|
|
if (game.creditposx > 40)
|
|
|
|
|
{
|
|
|
|
|
game.creditposy++;
|
|
|
|
|
game.creditposx = 0;
|
|
|
|
|
if (game.creditposy > 30) game.creditposy = 30;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2020-04-01 23:48:11 +02:00
|
|
|
if (graphics.fademode == 1)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
//Fix some graphical things
|
2020-04-01 23:48:11 +02:00
|
|
|
graphics.showcutscenebars = false;
|
2021-03-20 07:08:35 +01:00
|
|
|
graphics.setbars(0);
|
2020-01-01 21:29:24 +01:00
|
|
|
//Fix the save thingy
|
|
|
|
|
game.deletequick();
|
|
|
|
|
int tmp=music.currentsong;
|
|
|
|
|
music.currentsong=4;
|
Fix flag 67 turning on too early
Flag 67 seems to be a general-purpose Game Complete flag. It's used to
replace the CREW option with the SHIP option on the pause screen.
Unfortunately, it gets turned on too early during Game Complete. Right
when Viridian starts to teleport, you can bring up the pause screen and
select the SHIP option.
It will teleport you to the ship coordinates, but still keep you in
finalmode, and since the ship coordinates are at 102,111 (finalmode is
only around 46,54), you'll still be stuck in Outside Dimension VVVVVV,
and you've interrupted the Game Complete gamestate by switching to the
teleporting gamestate. Oh, and your checkpoint is set, too, so you can't
even press R. Oh and since there's no teleporter, and checkpoint setting
doesn't check the sentinel value, this results in Undefined Behavior,
too.
So this results in an in-game softlock. The only option you can do is
quit the game at this point.
To fix this issue, just move turning on flag 67 before the savetele() in
gamecompletelogic2().
2020-05-09 20:41:54 +02:00
|
|
|
obj.flags[67] = true;
|
2020-03-31 02:16:02 +02:00
|
|
|
game.savetele();
|
2020-01-01 21:29:24 +01:00
|
|
|
music.currentsong=tmp;
|
|
|
|
|
//Return to game
|
Clean up all exit paths to the menu to use common code
There are multiple different exit paths to the main menu. In 2.2, they
all had a bunch of copy-pasted code. In 2.3 currently, most of them use
game.quittomenu(), but there are some stragglers that still use
hand-copied code.
This is a bit of a problem, because all exit paths should consistently
have FILESYSTEM_unmountassets(), as part of the 2.3 feature of per-level
custom assets. Furthermore, most (but not all) of the paths call
script.hardreset() too, and some of the stragglers don't. So there could
be something persisting through to the title screen (like a really long
flash/shake timer) that could only persist if exiting to the title
screen through those paths.
But, actually, it seems like there's a good reason for some of those to
not call script.hardreset() - namely, dying or completing No Death Mode
and completing a Time Trial presents some information onscreen that
would get reset by script.hardreset(), so I'll fix that in a later
commit.
So what I've done for this commit is found every exit path that didn't
already use game.quittomenu(), and made them use game.quittomenu(). As
well, some of them had special handling that existed on top of them
already having a corresponding entry in game.quittomenu() (but the path
would take the special handling because it never did game.quittomenu()),
so I removed that special handling as well (e.g. exiting from a custom
level used returntomenu(Menu::levellist) when quittomenu() already had
that same returntomenu()).
The menu that exiting from the level editor returns to is now handled in
game.quittomenu() as well, where the map.custommode branch now also
checks for map.custommodeforreal. Unfortunately, it seems like entering
the level editor doesn't properly initialize map.custommode, so entering
the level editor now initializes map.custommode, too.
I've also taken the music.play(6) out of game.quittomenu(), because not
all exit paths immediately play Presenting VVVVVV, so all exit paths
that DO immediately play Presenting VVVVVV now have music.play(6)
special-cased for them, which is fine enough for me.
Here is the list of all exit paths to the menu:
- Exiting through the pause menu (without glitchrunner mode)
- Exiting through the pause menu (with glitchrunner mode)
- Completing a custom level
- Completing a Time Trial
- Dying in No Death Mode
- Completing No Death Mode
- Completing an Intermission replay
- Exiting from the level editor
- Completing the main game
2021-01-07 23:20:37 +01:00
|
|
|
game.quittomenu();
|
2020-04-16 06:53:36 +02:00
|
|
|
game.createmenu(Menu::gamecompletecontinue);
|
Clean up all exit paths to the menu to use common code
There are multiple different exit paths to the main menu. In 2.2, they
all had a bunch of copy-pasted code. In 2.3 currently, most of them use
game.quittomenu(), but there are some stragglers that still use
hand-copied code.
This is a bit of a problem, because all exit paths should consistently
have FILESYSTEM_unmountassets(), as part of the 2.3 feature of per-level
custom assets. Furthermore, most (but not all) of the paths call
script.hardreset() too, and some of the stragglers don't. So there could
be something persisting through to the title screen (like a really long
flash/shake timer) that could only persist if exiting to the title
screen through those paths.
But, actually, it seems like there's a good reason for some of those to
not call script.hardreset() - namely, dying or completing No Death Mode
and completing a Time Trial presents some information onscreen that
would get reset by script.hardreset(), so I'll fix that in a later
commit.
So what I've done for this commit is found every exit path that didn't
already use game.quittomenu(), and made them use game.quittomenu(). As
well, some of them had special handling that existed on top of them
already having a corresponding entry in game.quittomenu() (but the path
would take the special handling because it never did game.quittomenu()),
so I removed that special handling as well (e.g. exiting from a custom
level used returntomenu(Menu::levellist) when quittomenu() already had
that same returntomenu()).
The menu that exiting from the level editor returns to is now handled in
game.quittomenu() as well, where the map.custommode branch now also
checks for map.custommodeforreal. Unfortunately, it seems like entering
the level editor doesn't properly initialize map.custommode, so entering
the level editor now initializes map.custommode, too.
I've also taken the music.play(6) out of game.quittomenu(), because not
all exit paths immediately play Presenting VVVVVV, so all exit paths
that DO immediately play Presenting VVVVVV now have music.play(6)
special-cased for them, which is fine enough for me.
Here is the list of all exit paths to the menu:
- Exiting through the pause menu (without glitchrunner mode)
- Exiting through the pause menu (with glitchrunner mode)
- Completing a custom level
- Completing a Time Trial
- Dying in No Death Mode
- Completing No Death Mode
- Completing an Intermission replay
- Exiting from the level editor
- Completing the main game
2021-01-07 23:20:37 +01:00
|
|
|
graphics.titlebg.colstate = 10;
|
2020-01-01 21:29:24 +01:00
|
|
|
map.nexttowercolour();
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
Explicitly declare void for all void parameter functions (#628)
Apparently in C, if you have `void test();`, it's completely okay to do
`test(2);`. The function will take in the argument, but just discard it
and throw it away. It's like a trash can, and a rude one at that. If you
declare it like `void test(void);`, this is prevented.
This is not a problem in C++ - doing `void test();` and `test(2);` is
guaranteed to result in a compile error (this also means that right now,
at least in all `.cpp` files, nobody is ever calling a void parameter
function with arguments and having their arguments be thrown away).
However, we may not be using C++ in the future, so I just want to lay
down the precedent that if a function takes in no arguments, you must
explicitly declare it as such.
I would've added `-Wstrict-prototypes`, but it produces an annoying
warning message saying it doesn't work in C++ mode if you're compiling
in C++ mode. So it can be added later.
2021-02-25 23:23:59 +01:00
|
|
|
void gamelogic(void)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-11-08 02:00:42 +01:00
|
|
|
/* Update old lerp positions of entities */
|
|
|
|
|
{size_t i; for (i = 0; i < obj.entities.size(); ++i)
|
|
|
|
|
{
|
|
|
|
|
obj.entities[i].lerpoldxp = obj.entities[i].xp;
|
|
|
|
|
obj.entities[i].lerpoldyp = obj.entities[i].yp;
|
|
|
|
|
}}
|
|
|
|
|
|
2021-03-18 21:20:56 +01:00
|
|
|
if (!game.blackout && !game.completestop)
|
|
|
|
|
{
|
|
|
|
|
size_t i;
|
|
|
|
|
for (i = 0; i < obj.entities.size(); ++i)
|
|
|
|
|
{
|
|
|
|
|
/* Is this entity on the ground? (needed for jumping) */
|
|
|
|
|
if (obj.entitycollidefloor(i))
|
|
|
|
|
{
|
|
|
|
|
obj.entities[i].onground = 2;
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
--obj.entities[i].onground;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if (obj.entitycollideroof(i))
|
|
|
|
|
{
|
|
|
|
|
obj.entities[i].onroof = 2;
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
--obj.entities[i].onroof;
|
|
|
|
|
}
|
Fix regression: quick stopping changing drawframe
This fixes a regression that desyncs my Nova TAS after re-removing the
1-frame input delay.
Quick stopping is simply holding left/right but for less than 5 frames.
Viridian doesn't decelerate when you let go and they immediately stop in
place. (The code calls this tapping, but "quick stopping" is a better
name because you can immediately counter-strafe to stop yourself from
decelrating in the first place, and that works because of this same
code.)
So, the sequence of events in 2.2 and previous looks like this:
- gameinput()
- If quick stopping, set vx to 0
- gamerender()
- Change drawframe depending on vx
- gamelogic()
- Use drawframe for collision (whyyyyyyyyyyyyyyyyyyyyyyyyyyy)
And now (ignoring the intermediate period where the whole loop order was
wrong), the sequence of events in 2.3 looks like this:
- gamerenderfixed()
- Change drawframe depending on vx
- gamerender()
- gameinput()
- If quick stopping, set vx to 0
- gamelogic()
- Use drawframe for collision (my mind has become numb to pain)
So, this means that all the player movement stuff is completely the
same. Except their drawframe is going to be different.
Unfortunately, I had overlooked that gameinput() sets vx and that
animateentities() (in gamerenderfixed()) checks vx. Although, to be
fair, it's a pretty dumb decision to make collision detection be based
on the actual sprites' pixels themselves, instead of a hitbox, in the
first place, so you'd expect THAT to be the end of the dumb parade. Or
maybe you shouldn't, I don't know.
So, what's the solution?
What I've done here is added duplicates of framedelay, drawframe, and
walkingframe, for collision use only. They get updated in gamelogic(),
after gameinput(), which is after when vx could be set to 0.
I've kept the original framedelay, drawframe, and walkingframe around,
to keep the same visuals as closely as possible.
However, due to the removal of the input delay, whenever you quick stop,
your sprite will be wrong for just 1 frame - because when you let go of
the direction key, the game will set your vx to 0 and the logical
drawframe will update to reflect that, but the previous frame cannot
know in advance that you'll release the key on the next frame, and so
the visual drawframe will assume that you keep holding the key.
Whereas in 2.2 and below, when you release a direction key, the player's
position will only update to reflect that on the next frame, but the
current frame can immediately recognize that and update the drawframe
now, instead of retconning it later.
Basically the visual drawframe assumes that you keep holding the key,
and if you don't, then it takes on the value of the collision drawframe
anyway, so it's okay. And it's only visual, anyway - the collision
drawframe of the next frame (when you release the key) will be the same
as the drawframe of the frame you release the key in 2.2 and below.
But I really don't care to try and fix this for if you re-enable the
input delay because it's minor and it'd be more complicated.
2021-06-16 04:18:58 +02:00
|
|
|
|
|
|
|
|
obj.animatehumanoidcollision(i);
|
2021-03-18 21:20:56 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
//Misc
|
|
|
|
|
if (map.towermode)
|
|
|
|
|
{
|
2020-11-03 00:05:24 +01:00
|
|
|
map.updatetowerglow(graphics.towerbg);
|
2020-04-25 06:19:41 +02:00
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
help.updateglow();
|
|
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
if (game.alarmon)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
game.alarmdelay--;
|
|
|
|
|
if (game.alarmdelay <= 0)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
music.playef(19);
|
|
|
|
|
game.alarmdelay = 20;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
if (obj.nearelephant)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
obj.upset++;
|
|
|
|
|
if (obj.upset == 300)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
obj.upsetmode = true;
|
|
|
|
|
//change player to sad
|
|
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities))
|
2020-06-13 05:36:08 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[i].tile = 144;
|
|
|
|
|
}
|
2020-04-25 06:19:41 +02:00
|
|
|
music.playef(2);
|
|
|
|
|
}
|
|
|
|
|
if (obj.upset > 301) obj.upset = 301;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2020-04-25 06:19:41 +02:00
|
|
|
else if (obj.upsetmode)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
obj.upset--;
|
|
|
|
|
if (obj.upset <= 0)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
obj.upset = 0;
|
|
|
|
|
obj.upsetmode = false;
|
|
|
|
|
//change player to happy
|
|
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities))
|
2020-06-13 05:36:08 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[i].tile = 0;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
2020-04-25 06:19:41 +02:00
|
|
|
else
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
obj.upset = 0;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
|
2020-04-30 01:54:36 +02:00
|
|
|
obj.oldtrophytext = obj.trophytext;
|
|
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
if (map.towermode)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-30 20:52:21 +02:00
|
|
|
map.oldypos = map.ypos;
|
2020-04-30 21:58:08 +02:00
|
|
|
map.oldspikeleveltop = map.spikeleveltop;
|
|
|
|
|
map.oldspikelevelbottom = map.spikelevelbottom;
|
2020-04-25 06:19:41 +02:00
|
|
|
if(!game.completestop)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
if (map.cameramode == 0)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
//do nothing!
|
|
|
|
|
//a trigger will set this off in the game
|
|
|
|
|
map.cameramode = 1;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2020-04-25 06:19:41 +02:00
|
|
|
else if (map.cameramode == 1)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
//move normally
|
2020-11-03 00:05:24 +01:00
|
|
|
if(graphics.towerbg.scrolldir==0)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
map.ypos -= 2;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2020-04-25 06:19:41 +02:00
|
|
|
else
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
map.ypos += 2;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
2020-04-25 06:19:41 +02:00
|
|
|
else if (map.cameramode == 2)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
//do nothing, but cycle colours (for taking damage)
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2020-04-25 06:19:41 +02:00
|
|
|
else if (map.cameramode == 4)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities))
|
2020-06-13 05:36:08 +02:00
|
|
|
{
|
|
|
|
|
map.cameraseek = map.ypos - (obj.entities[i].yp - 120);
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
map.cameraseek = map.cameraseek / 10;
|
|
|
|
|
map.cameraseekframe = 10;
|
2020-01-01 21:29:24 +01:00
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
map.cameramode = 5;
|
|
|
|
|
}
|
|
|
|
|
else if (map.cameramode == 5)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
//actually do it
|
|
|
|
|
if (map.spikeleveltop > 0) map.spikeleveltop-=2;
|
|
|
|
|
if (map.spikelevelbottom > 0) map.spikelevelbottom-=2;
|
|
|
|
|
if (map.cameraseekframe > 0)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
int i = obj.getplayer();
|
|
|
|
|
map.ypos -= map.cameraseek;
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (map.cameraseek > 0 && INBOUNDS_VEC(i, obj.entities))
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
if (map.ypos < obj.entities[i].yp - 120)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
map.ypos = obj.entities[i].yp - 120;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
else if (INBOUNDS_VEC(i, obj.entities))
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
if (map.ypos > obj.entities[i].yp - 120)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
map.ypos = obj.entities[i].yp - 120;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
2020-04-25 06:19:41 +02:00
|
|
|
map.cameraseekframe--;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities))
|
2020-06-13 05:36:08 +02:00
|
|
|
{
|
|
|
|
|
map.ypos = obj.entities[i].yp - 120;
|
|
|
|
|
}
|
2020-04-25 06:19:41 +02:00
|
|
|
map.cameramode = 0;
|
|
|
|
|
map.colsuperstate = 0;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
2020-04-25 06:19:41 +02:00
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
if (map.ypos <= 0)
|
|
|
|
|
{
|
|
|
|
|
map.ypos = 0;
|
|
|
|
|
}
|
2020-05-05 20:26:26 +02:00
|
|
|
if (map.towermode && map.minitowermode)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
|
|
|
|
if (map.ypos >= 568)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
map.ypos = 568;
|
|
|
|
|
} //100-29 * 8 = 568
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
if (map.ypos >= 5368)
|
|
|
|
|
{
|
|
|
|
|
map.ypos = 5368; //700-29 * 8 = 5368
|
|
|
|
|
}
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
if (game.lifeseq > 0)
|
|
|
|
|
{
|
|
|
|
|
if (map.cameramode == 2)
|
|
|
|
|
{
|
|
|
|
|
map.cameraseekframe = 20;
|
|
|
|
|
map.cameramode = 4;
|
|
|
|
|
map.resumedelay = 4;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
if (map.cameraseekframe <= 0)
|
|
|
|
|
{
|
|
|
|
|
if (map.resumedelay <= 0)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
game.lifesequence();
|
|
|
|
|
if (game.lifeseq == 0) map.cameramode = 1;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
map.resumedelay--;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
game.lifesequence();
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
|
2020-05-02 21:06:40 +02:00
|
|
|
graphics.kludgeswnlinewidth = false;
|
2020-01-01 21:29:24 +01:00
|
|
|
|
|
|
|
|
if (game.deathseq != -1)
|
|
|
|
|
{
|
2020-04-25 06:19:41 +02:00
|
|
|
if (map.towermode)
|
|
|
|
|
{
|
|
|
|
|
map.colsuperstate = 1;
|
|
|
|
|
map.cameramode = 2;
|
|
|
|
|
}
|
|
|
|
|
|
2020-04-03 22:50:16 +02:00
|
|
|
for (size_t i = 0; i < obj.entities.size(); i++)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-02-04 03:53:30 +01:00
|
|
|
if (game.roomx == 111 && game.roomy == 107 && !map.custommode)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
if (obj.entities[i].type == 1)
|
|
|
|
|
{
|
|
|
|
|
if (obj.entities[i].xp < 152)
|
|
|
|
|
{
|
2020-11-12 22:57:11 +01:00
|
|
|
//Move the platform to the right side of the disappearing platform,
|
|
|
|
|
//otherwise it will get stuck on the kludge 18,9 tile we placed
|
|
|
|
|
//(and if the tile wasn't there it would pass straight through again)
|
|
|
|
|
int prevx = obj.entities[i].xp;
|
|
|
|
|
int prevy = obj.entities[i].yp;
|
Fix entity and block indices after destroying them
This patch restores some 2.2 behavior, fixing a regression caused by the
refactor of properly using std::vectors.
In 2.2, the game allocated 200 items in obj.entities, but used a system
where each entity had an `active` attribute to signify if the entity
actually existed or not. When dealing with entities, you would have to
check this `active` flag, or else you'd be dealing with an entity that
didn't actually exist. (By the way, what I'm saying applies to blocks
and obj.blocks as well, except for some small differing details like the
game allocating 500 block slots versus obj.entities's 200.)
As a consequence, the game had to use a separate tracking variable,
obj.nentity, because obj.entities.size() would just report 200, instead
of the actual amount of entities. Needless to say, having to check for
`active` and use `obj.nentity` is a bit error-prone, and it's messier
than simply using the std::vector the way it was intended. Also, this
resulted in a hard limit of 200 entities, which custom level makers ran
into surprisingly quite often.
2.3 comes along, and removes the whole system. Now, std::vectors are
properly being used, and obj.entities.size() reports the actual number
of entities in the vector; you no longer have to check for `active` when
dealing with entities of any sort.
But there was one previous behavior of 2.2 that this system kind of
forgets about - namely, the ability to have holes in between entities.
You see, when an entity got disabled in 2.2 (which just meant turning
its `active` off), the indices of all other entities stayed the same;
the indice of the entity that got disabled stays there as a hole in the
array. But when an entity gets removed in 2.3 (previous to this patch),
the indices of every entity afterwards in the array get shifted down by
one. std::vector isn't really meant to be able to contain holes.
Do the indices of entities and blocks matter? Yes; they determine the
order in which entities and blocks get evaluated (the highest indice
gets evaluated first), and I had to fix some block evaluation order
stuff in previous PRs.
And in the case of entities, they matter hugely when using the
recently-discovered Arbitrary Entity Manipulation glitch (where crewmate
script commands are used on arbitrary entities by setting the `i`
attribute of `scriptclass` and passing invalid crewmate identifiers to
the commands). If you use Arbitrary Entity Manipulation after destroying
some entities, there is a chance that your script won't work between 2.2
and 2.3.
The indices also still determine the rendering order of entities
(highest indice gets drawn first, which means lowest indice gets drawn
in front of other entities). As an example: let's say we have the player
at 0, a gravity line at 1, and a checkpoint at 2; then we destroy the
gravity line and create a crewmate (let's do Violet).
If we're able to have holes, then after removing the gravity line, none
of the other indices shift. Then Violet will be created at indice 1, and
will be drawn in front of the checkpoint.
But if we can't have holes, then removing the gravity line results in
the indice of the checkpoint shifting down to indice 1. Then Violet is
created at indice 2, and gets drawn behind the checkpoint! This is a
clear illustration of changing the behavior that existed in 2.2.
However, I also don't want to go back to the `active` system of having
to check an attribute before operating on an entity. So... what do we
do to restore the holes?
Well, we don't need to have an `active` attribute, or modify any
existing code that operates on entities. Instead, we can just set the
attributes of the entities so that they naturally get ignored by
everything that comes into contact with it. For entities, we set their
invis to true, and their size, type, and rule to -1 (the game never uses
a size, type, or rule of -1 anywhere); for blocks, we set their type to
-1, and their width and height to 0.
obj.entities.size() will no longer necessarily equal the amount of
entities in the room; rather, it will be the amount of entity SLOTS that
have been allocated. But nothing that uses obj.entities.size() needs to
actually know the amount of entities; it's mostly used for iterating
over every entity in the vector.
Excess entity slots get cleaned up upon every call of
mapclass::gotoroom(), which will now deallocate entity slots starting
from the end until it hits a player, at which point it will switch to
disabling entity slots instead of removing them entirely.
The entclass::clear() and blockclass::clear() functions have been
restored because we need to call their initialization functions when
reusing a block/entity slot; it's possible to create an entity with an
invalid type number (it creates a glitchy Viridian), and without calling
the initialization function again, it would simply not create anything.
After this patch is applied, entity and block indices will be restored
to how they behaved in 2.2.
2020-12-27 07:11:34 +01:00
|
|
|
obj.disableblockat(prevx, prevy);
|
2020-11-12 22:57:11 +01:00
|
|
|
|
2020-01-01 21:29:24 +01:00
|
|
|
obj.entities[i].xp = 152;
|
|
|
|
|
obj.entities[i].newxp = 152;
|
2020-11-12 22:57:11 +01:00
|
|
|
|
|
|
|
|
obj.moveblockto(prevx, prevy, obj.entities[i].xp, obj.entities[i].yp, obj.entities[i].w, obj.entities[i].h);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
if (obj.entities[i].type == 2 && obj.entities[i].state == 3)
|
|
|
|
|
{
|
|
|
|
|
//Ok! super magical exception for the room with the intention death for the shiny trinket
|
|
|
|
|
//fix this when the maps are finalised
|
Fix Prize for the Reckless quicksand fix kludge
If you died in Prize for the Reckless, which is at (11,7), and respawned
in the same room, tile 59 (a solid invisible tile) would be placed at
[18,9] to prevent the moving platform from going back through the
quicksand.
Unfortunately, the way that this kludge was added is poor.
First, the conditional makes it so that it doesn't happen in ONLY
(11,7). Instead of being behind a positive conditional, the tile is
placed in the else-branch of an if-conditional that checks for the
normal case, i.e. if the current room is NOT (11,7), thus being a
negative conditional.
In other words, the positive conditional is "game.roomx == 111 &&
game.roomy == 107". To negate it, all you would have to do is
"!(game.roomx == 111 && game.roomy == 107)".
However, whoever wrote this decided to go one step further, and actually
DISTRIBUTE the negative into both statements. This would be fine, except
if they actually got it right. You see, according to De Morgan's laws,
when you distribute a negative across multiple statements you not only
have to negate the statements themselves, but you have to negate all the
CONJUNCTIONS, too. In other words, you have to change all "and"s into
"or"s and all "or"s into "and"s.
Instead of making the conditional "game.roomx != 111 || game.roomy !=
107", the person who wrote this forgot to replace the "and" with an
"or". Thus, it is "game.roomx != 111 && game.roomy != 107" instead. As a
result, if we re-negate this and take a look at the positive
conditional, i.e. the conditional that results in the else-branch
executing, it turns out to be "game.roomx == 111 || game.roomy == 107".
This ends up forming a cross-shape of rooms where this kludge happens.
As long as your room is either on the line x=11 or on the line y=7, this
kludge will execute.
You can see this if you go to Boldly To Go, since it is (11,13), which
is on the line x=11. Checkpoint in that room, then touch a disappearing
platform, wait for it to fully disappear, then die. Then an invisible
tile will be placed to the left of the spikes on the ceiling.
Anyway, to fix this, it's simple. Just change the "and" in the negative
conditional to an "or".
The second problem was that this kludge was happening in custom levels.
So I've added a map.custommode check to it. I made sure not to make the
same mistake originally made, i.e. I made sure to use an "or" instead of
an "and". Thus, when you re-negate the negative conditional and turn it
into the positive conditional, it reads: "game.roomx == 111 &&
game.roomy == 107 && !map.custommode".
2020-02-02 11:26:49 +01:00
|
|
|
if (game.roomx != 111 || game.roomy != 107 || map.custommode)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
obj.entities[i].state = 4;
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
obj.entities[i].state = 4;
|
|
|
|
|
map.settile(18, 9, 59);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else if (obj.entities[i].type == 2 && obj.entities[i].state == 2)
|
|
|
|
|
{
|
|
|
|
|
//ok, unfortunate case where the disappearing platform hasn't fully disappeared. Accept a little
|
|
|
|
|
//graphical uglyness to avoid breaking the room!
|
Prevent updating an entity if updateentities() removed it
Otherwise, this would result in the game updating an entity twice, which
isn't good. This is most noticeable in the Gravitron, where many
Gravitron squares are created and destroyed at a time, and it's
especially noticeable during the part near the end of the Gravitron
where the pattern is two Gravitron squares, one at the top and bottom,
and then two Gravitron squares in the middle afterwards. The timing is
just right such that the top one of the two middle ones would be
misaligned with the bottom one of the two when a Gravitron square gets
outside the screen.
To do this, I changed entityclass::updateentities() into a bool, and
made every single caller check its return value. I only needed to do
this for the ones preceding updateentitylogic() and
entitymapcollision(), but I wanted to play it safe and be defensive, so
I did it for the disappearing platform kludge, as well as the
updateentities() within the updateentities() function.
2020-05-05 22:49:12 +02:00
|
|
|
bool entitygone = false;
|
|
|
|
|
while (obj.entities[i].state == 2)
|
|
|
|
|
{
|
|
|
|
|
entitygone = obj.updateentities(i);
|
|
|
|
|
if (entitygone)
|
|
|
|
|
{
|
|
|
|
|
i--;
|
|
|
|
|
break;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
if (!entitygone) obj.entities[i].state = 4;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
else if (obj.entities[i].type == 23 && game.swnmode && game.deathseq<15)
|
|
|
|
|
{
|
|
|
|
|
//if playing SWN, get the enemies offscreen.
|
|
|
|
|
obj.entities[i].xp += obj.entities[i].vx*5;
|
|
|
|
|
obj.entities[i].yp += obj.entities[i].vy*5;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
if (game.swnmode)
|
|
|
|
|
{
|
|
|
|
|
//if playing SWN game a, push the clock back to the nearest 10 second interval
|
|
|
|
|
if (game.swngame == 0)
|
|
|
|
|
{
|
|
|
|
|
game.swnpenalty();
|
|
|
|
|
}
|
|
|
|
|
else if (game.swngame == 1)
|
|
|
|
|
{
|
|
|
|
|
game.swnstate = 0;
|
|
|
|
|
game.swnstate2 = 0;
|
|
|
|
|
game.swnstate3 = 0;
|
|
|
|
|
game.swnstate4 = 0;
|
|
|
|
|
game.swndelay = 0;
|
2021-05-04 04:19:22 +02:00
|
|
|
#ifndef MAKEANDPLAY
|
|
|
|
|
if (game.swntimer >= game.swnrecord && !map.custommode)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
game.swnrecord = game.swntimer;
|
2020-07-28 00:50:51 +02:00
|
|
|
if (game.swnmessage == 0)
|
|
|
|
|
{
|
|
|
|
|
music.playef(25);
|
Refactor Game::savestats() to not use a default argument
In order to be able to fix the bug #556, I'm planning on adding
ScreenSettings* to the settings.vvv write function. However, that
entails adding another argument to Game::savesettings(), which is going
to be really messy given the default argument of Game::savestats().
That, combined with the fact that the code comment at the site of the
implementation of Game::savestats() being wrong (!!!), leads me to
believe that using default function arguments here isn't worth it.
Instead, what I've done is made it so callers are explicit about whether
or not they're calling savestats(), savesettings(), or both at the same
time. If they are calling both at the same time, then they will be using
a new function named savestatsandsettings().
In short, these are the interface changes:
* bool Game::savestats(bool) has been removed
* bool Game::savestatsandsettings() has been added
* void Game::savestats_menu() has been renamed to
void Game::savestatsandsettings_menu()
* All previous callers of bool Game::savestats() are now using bool
Game::savestatsandsettings()
* The one caller of bool Game::savestats(bool) is now using bool
Game::savestats()
2020-12-22 01:03:19 +01:00
|
|
|
game.savestatsandsettings();
|
2020-07-28 00:50:51 +02:00
|
|
|
}
|
|
|
|
|
game.swnmessage = 1;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2021-05-04 04:19:22 +02:00
|
|
|
#endif
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2020-03-31 02:16:02 +02:00
|
|
|
game.deathsequence();
|
2020-01-01 21:29:24 +01:00
|
|
|
game.deathseq--;
|
|
|
|
|
if (game.deathseq <= 0)
|
|
|
|
|
{
|
|
|
|
|
if (game.nodeathmode)
|
|
|
|
|
{
|
|
|
|
|
game.deathseq = 1;
|
2020-03-31 02:16:02 +02:00
|
|
|
game.gethardestroom();
|
2020-01-01 21:29:24 +01:00
|
|
|
//start depressing sequence here...
|
2020-04-01 23:48:11 +02:00
|
|
|
if (game.gameoverdelay <= -10 && graphics.fademode==0) graphics.fademode = 2;
|
2021-01-08 01:18:07 +01:00
|
|
|
if (graphics.fademode == 1)
|
|
|
|
|
{
|
|
|
|
|
game.copyndmresults();
|
|
|
|
|
script.resetgametomenu();
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
if (game.swnmode)
|
|
|
|
|
{
|
|
|
|
|
//if playing SWN game b, reset the clock
|
|
|
|
|
if (game.swngame == 1)
|
|
|
|
|
{
|
|
|
|
|
game.swntimer = 0;
|
|
|
|
|
game.swnmessage = 0;
|
|
|
|
|
game.swnrank = 0;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2020-03-31 02:16:02 +02:00
|
|
|
game.gethardestroom();
|
2020-01-01 21:29:24 +01:00
|
|
|
game.hascontrol = true;
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
game.gravitycontrol = game.savegc;
|
2020-04-01 23:48:11 +02:00
|
|
|
graphics.textboxremove();
|
2020-12-28 22:16:16 +01:00
|
|
|
map.resetplayer(true);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
//Update colour thingy
|
|
|
|
|
if (map.finalmode)
|
|
|
|
|
{
|
|
|
|
|
if (map.final_colormode)
|
|
|
|
|
{
|
|
|
|
|
if (map.final_colorframe > 0)
|
|
|
|
|
{
|
|
|
|
|
map.final_colorframedelay--;
|
|
|
|
|
if (map.final_colorframedelay <= 0)
|
|
|
|
|
{
|
|
|
|
|
if (map.final_colorframe == 1)
|
|
|
|
|
{
|
|
|
|
|
map.final_colorframedelay = 40;
|
2020-04-03 03:50:37 +02:00
|
|
|
int temp = 1+int(fRandom() * 6);
|
2020-01-01 21:29:24 +01:00
|
|
|
if (temp == map.final_mapcol) temp = (temp + 1) % 6;
|
|
|
|
|
if (temp == 0) temp = 6;
|
2020-03-31 10:09:42 +02:00
|
|
|
map.changefinalcol(temp);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
else if (map.final_colorframe == 2)
|
|
|
|
|
{
|
|
|
|
|
map.final_colorframedelay = 15;
|
2020-04-03 03:50:37 +02:00
|
|
|
int temp = 1+int(fRandom() * 6);
|
2020-01-01 21:29:24 +01:00
|
|
|
if (temp == map.final_mapcol) temp = (temp + 1) % 6;
|
|
|
|
|
if (temp == 0) temp = 6;
|
2020-03-31 10:09:42 +02:00
|
|
|
map.changefinalcol(temp);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
//State machine for game logic
|
2020-03-31 02:16:02 +02:00
|
|
|
game.updatestate();
|
2020-01-01 21:29:24 +01:00
|
|
|
if (game.startscript)
|
|
|
|
|
{
|
|
|
|
|
script.load(game.newscript);
|
|
|
|
|
game.startscript = false;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
//Intermission 1 Logic
|
|
|
|
|
//Player can't walk off a screen with SCM on it until they've left
|
|
|
|
|
if (game.supercrewmate)
|
|
|
|
|
{
|
|
|
|
|
if (game.roomx == 41 + game.scmprogress) //he's in the same room
|
|
|
|
|
{
|
|
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities) && obj.entities[i].ax > 0 && obj.entities[i].xp > 280)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
obj.entities[i].ax = 0;
|
|
|
|
|
obj.entities[i].dir = 0;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
//SWN Minigame Logic
|
|
|
|
|
if (game.swnmode) //which game?
|
|
|
|
|
{
|
|
|
|
|
if(game.swngame==0) //intermission, survive 60 seconds game
|
|
|
|
|
{
|
|
|
|
|
game.swntimer -= 1;
|
|
|
|
|
if (game.swntimer <= 0)
|
|
|
|
|
{
|
|
|
|
|
music.niceplay(8);
|
|
|
|
|
game.swngame = 5;
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
2020-03-31 02:46:36 +02:00
|
|
|
obj.generateswnwave(0);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else if(game.swngame==1) //super gravitron game
|
|
|
|
|
{
|
|
|
|
|
game.swntimer += 1;
|
2021-05-04 04:19:22 +02:00
|
|
|
#ifndef MAKEANDPLAY
|
|
|
|
|
if (!map.custommode)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2021-05-04 04:19:22 +02:00
|
|
|
if (game.swntimer > game.swnrecord)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2021-05-04 04:19:22 +02:00
|
|
|
game.swnrecord = game.swntimer;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2021-05-04 04:19:22 +02:00
|
|
|
|
|
|
|
|
if (game.swntimer >= 150 && game.swnrank == 0)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2021-05-04 04:19:22 +02:00
|
|
|
game.swnrank = 1;
|
|
|
|
|
if (game.swnbestrank < 1)
|
|
|
|
|
{
|
|
|
|
|
game.unlockAchievement("vvvvvvsupgrav5");
|
|
|
|
|
game.swnbestrank = 1;
|
|
|
|
|
game.swnmessage = 2+30;
|
|
|
|
|
music.playef(26);
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2021-05-04 04:19:22 +02:00
|
|
|
else if (game.swntimer >= 300 && game.swnrank == 1)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2021-05-04 04:19:22 +02:00
|
|
|
game.swnrank = 2;
|
|
|
|
|
if (game.swnbestrank < 2)
|
|
|
|
|
{
|
|
|
|
|
game.unlockAchievement("vvvvvvsupgrav10");
|
|
|
|
|
game.swnbestrank = 2;
|
|
|
|
|
game.swnmessage = 2+30;
|
|
|
|
|
music.playef(26);
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2021-05-04 04:19:22 +02:00
|
|
|
else if (game.swntimer >= 450 && game.swnrank == 2)
|
|
|
|
|
{
|
|
|
|
|
game.swnrank = 3;
|
|
|
|
|
if (game.swnbestrank < 3)
|
|
|
|
|
{
|
|
|
|
|
game.unlockAchievement("vvvvvvsupgrav15");
|
|
|
|
|
game.swnbestrank = 3;
|
|
|
|
|
game.swnmessage = 2+30;
|
|
|
|
|
music.playef(26);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else if (game.swntimer >= 600 && game.swnrank == 3)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2021-05-04 04:19:22 +02:00
|
|
|
game.swnrank = 4;
|
|
|
|
|
if (game.swnbestrank < 4)
|
|
|
|
|
{
|
|
|
|
|
game.unlockAchievement("vvvvvvsupgrav20");
|
|
|
|
|
game.swnbestrank = 4;
|
|
|
|
|
game.swnmessage = 2+30;
|
|
|
|
|
music.playef(26);
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2021-05-04 04:19:22 +02:00
|
|
|
else if (game.swntimer >= 900 && game.swnrank == 4)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2021-05-04 04:19:22 +02:00
|
|
|
game.swnrank = 5;
|
|
|
|
|
if (game.swnbestrank < 5)
|
|
|
|
|
{
|
|
|
|
|
game.unlockAchievement("vvvvvvsupgrav30");
|
|
|
|
|
game.swnbestrank = 5;
|
|
|
|
|
game.swnmessage = 2+30;
|
|
|
|
|
music.playef(26);
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2021-05-04 04:19:22 +02:00
|
|
|
else if (game.swntimer >= 1800 && game.swnrank == 5)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2021-05-04 04:19:22 +02:00
|
|
|
game.swnrank = 6;
|
|
|
|
|
if (game.swnbestrank < 6)
|
|
|
|
|
{
|
|
|
|
|
game.unlockAchievement("vvvvvvsupgrav60");
|
|
|
|
|
game.swnbestrank = 6;
|
|
|
|
|
game.swnmessage = 2+30;
|
|
|
|
|
music.playef(26);
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
2021-05-04 04:19:22 +02:00
|
|
|
#endif
|
2020-01-01 21:29:24 +01:00
|
|
|
|
2020-03-31 02:46:36 +02:00
|
|
|
obj.generateswnwave(1);
|
2020-01-01 21:29:24 +01:00
|
|
|
|
|
|
|
|
game.swncoldelay--;
|
|
|
|
|
if(game.swncoldelay<=0)
|
|
|
|
|
{
|
|
|
|
|
game.swncolstate = (game.swncolstate+1)%6;
|
|
|
|
|
game.swncoldelay = 30;
|
2020-04-01 23:48:11 +02:00
|
|
|
graphics.rcol = game.swncolstate;
|
2020-01-01 21:29:24 +01:00
|
|
|
obj.swnenemiescol(game.swncolstate);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else if (game.swngame == 2) //introduce game a
|
|
|
|
|
{
|
|
|
|
|
game.swndelay--;
|
|
|
|
|
if (game.swndelay <= 0)
|
|
|
|
|
{
|
|
|
|
|
game.swngame = 0;
|
|
|
|
|
game.swndelay = 0;
|
|
|
|
|
game.swntimer = (60 * 30) - 1;
|
|
|
|
|
//game.swntimer = 15;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else if (game.swngame == 3) //extend line
|
|
|
|
|
{
|
Bounds check all entity getters that can return 0
The entity getters I'm referring to are entityclass::getscm(),
entityclass::getlineat(), entityclass::getcrewman(), and
entityclass::getcustomcrewman().
Even though the player should always exist, and the player should always
be indice 0, I wouldn't want to make that assumption. I've been wrong
before.
Also, these functions returning 0 lull you into a false sense of
security. If you assume that commands using these functions are fine,
you'll forget about the fact that `i` in those commands could be
potentially anything, given an invalid argument. In fact, it's possible
to index createactivityzone(), flipgravity(), and customposition()
out-of-bounds by setting `i` to anything! Well, WAS possible. I fixed it
so now they can't.
Furthermore, in the game.scmmoveme block in gamelogic(), obj.getplayer()
wasn't even checked, even though it's been checked in all other places.
I only caught it just now because I wanted to bounds-check all usages of
obj.getscm(), too, and that game.scmmove block also used obj.getscm()
without bounds-checking it as well.
2020-09-10 07:31:09 +02:00
|
|
|
int line = obj.getlineat(84 - 32);
|
|
|
|
|
if (INBOUNDS_VEC(line, obj.entities))
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
Bounds check all entity getters that can return 0
The entity getters I'm referring to are entityclass::getscm(),
entityclass::getlineat(), entityclass::getcrewman(), and
entityclass::getcustomcrewman().
Even though the player should always exist, and the player should always
be indice 0, I wouldn't want to make that assumption. I've been wrong
before.
Also, these functions returning 0 lull you into a false sense of
security. If you assume that commands using these functions are fine,
you'll forget about the fact that `i` in those commands could be
potentially anything, given an invalid argument. In fact, it's possible
to index createactivityzone(), flipgravity(), and customposition()
out-of-bounds by setting `i` to anything! Well, WAS possible. I fixed it
so now they can't.
Furthermore, in the game.scmmoveme block in gamelogic(), obj.getplayer()
wasn't even checked, even though it's been checked in all other places.
I only caught it just now because I wanted to bounds-check all usages of
obj.getscm(), too, and that game.scmmove block also used obj.getscm()
without bounds-checking it as well.
2020-09-10 07:31:09 +02:00
|
|
|
obj.entities[line].w += 24;
|
|
|
|
|
if (obj.entities[line].w > 332)
|
|
|
|
|
{
|
|
|
|
|
obj.entities[line].w = 332;
|
|
|
|
|
game.swngame = 2;
|
|
|
|
|
graphics.kludgeswnlinewidth = true;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else if (game.swngame == 4) //create top line
|
|
|
|
|
{
|
|
|
|
|
game.swngame = 3;
|
2020-03-31 02:46:36 +02:00
|
|
|
obj.createentity(-8, 84 - 32, 11, 8); // (horizontal gravity line)
|
2020-01-01 21:29:24 +01:00
|
|
|
music.niceplay(2);
|
|
|
|
|
game.swndeaths = game.deathcounts;
|
|
|
|
|
}
|
|
|
|
|
else if (game.swngame == 5) //remove line
|
|
|
|
|
{
|
Bounds check all entity getters that can return 0
The entity getters I'm referring to are entityclass::getscm(),
entityclass::getlineat(), entityclass::getcrewman(), and
entityclass::getcustomcrewman().
Even though the player should always exist, and the player should always
be indice 0, I wouldn't want to make that assumption. I've been wrong
before.
Also, these functions returning 0 lull you into a false sense of
security. If you assume that commands using these functions are fine,
you'll forget about the fact that `i` in those commands could be
potentially anything, given an invalid argument. In fact, it's possible
to index createactivityzone(), flipgravity(), and customposition()
out-of-bounds by setting `i` to anything! Well, WAS possible. I fixed it
so now they can't.
Furthermore, in the game.scmmoveme block in gamelogic(), obj.getplayer()
wasn't even checked, even though it's been checked in all other places.
I only caught it just now because I wanted to bounds-check all usages of
obj.getscm(), too, and that game.scmmove block also used obj.getscm()
without bounds-checking it as well.
2020-09-10 07:31:09 +02:00
|
|
|
int line = obj.getlineat(148 + 32);
|
|
|
|
|
if (INBOUNDS_VEC(line, obj.entities))
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
Bounds check all entity getters that can return 0
The entity getters I'm referring to are entityclass::getscm(),
entityclass::getlineat(), entityclass::getcrewman(), and
entityclass::getcustomcrewman().
Even though the player should always exist, and the player should always
be indice 0, I wouldn't want to make that assumption. I've been wrong
before.
Also, these functions returning 0 lull you into a false sense of
security. If you assume that commands using these functions are fine,
you'll forget about the fact that `i` in those commands could be
potentially anything, given an invalid argument. In fact, it's possible
to index createactivityzone(), flipgravity(), and customposition()
out-of-bounds by setting `i` to anything! Well, WAS possible. I fixed it
so now they can't.
Furthermore, in the game.scmmoveme block in gamelogic(), obj.getplayer()
wasn't even checked, even though it's been checked in all other places.
I only caught it just now because I wanted to bounds-check all usages of
obj.getscm(), too, and that game.scmmove block also used obj.getscm()
without bounds-checking it as well.
2020-09-10 07:31:09 +02:00
|
|
|
obj.entities[line].xp += 24;
|
|
|
|
|
if (obj.entities[line].xp > 320)
|
|
|
|
|
{
|
Fix entity and block indices after destroying them
This patch restores some 2.2 behavior, fixing a regression caused by the
refactor of properly using std::vectors.
In 2.2, the game allocated 200 items in obj.entities, but used a system
where each entity had an `active` attribute to signify if the entity
actually existed or not. When dealing with entities, you would have to
check this `active` flag, or else you'd be dealing with an entity that
didn't actually exist. (By the way, what I'm saying applies to blocks
and obj.blocks as well, except for some small differing details like the
game allocating 500 block slots versus obj.entities's 200.)
As a consequence, the game had to use a separate tracking variable,
obj.nentity, because obj.entities.size() would just report 200, instead
of the actual amount of entities. Needless to say, having to check for
`active` and use `obj.nentity` is a bit error-prone, and it's messier
than simply using the std::vector the way it was intended. Also, this
resulted in a hard limit of 200 entities, which custom level makers ran
into surprisingly quite often.
2.3 comes along, and removes the whole system. Now, std::vectors are
properly being used, and obj.entities.size() reports the actual number
of entities in the vector; you no longer have to check for `active` when
dealing with entities of any sort.
But there was one previous behavior of 2.2 that this system kind of
forgets about - namely, the ability to have holes in between entities.
You see, when an entity got disabled in 2.2 (which just meant turning
its `active` off), the indices of all other entities stayed the same;
the indice of the entity that got disabled stays there as a hole in the
array. But when an entity gets removed in 2.3 (previous to this patch),
the indices of every entity afterwards in the array get shifted down by
one. std::vector isn't really meant to be able to contain holes.
Do the indices of entities and blocks matter? Yes; they determine the
order in which entities and blocks get evaluated (the highest indice
gets evaluated first), and I had to fix some block evaluation order
stuff in previous PRs.
And in the case of entities, they matter hugely when using the
recently-discovered Arbitrary Entity Manipulation glitch (where crewmate
script commands are used on arbitrary entities by setting the `i`
attribute of `scriptclass` and passing invalid crewmate identifiers to
the commands). If you use Arbitrary Entity Manipulation after destroying
some entities, there is a chance that your script won't work between 2.2
and 2.3.
The indices also still determine the rendering order of entities
(highest indice gets drawn first, which means lowest indice gets drawn
in front of other entities). As an example: let's say we have the player
at 0, a gravity line at 1, and a checkpoint at 2; then we destroy the
gravity line and create a crewmate (let's do Violet).
If we're able to have holes, then after removing the gravity line, none
of the other indices shift. Then Violet will be created at indice 1, and
will be drawn in front of the checkpoint.
But if we can't have holes, then removing the gravity line results in
the indice of the checkpoint shifting down to indice 1. Then Violet is
created at indice 2, and gets drawn behind the checkpoint! This is a
clear illustration of changing the behavior that existed in 2.2.
However, I also don't want to go back to the `active` system of having
to check an attribute before operating on an entity. So... what do we
do to restore the holes?
Well, we don't need to have an `active` attribute, or modify any
existing code that operates on entities. Instead, we can just set the
attributes of the entities so that they naturally get ignored by
everything that comes into contact with it. For entities, we set their
invis to true, and their size, type, and rule to -1 (the game never uses
a size, type, or rule of -1 anywhere); for blocks, we set their type to
-1, and their width and height to 0.
obj.entities.size() will no longer necessarily equal the amount of
entities in the room; rather, it will be the amount of entity SLOTS that
have been allocated. But nothing that uses obj.entities.size() needs to
actually know the amount of entities; it's mostly used for iterating
over every entity in the vector.
Excess entity slots get cleaned up upon every call of
mapclass::gotoroom(), which will now deallocate entity slots starting
from the end until it hits a player, at which point it will switch to
disabling entity slots instead of removing them entirely.
The entclass::clear() and blockclass::clear() functions have been
restored because we need to call their initialization functions when
reusing a block/entity slot; it's possible to create an entity with an
invalid type number (it creates a glitchy Viridian), and without calling
the initialization function again, it would simply not create anything.
After this patch is applied, entity and block indices will be restored
to how they behaved in 2.2.
2020-12-27 07:11:34 +01:00
|
|
|
obj.disableentity(line);
|
2020-11-20 00:16:54 +01:00
|
|
|
game.swngame = 8;
|
Bounds check all entity getters that can return 0
The entity getters I'm referring to are entityclass::getscm(),
entityclass::getlineat(), entityclass::getcrewman(), and
entityclass::getcustomcrewman().
Even though the player should always exist, and the player should always
be indice 0, I wouldn't want to make that assumption. I've been wrong
before.
Also, these functions returning 0 lull you into a false sense of
security. If you assume that commands using these functions are fine,
you'll forget about the fact that `i` in those commands could be
potentially anything, given an invalid argument. In fact, it's possible
to index createactivityzone(), flipgravity(), and customposition()
out-of-bounds by setting `i` to anything! Well, WAS possible. I fixed it
so now they can't.
Furthermore, in the game.scmmoveme block in gamelogic(), obj.getplayer()
wasn't even checked, even though it's been checked in all other places.
I only caught it just now because I wanted to bounds-check all usages of
obj.getscm(), too, and that game.scmmove block also used obj.getscm()
without bounds-checking it as well.
2020-09-10 07:31:09 +02:00
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else if (game.swngame == 6) //Init the super gravitron
|
|
|
|
|
{
|
|
|
|
|
game.swngame = 7;
|
|
|
|
|
music.niceplay(3);
|
|
|
|
|
}
|
|
|
|
|
else if (game.swngame == 7) //introduce game b
|
|
|
|
|
{
|
|
|
|
|
game.swndelay--;
|
|
|
|
|
if (game.swndelay <= 0)
|
|
|
|
|
{
|
|
|
|
|
game.swngame = 1;
|
|
|
|
|
game.swndelay = 0;
|
|
|
|
|
game.swntimer = 0;
|
|
|
|
|
game.swncolstate = 3;
|
|
|
|
|
game.swncoldelay = 30;
|
|
|
|
|
}
|
|
|
|
|
}
|
2020-11-20 00:16:54 +01:00
|
|
|
else if (game.swngame == 8) //extra kludge if player dies after game a ends
|
|
|
|
|
{
|
|
|
|
|
bool square_onscreen = false;
|
|
|
|
|
for (size_t i = 0; i < obj.entities.size(); i++)
|
|
|
|
|
{
|
|
|
|
|
if (obj.entities[i].type == 23)
|
|
|
|
|
{
|
|
|
|
|
square_onscreen = true;
|
|
|
|
|
break;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
if (!square_onscreen)
|
|
|
|
|
{
|
|
|
|
|
game.swnmode = false;
|
|
|
|
|
}
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
//Time trial stuff
|
|
|
|
|
if (game.intimetrial)
|
|
|
|
|
{
|
|
|
|
|
|
|
|
|
|
if (game.timetrialcountdown > 0)
|
|
|
|
|
{
|
|
|
|
|
game.hascontrol = true;
|
|
|
|
|
game.timetrialcountdown--;
|
|
|
|
|
if (game.timetrialcountdown > 30)
|
|
|
|
|
{
|
|
|
|
|
game.hascontrol = false;
|
|
|
|
|
}
|
2020-04-02 01:36:35 +02:00
|
|
|
if(game.timetrialcountdown == 120) music.playef(21);
|
|
|
|
|
if(game.timetrialcountdown == 90) music.playef(21);
|
|
|
|
|
if(game.timetrialcountdown == 60) music.playef(21);
|
2020-01-01 21:29:24 +01:00
|
|
|
if (game.timetrialcountdown == 30)
|
|
|
|
|
{
|
|
|
|
|
switch(game.timetriallevel)
|
|
|
|
|
{
|
|
|
|
|
case 0:
|
|
|
|
|
music.play(1);
|
|
|
|
|
break;
|
|
|
|
|
case 1:
|
|
|
|
|
music.play(3);
|
|
|
|
|
break;
|
|
|
|
|
case 2:
|
|
|
|
|
music.play(2);
|
|
|
|
|
break;
|
|
|
|
|
case 3:
|
|
|
|
|
music.play(1);
|
|
|
|
|
break;
|
|
|
|
|
case 4:
|
|
|
|
|
music.play(12);
|
|
|
|
|
break;
|
|
|
|
|
case 5:
|
|
|
|
|
music.play(15);
|
|
|
|
|
break;
|
|
|
|
|
}
|
2020-04-02 01:36:35 +02:00
|
|
|
music.playef(22);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
//Have we lost the par?
|
|
|
|
|
if (!game.timetrialparlost)
|
|
|
|
|
{
|
|
|
|
|
if ((game.minutes * 60) + game.seconds > game.timetrialpar)
|
|
|
|
|
{
|
|
|
|
|
game.timetrialparlost = true;
|
|
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities))
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
obj.entities[i].tile = 144;
|
|
|
|
|
}
|
2020-04-02 01:36:35 +02:00
|
|
|
music.playef(2);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
//Update entities
|
|
|
|
|
//Ok, moving platform fuckers
|
|
|
|
|
if(!game.completestop)
|
|
|
|
|
{
|
|
|
|
|
if(obj.vertplatforms)
|
|
|
|
|
{
|
2020-04-03 22:50:16 +02:00
|
|
|
for (int i = obj.entities.size() - 1; i >= 0; i--)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-10-07 01:15:44 +02:00
|
|
|
if (!obj.entities[i].isplatform
|
|
|
|
|
|| SDL_abs(obj.entities[i].vx) >= 0.000001f)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-10-07 01:15:44 +02:00
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
|
Restore platform evaluation order to 2.2
This commit restores the evaluation order of moving platforms and conveyors to
be what it was in 2.2. The evaluation order changed in 2.3 after the patchset
to improve the handling of the `obj.entities` and `obj.blocks` vectors (#191).
By evaluation order, I'm talking about the order in which platforms and
conveyors will be evaluated (and thus will take priority) if Viridian stands
on both a conveyor or platform at once, and they either have different speeds
or are pointing in different directions. Nowhere in the main game is there a
place where you can stand on two different conveyors/platforms at once, so
this is solely within the territory of custom levels, which is my specialty.
So what caused this evaluation order to change? Well, every moving platform
and conveyor in the game is actually made up of two objects: an entity, and a
block. The entity is the part that moves around, and the block is the part
that actually has the collision.
But if the entity is the part that moves around, and entities and blocks are
in entirely separate vectors, how is the block part going to move along with
it? Well, maybe you'd guess some sort of unique ID system, but spend some time
digging around the code and you won't find any trace of any (there's no
attribute on an entity to store such an ID, for starters).
Instead, what the game does is actually remove all blocks that coincide with
the exact top-left corner of the entity, and then create a new one. Destroying
and creating blocks like this all the time is hugely wasteful, but hey, it
worked.
So why did the evaluation order change in 2.3? Well, to understand that,
you'll need to understand 2.2's `active` system. Instead of having an object
be real simply by virtue of it existing, 2.2 had this system where the object
was only real if it had its `active` attribute set to true. In other words,
you would be looking at a fake object that didn't actually exist if its
`active` attribute was false.
On the surface, this doesn't seem that bad. But this can lead to "holes" in a
given vector of objects. A hole is simply an inactive object neighbored by
active objects (or the inactive object could be the first one in the vector,
but then have an active object immediately following it).
If you have a vector of 3 objects, all of them active, then removing the
second one will result in the vector containing an active object, followed by
an inactive object, followed by an active one. However, since the switch to
more properly use vectors instead of relying on this `active` system, there's
no longer any way for holes to exist in a vector. Properly removing an object
from a vector will just shift the rest of the objects down, so if we remove
the second object after the vector fix, then this will simply move the third
object into the slot of where the second object used to be.
So, what happens if you destroy a block and then create a new one in the
`active` system? Let's say that your `obj.blocks` looks like this, and here
I'm denoting each block by writing out its coordinates:
[30,60] [70,90] [80,100]
and that you want to update the position of the second one, because the entity
that that blocks belongs to has been updated. Okay, so, you delete that block,
which then makes things look like this:
[30,60] [-] [80,100]
and then afterwards, you create a new block with the updated position,
resulting in this:
[30,60] [74,90] [80,100]
Since `entityclass::createblock()` will find the first block slot that has a
false `active` attribute, it puts the new object in the same slot as the old
one. What has been essentially done here is that the slot of the block has
basically been reserved for the new block with the new position. Here, the
evaluation order of each block will stay the same.
But then 2.3 comes along and changes things up. So we start with an
`obj.blocks` like this again:
[30,60] [70,90] [80,100]
and we want to update the second block, like before. So we remove the second
block, resulting in this:
[30,60] [80,100]
It should be obvious that unlike before, where the third block stayed in the
third slot, the third block has now been moved to the second slot. But
continuing on; we are now going to create the new block with its updated
position, resulting in this:
[30,60] [80,100] [70,90]
At this point, we can see that the evaluation order of these blocks has been
changed due to the fact that the third block has now been moved to the slot
that was previously the slot of the second block.
So what can we do about this? Well, we can basically emulate what VVVVVV did
in 2.2, which is disable a block without actually removing it - except I'm not
going to reintroduce an `active` attribute or anything. I'll disable the
collision of all blocks at a certain position by setting their widths and
heights to 0, and then re-enable them later by finding the first block at that
same position, updating its position, and re-assigning its width and height
again.
The former is what `entityclass::nocollisionat()` does; the latter is what
`entityclass::moveblockto()` does. The former mimicks turning off the `active`
attribute of all blocks sharing a certain top-left corner; the latter mimicks
creating a new block - and it will only do this for one block, because
`entityclass::createblock()` in 2.2 only looked for the first block with a
false `active` attribute.
Now, some quirks relied on the previous behavior of destroying and creating
blocks, but all of these quirks have been preserved with the way I implemented
this fix.
The first quirk is that platforms passing through 0,0 will destroy all spike
hitboxes, script boxes, activity zones, and one-way hitboxes in the room. The
hitboxes of moving platforms, disappearing platforms, 1x1 quicksand, and
conveyors will not be affected.
This is a consequence of the fact that the former group uses the `x` and `y`
of their `rect`, while the latter group uses the `xp` and `yp` attributes. So
the `xp` and `yp` of the former are both 0. Meaning, a platform passing
through 0,0 destroys them all.
Having these separate coordinates seems like an artifact from the Flash days.
(And furthermore, there's an unused `x` and `y` attribute on all blocks,
making for technically three separate sets of coordinates! This should
probably be cleaned up, except for what I'm about to say...) But actually, if
you merge both sets of coordinates into one, this lets moving platforms
destroy script boxes and activity zones if it passes through the top-left
corner of them, which is probably far worse than the destruction being
localized to a specific coordinate that would never likely be reached
normally.
This quirk is preserved just fine without any special-casing, because instead
of destroying all blocks at 0,0, they just get disabled, which does the same
job. This quirk seems trivial to fix if I made it so that the position of a
platform's block was updated instantaneously instead of having one step to
disable it and another step to re-enable it, but I aim to preserve as much
quirks as possible.
The second quirk is that a moving platform passing through the top-left corner
of a disappearing platform or 1x1 quicksand will destroy the block of that
disappearing platform. This is because, again, when a moving platform updates,
it destroys all blocks at its previous position, not just only one block. This
is automatically preserved because this commit just disables the block of the
disappearing platform instead of removing it. Just like the last one, this
quirk seems extremely trivial to fix, and this time by simply making it so
`entityclass::nocollisionat()` would have a `break` statement, i.e. only
disabling the first block it finds instead of all blocks it finds, but I want
to keep all quirks that are possible to keep.
The last quirk is that, apparently, in order to prevent pushing the player
vertically out of a moving platform if they get inside of one, the game
destroys the block of the moving platform. If I had missed this edge case,
then the block would've been destroyed, leaving the moving platform with no
collision. But I caught it in my testing, so the block gets disabled instead
of destroyed. Also, it seems obtuse for those who don't understand why a
platform's block gets destroyed or disabled whenever the player collides with
it in `entityclass::collisioncheck()`, so I've put up a comment documenting it
as well.
The different platform evaluation order desyncs my Nova TAS, but after
applying this patchset and #504, my TAS syncs fine (save for the different
walkingframe from starting immediately on the ground instead of in the air
(#502), but that's minor and can be easily fixed).
I've attached a test level to the pull request for this commit (#503) to
demonstrate that this patchset not only fixes platform evaluation order, but
preserves some bugs and quirks with the existing block system.
The first room demonstrates the fixed platform evaluation order, by stepping
on the conveyors that both point into each other. In 2.2, Viridian will move
to the right of the background pillar, but in 2.3, Viridian will move to the
left of the pillar. However, after applying this patch, Viridian will now move
to the right of the pillar once again.
The second room demonstrates that the platform-passing-through-0,0 trick still
works (as explained above).
The last room demonstrates that platforms passing through the top-left corners
of disappearing platforms or 1x1 quicksand will remove the blocks of those
entities, causing Viridian to immediately pass through them. This trick is
still preserved after my patchset is applied.
2020-10-10 02:09:11 +02:00
|
|
|
int prevx = obj.entities[i].xp;
|
|
|
|
|
int prevy = obj.entities[i].yp;
|
Fix entity and block indices after destroying them
This patch restores some 2.2 behavior, fixing a regression caused by the
refactor of properly using std::vectors.
In 2.2, the game allocated 200 items in obj.entities, but used a system
where each entity had an `active` attribute to signify if the entity
actually existed or not. When dealing with entities, you would have to
check this `active` flag, or else you'd be dealing with an entity that
didn't actually exist. (By the way, what I'm saying applies to blocks
and obj.blocks as well, except for some small differing details like the
game allocating 500 block slots versus obj.entities's 200.)
As a consequence, the game had to use a separate tracking variable,
obj.nentity, because obj.entities.size() would just report 200, instead
of the actual amount of entities. Needless to say, having to check for
`active` and use `obj.nentity` is a bit error-prone, and it's messier
than simply using the std::vector the way it was intended. Also, this
resulted in a hard limit of 200 entities, which custom level makers ran
into surprisingly quite often.
2.3 comes along, and removes the whole system. Now, std::vectors are
properly being used, and obj.entities.size() reports the actual number
of entities in the vector; you no longer have to check for `active` when
dealing with entities of any sort.
But there was one previous behavior of 2.2 that this system kind of
forgets about - namely, the ability to have holes in between entities.
You see, when an entity got disabled in 2.2 (which just meant turning
its `active` off), the indices of all other entities stayed the same;
the indice of the entity that got disabled stays there as a hole in the
array. But when an entity gets removed in 2.3 (previous to this patch),
the indices of every entity afterwards in the array get shifted down by
one. std::vector isn't really meant to be able to contain holes.
Do the indices of entities and blocks matter? Yes; they determine the
order in which entities and blocks get evaluated (the highest indice
gets evaluated first), and I had to fix some block evaluation order
stuff in previous PRs.
And in the case of entities, they matter hugely when using the
recently-discovered Arbitrary Entity Manipulation glitch (where crewmate
script commands are used on arbitrary entities by setting the `i`
attribute of `scriptclass` and passing invalid crewmate identifiers to
the commands). If you use Arbitrary Entity Manipulation after destroying
some entities, there is a chance that your script won't work between 2.2
and 2.3.
The indices also still determine the rendering order of entities
(highest indice gets drawn first, which means lowest indice gets drawn
in front of other entities). As an example: let's say we have the player
at 0, a gravity line at 1, and a checkpoint at 2; then we destroy the
gravity line and create a crewmate (let's do Violet).
If we're able to have holes, then after removing the gravity line, none
of the other indices shift. Then Violet will be created at indice 1, and
will be drawn in front of the checkpoint.
But if we can't have holes, then removing the gravity line results in
the indice of the checkpoint shifting down to indice 1. Then Violet is
created at indice 2, and gets drawn behind the checkpoint! This is a
clear illustration of changing the behavior that existed in 2.2.
However, I also don't want to go back to the `active` system of having
to check an attribute before operating on an entity. So... what do we
do to restore the holes?
Well, we don't need to have an `active` attribute, or modify any
existing code that operates on entities. Instead, we can just set the
attributes of the entities so that they naturally get ignored by
everything that comes into contact with it. For entities, we set their
invis to true, and their size, type, and rule to -1 (the game never uses
a size, type, or rule of -1 anywhere); for blocks, we set their type to
-1, and their width and height to 0.
obj.entities.size() will no longer necessarily equal the amount of
entities in the room; rather, it will be the amount of entity SLOTS that
have been allocated. But nothing that uses obj.entities.size() needs to
actually know the amount of entities; it's mostly used for iterating
over every entity in the vector.
Excess entity slots get cleaned up upon every call of
mapclass::gotoroom(), which will now deallocate entity slots starting
from the end until it hits a player, at which point it will switch to
disabling entity slots instead of removing them entirely.
The entclass::clear() and blockclass::clear() functions have been
restored because we need to call their initialization functions when
reusing a block/entity slot; it's possible to create an entity with an
invalid type number (it creates a glitchy Viridian), and without calling
the initialization function again, it would simply not create anything.
After this patch is applied, entity and block indices will be restored
to how they behaved in 2.2.
2020-12-27 07:11:34 +01:00
|
|
|
obj.disableblockat(prevx, prevy);
|
2020-01-01 21:29:24 +01:00
|
|
|
|
2020-10-07 01:16:48 +02:00
|
|
|
bool entitygone = obj.updateentities(i); // Behavioral logic
|
|
|
|
|
if (entitygone) continue;
|
|
|
|
|
obj.updateentitylogic(i); // Basic Physics
|
|
|
|
|
obj.entitymapcollision(i); // Collisions with walls
|
2020-01-01 21:29:24 +01:00
|
|
|
|
Restore platform evaluation order to 2.2
This commit restores the evaluation order of moving platforms and conveyors to
be what it was in 2.2. The evaluation order changed in 2.3 after the patchset
to improve the handling of the `obj.entities` and `obj.blocks` vectors (#191).
By evaluation order, I'm talking about the order in which platforms and
conveyors will be evaluated (and thus will take priority) if Viridian stands
on both a conveyor or platform at once, and they either have different speeds
or are pointing in different directions. Nowhere in the main game is there a
place where you can stand on two different conveyors/platforms at once, so
this is solely within the territory of custom levels, which is my specialty.
So what caused this evaluation order to change? Well, every moving platform
and conveyor in the game is actually made up of two objects: an entity, and a
block. The entity is the part that moves around, and the block is the part
that actually has the collision.
But if the entity is the part that moves around, and entities and blocks are
in entirely separate vectors, how is the block part going to move along with
it? Well, maybe you'd guess some sort of unique ID system, but spend some time
digging around the code and you won't find any trace of any (there's no
attribute on an entity to store such an ID, for starters).
Instead, what the game does is actually remove all blocks that coincide with
the exact top-left corner of the entity, and then create a new one. Destroying
and creating blocks like this all the time is hugely wasteful, but hey, it
worked.
So why did the evaluation order change in 2.3? Well, to understand that,
you'll need to understand 2.2's `active` system. Instead of having an object
be real simply by virtue of it existing, 2.2 had this system where the object
was only real if it had its `active` attribute set to true. In other words,
you would be looking at a fake object that didn't actually exist if its
`active` attribute was false.
On the surface, this doesn't seem that bad. But this can lead to "holes" in a
given vector of objects. A hole is simply an inactive object neighbored by
active objects (or the inactive object could be the first one in the vector,
but then have an active object immediately following it).
If you have a vector of 3 objects, all of them active, then removing the
second one will result in the vector containing an active object, followed by
an inactive object, followed by an active one. However, since the switch to
more properly use vectors instead of relying on this `active` system, there's
no longer any way for holes to exist in a vector. Properly removing an object
from a vector will just shift the rest of the objects down, so if we remove
the second object after the vector fix, then this will simply move the third
object into the slot of where the second object used to be.
So, what happens if you destroy a block and then create a new one in the
`active` system? Let's say that your `obj.blocks` looks like this, and here
I'm denoting each block by writing out its coordinates:
[30,60] [70,90] [80,100]
and that you want to update the position of the second one, because the entity
that that blocks belongs to has been updated. Okay, so, you delete that block,
which then makes things look like this:
[30,60] [-] [80,100]
and then afterwards, you create a new block with the updated position,
resulting in this:
[30,60] [74,90] [80,100]
Since `entityclass::createblock()` will find the first block slot that has a
false `active` attribute, it puts the new object in the same slot as the old
one. What has been essentially done here is that the slot of the block has
basically been reserved for the new block with the new position. Here, the
evaluation order of each block will stay the same.
But then 2.3 comes along and changes things up. So we start with an
`obj.blocks` like this again:
[30,60] [70,90] [80,100]
and we want to update the second block, like before. So we remove the second
block, resulting in this:
[30,60] [80,100]
It should be obvious that unlike before, where the third block stayed in the
third slot, the third block has now been moved to the second slot. But
continuing on; we are now going to create the new block with its updated
position, resulting in this:
[30,60] [80,100] [70,90]
At this point, we can see that the evaluation order of these blocks has been
changed due to the fact that the third block has now been moved to the slot
that was previously the slot of the second block.
So what can we do about this? Well, we can basically emulate what VVVVVV did
in 2.2, which is disable a block without actually removing it - except I'm not
going to reintroduce an `active` attribute or anything. I'll disable the
collision of all blocks at a certain position by setting their widths and
heights to 0, and then re-enable them later by finding the first block at that
same position, updating its position, and re-assigning its width and height
again.
The former is what `entityclass::nocollisionat()` does; the latter is what
`entityclass::moveblockto()` does. The former mimicks turning off the `active`
attribute of all blocks sharing a certain top-left corner; the latter mimicks
creating a new block - and it will only do this for one block, because
`entityclass::createblock()` in 2.2 only looked for the first block with a
false `active` attribute.
Now, some quirks relied on the previous behavior of destroying and creating
blocks, but all of these quirks have been preserved with the way I implemented
this fix.
The first quirk is that platforms passing through 0,0 will destroy all spike
hitboxes, script boxes, activity zones, and one-way hitboxes in the room. The
hitboxes of moving platforms, disappearing platforms, 1x1 quicksand, and
conveyors will not be affected.
This is a consequence of the fact that the former group uses the `x` and `y`
of their `rect`, while the latter group uses the `xp` and `yp` attributes. So
the `xp` and `yp` of the former are both 0. Meaning, a platform passing
through 0,0 destroys them all.
Having these separate coordinates seems like an artifact from the Flash days.
(And furthermore, there's an unused `x` and `y` attribute on all blocks,
making for technically three separate sets of coordinates! This should
probably be cleaned up, except for what I'm about to say...) But actually, if
you merge both sets of coordinates into one, this lets moving platforms
destroy script boxes and activity zones if it passes through the top-left
corner of them, which is probably far worse than the destruction being
localized to a specific coordinate that would never likely be reached
normally.
This quirk is preserved just fine without any special-casing, because instead
of destroying all blocks at 0,0, they just get disabled, which does the same
job. This quirk seems trivial to fix if I made it so that the position of a
platform's block was updated instantaneously instead of having one step to
disable it and another step to re-enable it, but I aim to preserve as much
quirks as possible.
The second quirk is that a moving platform passing through the top-left corner
of a disappearing platform or 1x1 quicksand will destroy the block of that
disappearing platform. This is because, again, when a moving platform updates,
it destroys all blocks at its previous position, not just only one block. This
is automatically preserved because this commit just disables the block of the
disappearing platform instead of removing it. Just like the last one, this
quirk seems extremely trivial to fix, and this time by simply making it so
`entityclass::nocollisionat()` would have a `break` statement, i.e. only
disabling the first block it finds instead of all blocks it finds, but I want
to keep all quirks that are possible to keep.
The last quirk is that, apparently, in order to prevent pushing the player
vertically out of a moving platform if they get inside of one, the game
destroys the block of the moving platform. If I had missed this edge case,
then the block would've been destroyed, leaving the moving platform with no
collision. But I caught it in my testing, so the block gets disabled instead
of destroyed. Also, it seems obtuse for those who don't understand why a
platform's block gets destroyed or disabled whenever the player collides with
it in `entityclass::collisioncheck()`, so I've put up a comment documenting it
as well.
The different platform evaluation order desyncs my Nova TAS, but after
applying this patchset and #504, my TAS syncs fine (save for the different
walkingframe from starting immediately on the ground instead of in the air
(#502), but that's minor and can be easily fixed).
I've attached a test level to the pull request for this commit (#503) to
demonstrate that this patchset not only fixes platform evaluation order, but
preserves some bugs and quirks with the existing block system.
The first room demonstrates the fixed platform evaluation order, by stepping
on the conveyors that both point into each other. In 2.2, Viridian will move
to the right of the background pillar, but in 2.3, Viridian will move to the
left of the pillar. However, after applying this patch, Viridian will now move
to the right of the pillar once again.
The second room demonstrates that the platform-passing-through-0,0 trick still
works (as explained above).
The last room demonstrates that platforms passing through the top-left corners
of disappearing platforms or 1x1 quicksand will remove the blocks of those
entities, causing Viridian to immediately pass through them. This trick is
still preserved after my patchset is applied.
2020-10-10 02:09:11 +02:00
|
|
|
obj.moveblockto(prevx, prevy, obj.entities[i].xp, obj.entities[i].yp, obj.entities[i].w, obj.entities[i].h);
|
2020-10-07 01:16:48 +02:00
|
|
|
obj.movingplatformfix(i, obj.getplayer());
|
|
|
|
|
if (game.supercrewmate)
|
|
|
|
|
{
|
|
|
|
|
obj.movingplatformfix(i, obj.getscm());
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if(obj.horplatforms)
|
|
|
|
|
{
|
2020-04-03 22:50:16 +02:00
|
|
|
for (int ie = obj.entities.size() - 1; ie >= 0; ie--)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-10-07 01:15:44 +02:00
|
|
|
if (!obj.entities[ie].isplatform
|
|
|
|
|
|| SDL_abs(obj.entities[ie].vy) >= 0.000001f)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-10-07 01:15:44 +02:00
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
|
Restore platform evaluation order to 2.2
This commit restores the evaluation order of moving platforms and conveyors to
be what it was in 2.2. The evaluation order changed in 2.3 after the patchset
to improve the handling of the `obj.entities` and `obj.blocks` vectors (#191).
By evaluation order, I'm talking about the order in which platforms and
conveyors will be evaluated (and thus will take priority) if Viridian stands
on both a conveyor or platform at once, and they either have different speeds
or are pointing in different directions. Nowhere in the main game is there a
place where you can stand on two different conveyors/platforms at once, so
this is solely within the territory of custom levels, which is my specialty.
So what caused this evaluation order to change? Well, every moving platform
and conveyor in the game is actually made up of two objects: an entity, and a
block. The entity is the part that moves around, and the block is the part
that actually has the collision.
But if the entity is the part that moves around, and entities and blocks are
in entirely separate vectors, how is the block part going to move along with
it? Well, maybe you'd guess some sort of unique ID system, but spend some time
digging around the code and you won't find any trace of any (there's no
attribute on an entity to store such an ID, for starters).
Instead, what the game does is actually remove all blocks that coincide with
the exact top-left corner of the entity, and then create a new one. Destroying
and creating blocks like this all the time is hugely wasteful, but hey, it
worked.
So why did the evaluation order change in 2.3? Well, to understand that,
you'll need to understand 2.2's `active` system. Instead of having an object
be real simply by virtue of it existing, 2.2 had this system where the object
was only real if it had its `active` attribute set to true. In other words,
you would be looking at a fake object that didn't actually exist if its
`active` attribute was false.
On the surface, this doesn't seem that bad. But this can lead to "holes" in a
given vector of objects. A hole is simply an inactive object neighbored by
active objects (or the inactive object could be the first one in the vector,
but then have an active object immediately following it).
If you have a vector of 3 objects, all of them active, then removing the
second one will result in the vector containing an active object, followed by
an inactive object, followed by an active one. However, since the switch to
more properly use vectors instead of relying on this `active` system, there's
no longer any way for holes to exist in a vector. Properly removing an object
from a vector will just shift the rest of the objects down, so if we remove
the second object after the vector fix, then this will simply move the third
object into the slot of where the second object used to be.
So, what happens if you destroy a block and then create a new one in the
`active` system? Let's say that your `obj.blocks` looks like this, and here
I'm denoting each block by writing out its coordinates:
[30,60] [70,90] [80,100]
and that you want to update the position of the second one, because the entity
that that blocks belongs to has been updated. Okay, so, you delete that block,
which then makes things look like this:
[30,60] [-] [80,100]
and then afterwards, you create a new block with the updated position,
resulting in this:
[30,60] [74,90] [80,100]
Since `entityclass::createblock()` will find the first block slot that has a
false `active` attribute, it puts the new object in the same slot as the old
one. What has been essentially done here is that the slot of the block has
basically been reserved for the new block with the new position. Here, the
evaluation order of each block will stay the same.
But then 2.3 comes along and changes things up. So we start with an
`obj.blocks` like this again:
[30,60] [70,90] [80,100]
and we want to update the second block, like before. So we remove the second
block, resulting in this:
[30,60] [80,100]
It should be obvious that unlike before, where the third block stayed in the
third slot, the third block has now been moved to the second slot. But
continuing on; we are now going to create the new block with its updated
position, resulting in this:
[30,60] [80,100] [70,90]
At this point, we can see that the evaluation order of these blocks has been
changed due to the fact that the third block has now been moved to the slot
that was previously the slot of the second block.
So what can we do about this? Well, we can basically emulate what VVVVVV did
in 2.2, which is disable a block without actually removing it - except I'm not
going to reintroduce an `active` attribute or anything. I'll disable the
collision of all blocks at a certain position by setting their widths and
heights to 0, and then re-enable them later by finding the first block at that
same position, updating its position, and re-assigning its width and height
again.
The former is what `entityclass::nocollisionat()` does; the latter is what
`entityclass::moveblockto()` does. The former mimicks turning off the `active`
attribute of all blocks sharing a certain top-left corner; the latter mimicks
creating a new block - and it will only do this for one block, because
`entityclass::createblock()` in 2.2 only looked for the first block with a
false `active` attribute.
Now, some quirks relied on the previous behavior of destroying and creating
blocks, but all of these quirks have been preserved with the way I implemented
this fix.
The first quirk is that platforms passing through 0,0 will destroy all spike
hitboxes, script boxes, activity zones, and one-way hitboxes in the room. The
hitboxes of moving platforms, disappearing platforms, 1x1 quicksand, and
conveyors will not be affected.
This is a consequence of the fact that the former group uses the `x` and `y`
of their `rect`, while the latter group uses the `xp` and `yp` attributes. So
the `xp` and `yp` of the former are both 0. Meaning, a platform passing
through 0,0 destroys them all.
Having these separate coordinates seems like an artifact from the Flash days.
(And furthermore, there's an unused `x` and `y` attribute on all blocks,
making for technically three separate sets of coordinates! This should
probably be cleaned up, except for what I'm about to say...) But actually, if
you merge both sets of coordinates into one, this lets moving platforms
destroy script boxes and activity zones if it passes through the top-left
corner of them, which is probably far worse than the destruction being
localized to a specific coordinate that would never likely be reached
normally.
This quirk is preserved just fine without any special-casing, because instead
of destroying all blocks at 0,0, they just get disabled, which does the same
job. This quirk seems trivial to fix if I made it so that the position of a
platform's block was updated instantaneously instead of having one step to
disable it and another step to re-enable it, but I aim to preserve as much
quirks as possible.
The second quirk is that a moving platform passing through the top-left corner
of a disappearing platform or 1x1 quicksand will destroy the block of that
disappearing platform. This is because, again, when a moving platform updates,
it destroys all blocks at its previous position, not just only one block. This
is automatically preserved because this commit just disables the block of the
disappearing platform instead of removing it. Just like the last one, this
quirk seems extremely trivial to fix, and this time by simply making it so
`entityclass::nocollisionat()` would have a `break` statement, i.e. only
disabling the first block it finds instead of all blocks it finds, but I want
to keep all quirks that are possible to keep.
The last quirk is that, apparently, in order to prevent pushing the player
vertically out of a moving platform if they get inside of one, the game
destroys the block of the moving platform. If I had missed this edge case,
then the block would've been destroyed, leaving the moving platform with no
collision. But I caught it in my testing, so the block gets disabled instead
of destroyed. Also, it seems obtuse for those who don't understand why a
platform's block gets destroyed or disabled whenever the player collides with
it in `entityclass::collisioncheck()`, so I've put up a comment documenting it
as well.
The different platform evaluation order desyncs my Nova TAS, but after
applying this patchset and #504, my TAS syncs fine (save for the different
walkingframe from starting immediately on the ground instead of in the air
(#502), but that's minor and can be easily fixed).
I've attached a test level to the pull request for this commit (#503) to
demonstrate that this patchset not only fixes platform evaluation order, but
preserves some bugs and quirks with the existing block system.
The first room demonstrates the fixed platform evaluation order, by stepping
on the conveyors that both point into each other. In 2.2, Viridian will move
to the right of the background pillar, but in 2.3, Viridian will move to the
left of the pillar. However, after applying this patch, Viridian will now move
to the right of the pillar once again.
The second room demonstrates that the platform-passing-through-0,0 trick still
works (as explained above).
The last room demonstrates that platforms passing through the top-left corners
of disappearing platforms or 1x1 quicksand will remove the blocks of those
entities, causing Viridian to immediately pass through them. This trick is
still preserved after my patchset is applied.
2020-10-10 02:09:11 +02:00
|
|
|
int prevx = obj.entities[ie].xp;
|
|
|
|
|
int prevy = obj.entities[ie].yp;
|
Fix entity and block indices after destroying them
This patch restores some 2.2 behavior, fixing a regression caused by the
refactor of properly using std::vectors.
In 2.2, the game allocated 200 items in obj.entities, but used a system
where each entity had an `active` attribute to signify if the entity
actually existed or not. When dealing with entities, you would have to
check this `active` flag, or else you'd be dealing with an entity that
didn't actually exist. (By the way, what I'm saying applies to blocks
and obj.blocks as well, except for some small differing details like the
game allocating 500 block slots versus obj.entities's 200.)
As a consequence, the game had to use a separate tracking variable,
obj.nentity, because obj.entities.size() would just report 200, instead
of the actual amount of entities. Needless to say, having to check for
`active` and use `obj.nentity` is a bit error-prone, and it's messier
than simply using the std::vector the way it was intended. Also, this
resulted in a hard limit of 200 entities, which custom level makers ran
into surprisingly quite often.
2.3 comes along, and removes the whole system. Now, std::vectors are
properly being used, and obj.entities.size() reports the actual number
of entities in the vector; you no longer have to check for `active` when
dealing with entities of any sort.
But there was one previous behavior of 2.2 that this system kind of
forgets about - namely, the ability to have holes in between entities.
You see, when an entity got disabled in 2.2 (which just meant turning
its `active` off), the indices of all other entities stayed the same;
the indice of the entity that got disabled stays there as a hole in the
array. But when an entity gets removed in 2.3 (previous to this patch),
the indices of every entity afterwards in the array get shifted down by
one. std::vector isn't really meant to be able to contain holes.
Do the indices of entities and blocks matter? Yes; they determine the
order in which entities and blocks get evaluated (the highest indice
gets evaluated first), and I had to fix some block evaluation order
stuff in previous PRs.
And in the case of entities, they matter hugely when using the
recently-discovered Arbitrary Entity Manipulation glitch (where crewmate
script commands are used on arbitrary entities by setting the `i`
attribute of `scriptclass` and passing invalid crewmate identifiers to
the commands). If you use Arbitrary Entity Manipulation after destroying
some entities, there is a chance that your script won't work between 2.2
and 2.3.
The indices also still determine the rendering order of entities
(highest indice gets drawn first, which means lowest indice gets drawn
in front of other entities). As an example: let's say we have the player
at 0, a gravity line at 1, and a checkpoint at 2; then we destroy the
gravity line and create a crewmate (let's do Violet).
If we're able to have holes, then after removing the gravity line, none
of the other indices shift. Then Violet will be created at indice 1, and
will be drawn in front of the checkpoint.
But if we can't have holes, then removing the gravity line results in
the indice of the checkpoint shifting down to indice 1. Then Violet is
created at indice 2, and gets drawn behind the checkpoint! This is a
clear illustration of changing the behavior that existed in 2.2.
However, I also don't want to go back to the `active` system of having
to check an attribute before operating on an entity. So... what do we
do to restore the holes?
Well, we don't need to have an `active` attribute, or modify any
existing code that operates on entities. Instead, we can just set the
attributes of the entities so that they naturally get ignored by
everything that comes into contact with it. For entities, we set their
invis to true, and their size, type, and rule to -1 (the game never uses
a size, type, or rule of -1 anywhere); for blocks, we set their type to
-1, and their width and height to 0.
obj.entities.size() will no longer necessarily equal the amount of
entities in the room; rather, it will be the amount of entity SLOTS that
have been allocated. But nothing that uses obj.entities.size() needs to
actually know the amount of entities; it's mostly used for iterating
over every entity in the vector.
Excess entity slots get cleaned up upon every call of
mapclass::gotoroom(), which will now deallocate entity slots starting
from the end until it hits a player, at which point it will switch to
disabling entity slots instead of removing them entirely.
The entclass::clear() and blockclass::clear() functions have been
restored because we need to call their initialization functions when
reusing a block/entity slot; it's possible to create an entity with an
invalid type number (it creates a glitchy Viridian), and without calling
the initialization function again, it would simply not create anything.
After this patch is applied, entity and block indices will be restored
to how they behaved in 2.2.
2020-12-27 07:11:34 +01:00
|
|
|
obj.disableblockat(prevx, prevy);
|
2020-01-01 21:29:24 +01:00
|
|
|
|
2020-10-07 01:16:48 +02:00
|
|
|
bool entitygone = obj.updateentities(ie); // Behavioral logic
|
|
|
|
|
if (entitygone) continue;
|
|
|
|
|
obj.updateentitylogic(ie); // Basic Physics
|
|
|
|
|
obj.entitymapcollision(ie); // Collisions with walls
|
2020-01-01 21:29:24 +01:00
|
|
|
|
Restore platform evaluation order to 2.2
This commit restores the evaluation order of moving platforms and conveyors to
be what it was in 2.2. The evaluation order changed in 2.3 after the patchset
to improve the handling of the `obj.entities` and `obj.blocks` vectors (#191).
By evaluation order, I'm talking about the order in which platforms and
conveyors will be evaluated (and thus will take priority) if Viridian stands
on both a conveyor or platform at once, and they either have different speeds
or are pointing in different directions. Nowhere in the main game is there a
place where you can stand on two different conveyors/platforms at once, so
this is solely within the territory of custom levels, which is my specialty.
So what caused this evaluation order to change? Well, every moving platform
and conveyor in the game is actually made up of two objects: an entity, and a
block. The entity is the part that moves around, and the block is the part
that actually has the collision.
But if the entity is the part that moves around, and entities and blocks are
in entirely separate vectors, how is the block part going to move along with
it? Well, maybe you'd guess some sort of unique ID system, but spend some time
digging around the code and you won't find any trace of any (there's no
attribute on an entity to store such an ID, for starters).
Instead, what the game does is actually remove all blocks that coincide with
the exact top-left corner of the entity, and then create a new one. Destroying
and creating blocks like this all the time is hugely wasteful, but hey, it
worked.
So why did the evaluation order change in 2.3? Well, to understand that,
you'll need to understand 2.2's `active` system. Instead of having an object
be real simply by virtue of it existing, 2.2 had this system where the object
was only real if it had its `active` attribute set to true. In other words,
you would be looking at a fake object that didn't actually exist if its
`active` attribute was false.
On the surface, this doesn't seem that bad. But this can lead to "holes" in a
given vector of objects. A hole is simply an inactive object neighbored by
active objects (or the inactive object could be the first one in the vector,
but then have an active object immediately following it).
If you have a vector of 3 objects, all of them active, then removing the
second one will result in the vector containing an active object, followed by
an inactive object, followed by an active one. However, since the switch to
more properly use vectors instead of relying on this `active` system, there's
no longer any way for holes to exist in a vector. Properly removing an object
from a vector will just shift the rest of the objects down, so if we remove
the second object after the vector fix, then this will simply move the third
object into the slot of where the second object used to be.
So, what happens if you destroy a block and then create a new one in the
`active` system? Let's say that your `obj.blocks` looks like this, and here
I'm denoting each block by writing out its coordinates:
[30,60] [70,90] [80,100]
and that you want to update the position of the second one, because the entity
that that blocks belongs to has been updated. Okay, so, you delete that block,
which then makes things look like this:
[30,60] [-] [80,100]
and then afterwards, you create a new block with the updated position,
resulting in this:
[30,60] [74,90] [80,100]
Since `entityclass::createblock()` will find the first block slot that has a
false `active` attribute, it puts the new object in the same slot as the old
one. What has been essentially done here is that the slot of the block has
basically been reserved for the new block with the new position. Here, the
evaluation order of each block will stay the same.
But then 2.3 comes along and changes things up. So we start with an
`obj.blocks` like this again:
[30,60] [70,90] [80,100]
and we want to update the second block, like before. So we remove the second
block, resulting in this:
[30,60] [80,100]
It should be obvious that unlike before, where the third block stayed in the
third slot, the third block has now been moved to the second slot. But
continuing on; we are now going to create the new block with its updated
position, resulting in this:
[30,60] [80,100] [70,90]
At this point, we can see that the evaluation order of these blocks has been
changed due to the fact that the third block has now been moved to the slot
that was previously the slot of the second block.
So what can we do about this? Well, we can basically emulate what VVVVVV did
in 2.2, which is disable a block without actually removing it - except I'm not
going to reintroduce an `active` attribute or anything. I'll disable the
collision of all blocks at a certain position by setting their widths and
heights to 0, and then re-enable them later by finding the first block at that
same position, updating its position, and re-assigning its width and height
again.
The former is what `entityclass::nocollisionat()` does; the latter is what
`entityclass::moveblockto()` does. The former mimicks turning off the `active`
attribute of all blocks sharing a certain top-left corner; the latter mimicks
creating a new block - and it will only do this for one block, because
`entityclass::createblock()` in 2.2 only looked for the first block with a
false `active` attribute.
Now, some quirks relied on the previous behavior of destroying and creating
blocks, but all of these quirks have been preserved with the way I implemented
this fix.
The first quirk is that platforms passing through 0,0 will destroy all spike
hitboxes, script boxes, activity zones, and one-way hitboxes in the room. The
hitboxes of moving platforms, disappearing platforms, 1x1 quicksand, and
conveyors will not be affected.
This is a consequence of the fact that the former group uses the `x` and `y`
of their `rect`, while the latter group uses the `xp` and `yp` attributes. So
the `xp` and `yp` of the former are both 0. Meaning, a platform passing
through 0,0 destroys them all.
Having these separate coordinates seems like an artifact from the Flash days.
(And furthermore, there's an unused `x` and `y` attribute on all blocks,
making for technically three separate sets of coordinates! This should
probably be cleaned up, except for what I'm about to say...) But actually, if
you merge both sets of coordinates into one, this lets moving platforms
destroy script boxes and activity zones if it passes through the top-left
corner of them, which is probably far worse than the destruction being
localized to a specific coordinate that would never likely be reached
normally.
This quirk is preserved just fine without any special-casing, because instead
of destroying all blocks at 0,0, they just get disabled, which does the same
job. This quirk seems trivial to fix if I made it so that the position of a
platform's block was updated instantaneously instead of having one step to
disable it and another step to re-enable it, but I aim to preserve as much
quirks as possible.
The second quirk is that a moving platform passing through the top-left corner
of a disappearing platform or 1x1 quicksand will destroy the block of that
disappearing platform. This is because, again, when a moving platform updates,
it destroys all blocks at its previous position, not just only one block. This
is automatically preserved because this commit just disables the block of the
disappearing platform instead of removing it. Just like the last one, this
quirk seems extremely trivial to fix, and this time by simply making it so
`entityclass::nocollisionat()` would have a `break` statement, i.e. only
disabling the first block it finds instead of all blocks it finds, but I want
to keep all quirks that are possible to keep.
The last quirk is that, apparently, in order to prevent pushing the player
vertically out of a moving platform if they get inside of one, the game
destroys the block of the moving platform. If I had missed this edge case,
then the block would've been destroyed, leaving the moving platform with no
collision. But I caught it in my testing, so the block gets disabled instead
of destroyed. Also, it seems obtuse for those who don't understand why a
platform's block gets destroyed or disabled whenever the player collides with
it in `entityclass::collisioncheck()`, so I've put up a comment documenting it
as well.
The different platform evaluation order desyncs my Nova TAS, but after
applying this patchset and #504, my TAS syncs fine (save for the different
walkingframe from starting immediately on the ground instead of in the air
(#502), but that's minor and can be easily fixed).
I've attached a test level to the pull request for this commit (#503) to
demonstrate that this patchset not only fixes platform evaluation order, but
preserves some bugs and quirks with the existing block system.
The first room demonstrates the fixed platform evaluation order, by stepping
on the conveyors that both point into each other. In 2.2, Viridian will move
to the right of the background pillar, but in 2.3, Viridian will move to the
left of the pillar. However, after applying this patch, Viridian will now move
to the right of the pillar once again.
The second room demonstrates that the platform-passing-through-0,0 trick still
works (as explained above).
The last room demonstrates that platforms passing through the top-left corners
of disappearing platforms or 1x1 quicksand will remove the blocks of those
entities, causing Viridian to immediately pass through them. This trick is
still preserved after my patchset is applied.
2020-10-10 02:09:11 +02:00
|
|
|
obj.moveblockto(prevx, prevy, obj.entities[ie].xp, obj.entities[ie].yp, obj.entities[ie].w, obj.entities[ie].h);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
//is the player standing on a moving platform?
|
|
|
|
|
int i = obj.getplayer();
|
2020-03-31 02:46:36 +02:00
|
|
|
float j = obj.entitycollideplatformfloor(i);
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities) && j > -1000)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
obj.entities[i].newxp = obj.entities[i].xp + j;
|
2020-03-31 02:46:36 +02:00
|
|
|
obj.entitymapcollision(i);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
2020-03-31 02:46:36 +02:00
|
|
|
j = obj.entitycollideplatformroof(i);
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities) && j > -1000)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
obj.entities[i].newxp = obj.entities[i].xp + j;
|
2020-03-31 02:46:36 +02:00
|
|
|
obj.entitymapcollision(i);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2020-04-03 22:50:16 +02:00
|
|
|
for (int ie = obj.entities.size() - 1; ie >= 0; ie--)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-10-07 01:15:44 +02:00
|
|
|
if (obj.entities[ie].isplatform)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-10-07 01:15:44 +02:00
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
|
2020-10-07 01:16:48 +02:00
|
|
|
bool entitygone = obj.updateentities(ie); // Behavioral logic
|
|
|
|
|
if (entitygone) continue;
|
|
|
|
|
obj.updateentitylogic(ie); // Basic Physics
|
|
|
|
|
obj.entitymapcollision(ie); // Collisions with walls
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
|
2020-03-31 02:46:36 +02:00
|
|
|
obj.entitycollisioncheck(); // Check ent v ent collisions, update states
|
2020-04-25 06:19:41 +02:00
|
|
|
|
|
|
|
|
if (map.towermode)
|
|
|
|
|
{
|
|
|
|
|
//special for tower: is the player touching any spike blocks?
|
|
|
|
|
int player = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if(INBOUNDS_VEC(player, obj.entities) && obj.checktowerspikes(player) && graphics.fademode==0)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
|
|
|
|
game.deathseq = 30;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if(map.towermode && game.lifeseq==0)
|
|
|
|
|
{
|
|
|
|
|
int player = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if(!map.invincibility && INBOUNDS_VEC(player, obj.entities))
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
|
|
|
|
if (obj.entities[player].yp-map.ypos <= 0)
|
|
|
|
|
{
|
|
|
|
|
game.deathseq = 30;
|
|
|
|
|
}
|
|
|
|
|
else if (obj.entities[player].yp-map.ypos >= 208)
|
|
|
|
|
{
|
|
|
|
|
game.deathseq = 30;
|
|
|
|
|
}
|
|
|
|
|
}
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
else if (INBOUNDS_VEC(player, obj.entities))
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
2021-03-28 21:53:08 +02:00
|
|
|
const bool above_screen = obj.entities[player].yp-map.ypos <= 8;
|
|
|
|
|
const bool below_screen = obj.entities[player].yp-map.ypos >= 200;
|
|
|
|
|
|
|
|
|
|
if (above_screen)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
2021-03-28 21:53:08 +02:00
|
|
|
if (obj.entities[player].yp - map.ypos <= 0)
|
|
|
|
|
{
|
|
|
|
|
if (graphics.towerbg.scrolldir == 1)
|
|
|
|
|
{
|
|
|
|
|
/* Descending tower:
|
|
|
|
|
* Counteract 10 pixels of terminal velocity
|
|
|
|
|
* + 2 pixels of camera movement */
|
|
|
|
|
map.ypos -= 12;
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
/* Ascending tower:
|
|
|
|
|
* Move 8 out of 10 pixels of terminal velocity
|
|
|
|
|
* Camera movement will move 2 pixels for us */
|
|
|
|
|
map.ypos -= 8;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
/* Counter 2 pixels of camera movement */
|
|
|
|
|
map.ypos -= 2;
|
|
|
|
|
}
|
2020-04-25 06:19:41 +02:00
|
|
|
}
|
2021-03-28 21:53:08 +02:00
|
|
|
else if (below_screen)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
2021-03-28 21:53:08 +02:00
|
|
|
if (obj.entities[player].yp - map.ypos >= 208)
|
|
|
|
|
{
|
|
|
|
|
if (graphics.towerbg.scrolldir == 0)
|
|
|
|
|
{
|
|
|
|
|
/* Ascending tower:
|
|
|
|
|
* Counteract 10 pixels of terminal velocity
|
|
|
|
|
* + 2 pixels of camera movement */
|
|
|
|
|
map.ypos += 12;
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
/* Descending tower:
|
|
|
|
|
* Move 8 out of 10 pixels of terminal velocity
|
|
|
|
|
* Camera movement will move 2 pixels for us */
|
|
|
|
|
map.ypos += 8;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
/* Counter 2 pixels of camera movement */
|
|
|
|
|
map.ypos += 2;
|
|
|
|
|
}
|
2021-03-28 21:53:08 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if (above_screen || below_screen)
|
|
|
|
|
{
|
|
|
|
|
/* The buffer isn't big enough; we have to redraw */
|
|
|
|
|
graphics.towerbg.tdrawback = true;
|
2020-04-25 06:19:41 +02:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(player, obj.entities) && obj.entities[player].yp - map.ypos <= 40)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
|
|
|
|
map.spikeleveltop++;
|
|
|
|
|
if (map.spikeleveltop >= 8) map.spikeleveltop = 8;
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
if (map.spikeleveltop > 0) map.spikeleveltop--;
|
|
|
|
|
}
|
|
|
|
|
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(player, obj.entities) && obj.entities[player].yp - map.ypos >= 164)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
|
|
|
|
map.spikelevelbottom++;
|
|
|
|
|
if (map.spikelevelbottom >= 8) map.spikelevelbottom = 8;
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
if (map.spikelevelbottom > 0) map.spikelevelbottom--;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
//Using warplines?
|
2020-04-02 22:44:45 +02:00
|
|
|
if (obj.customwarpmode) {
|
Split glitchrunner mode into multiple versions
Previously, turning glitchrunner mode on essentially locked you to
emulating 2.0, and turning it off just meant normal 2.3 behavior. But
what if you wanted 2.2 behavior instead? Well, that's what I had to ask
when a TAS of mine would desync in 2.3 because of the two-frame delay
fix (glitchrunner off), but would also desync because of 2.0 warp lines
(glitchrunner on).
What I've done is made it so there are three states to glitchrunner mode
now: 2.0 (previously just the "on" state), 2.2 (previously a state you
couldn't use), and "off". Furthermore, I made it an enum, so in case
future versions of the game patch out more glitches, we can add them to
the enum (and the only other thing we have to update is a lookup table
in GlitchrunnerMode.c). Also, 2.2 glitches exist in 2.0, so you'll want
to use GlitchrunnerMode_less_than_or_equal() to check glitchrunner
version.
2021-08-05 02:09:49 +02:00
|
|
|
if (!GlitchrunnerMode_less_than_or_equal(Glitchrunner2_0)) {
|
Restore pre-2.1 warp bypass glitch in glitchrunner mode
So, I was staring at VVVVVV code one day, as I usually do, and I noticed
that warp lines had this curious code in entityclass::updateentities()
that set their statedelay to 2, and I thought, hm, maybe the pre-2.1
warp line bypass is caused by this statedelay. And, it doesn't seem like
this is the primary code used to detect if the player collides with warp
lines, the actual code is commented with "Rewritten system for mobile
update" and bolted-on in gamelogic() instead of properly being in
entityclass::entitycollisioncheck().
So, after getting tripped up on the misleading indentation of that
"Rewritten system" block, I removed the rewritten system, re-added
collision detection for rule 7 (horizontal warp lines), and after
checking the resulting behavior, it appears to be nearly identical to
that of warp lines in 2.0.
You see, if you use warp lines to flip up from the top of the screen
onto the bottom of the screen, close to the edge of the bottom of the
screen, Viridian's head will display on the top of the screen in 2.0. In
2.1 and later, this doesn't happen, confirming that my theory is
correct. I also performed warp line bypass multiple times in 2.0 and
with my restored code, and it is pretty much the exact same behavior.
So now, the pre-2.1 warp line bypass glitch has been re-enabled in
glitchrunner mode.
2020-09-10 01:10:16 +02:00
|
|
|
//Rewritten system for mobile update: basically, the new logic is to
|
|
|
|
|
//check if the player is leaving the map, and if so do a special check against
|
|
|
|
|
//warp lines for collision
|
|
|
|
|
obj.customwarpmodehon = false;
|
|
|
|
|
obj.customwarpmodevon = false;
|
2020-04-02 22:44:45 +02:00
|
|
|
|
Restore pre-2.1 warp bypass glitch in glitchrunner mode
So, I was staring at VVVVVV code one day, as I usually do, and I noticed
that warp lines had this curious code in entityclass::updateentities()
that set their statedelay to 2, and I thought, hm, maybe the pre-2.1
warp line bypass is caused by this statedelay. And, it doesn't seem like
this is the primary code used to detect if the player collides with warp
lines, the actual code is commented with "Rewritten system for mobile
update" and bolted-on in gamelogic() instead of properly being in
entityclass::entitycollisioncheck().
So, after getting tripped up on the misleading indentation of that
"Rewritten system" block, I removed the rewritten system, re-added
collision detection for rule 7 (horizontal warp lines), and after
checking the resulting behavior, it appears to be nearly identical to
that of warp lines in 2.0.
You see, if you use warp lines to flip up from the top of the screen
onto the bottom of the screen, close to the edge of the bottom of the
screen, Viridian's head will display on the top of the screen in 2.0. In
2.1 and later, this doesn't happen, confirming that my theory is
correct. I also performed warp line bypass multiple times in 2.0 and
with my restored code, and it is pretty much the exact same behavior.
So now, the pre-2.1 warp line bypass glitch has been re-enabled in
glitchrunner mode.
2020-09-10 01:10:16 +02:00
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities) && ((game.door_down > -2 && obj.entities[i].yp >= 226-16) || (game.door_up > -2 && obj.entities[i].yp < -2+16) || (game.door_left > -2 && obj.entities[i].xp < -14+16) || (game.door_right > -2 && obj.entities[i].xp >= 308-16))){
|
Restore pre-2.1 warp bypass glitch in glitchrunner mode
So, I was staring at VVVVVV code one day, as I usually do, and I noticed
that warp lines had this curious code in entityclass::updateentities()
that set their statedelay to 2, and I thought, hm, maybe the pre-2.1
warp line bypass is caused by this statedelay. And, it doesn't seem like
this is the primary code used to detect if the player collides with warp
lines, the actual code is commented with "Rewritten system for mobile
update" and bolted-on in gamelogic() instead of properly being in
entityclass::entitycollisioncheck().
So, after getting tripped up on the misleading indentation of that
"Rewritten system" block, I removed the rewritten system, re-added
collision detection for rule 7 (horizontal warp lines), and after
checking the resulting behavior, it appears to be nearly identical to
that of warp lines in 2.0.
You see, if you use warp lines to flip up from the top of the screen
onto the bottom of the screen, close to the edge of the bottom of the
screen, Viridian's head will display on the top of the screen in 2.0. In
2.1 and later, this doesn't happen, confirming that my theory is
correct. I also performed warp line bypass multiple times in 2.0 and
with my restored code, and it is pretty much the exact same behavior.
So now, the pre-2.1 warp line bypass glitch has been re-enabled in
glitchrunner mode.
2020-09-10 01:10:16 +02:00
|
|
|
//Player is leaving room
|
|
|
|
|
obj.customwarplinecheck(i);
|
|
|
|
|
}
|
2020-09-10 00:45:03 +02:00
|
|
|
}
|
2020-04-02 22:44:45 +02:00
|
|
|
|
2020-09-10 00:45:03 +02:00
|
|
|
if(obj.customwarpmodehon){ map.warpy=true;
|
|
|
|
|
}else{ map.warpy=false; }
|
|
|
|
|
if(obj.customwarpmodevon){ map.warpx=true;
|
|
|
|
|
}else{ map.warpx=false; }
|
2020-04-02 22:44:45 +02:00
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
|
|
|
|
|
//Finally: Are we changing room?
|
2020-04-25 06:19:41 +02:00
|
|
|
if (map.warpx && !map.towermode)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2021-04-23 00:31:23 +02:00
|
|
|
size_t i;
|
|
|
|
|
for (i = 0; i < obj.entities.size(); ++i)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
Fix rescuable crewmates not warping
The game excluded every entity whose `type` was 50 or higher. The `type`
of rescuable crewmates is 55.
Could some levels be broken by this behavior? Unlikely; without warping,
the crewmates would end up falling out of the room and would become
unrescuable. So this is more likely to fix than to break.
But more importantly, *no one knows that rescuable crewmates don't
warp*. If anyone would know, it would be me, because I've been in the
custom levels community for over 7 years - and yet, during that time, I
have not seen anyone run into this corner case. If they did, I would
remember! This implies that people simply have never thought about
putting rescuable crewmates in places where they would warp - or they
have, ran into this issue, and worked around it.
With those two reasons, I'm comfortable fixing this inconsistency.
2021-04-23 00:41:09 +02:00
|
|
|
if ((obj.entities[i].type >= 51
|
|
|
|
|
&& obj.entities[i].type <= 54) /* Don't warp warp lines */
|
2021-04-23 00:47:57 +02:00
|
|
|
|| obj.entities[i].size == 12) /* Don't warp gravitron squares */
|
2020-04-04 21:16:16 +02:00
|
|
|
{
|
2021-04-23 00:36:16 +02:00
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
|
2021-04-23 00:38:29 +02:00
|
|
|
if (game.roomx == 118 && game.roomy == 102 && obj.entities[i].rule==1 && !map.custommode)
|
|
|
|
|
{
|
|
|
|
|
//ascii snakes
|
|
|
|
|
if (obj.entities[i].xp <= -80)
|
2020-04-04 21:13:24 +02:00
|
|
|
{
|
2021-04-23 00:38:29 +02:00
|
|
|
if (obj.entities[i].isplatform)
|
2020-04-04 21:13:24 +02:00
|
|
|
{
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.moveblockto(obj.entities[i].xp, obj.entities[i].yp, obj.entities[i].xp + 400, obj.entities[i].yp, obj.entities[i].w, obj.entities[i].h);
|
2020-04-04 21:13:24 +02:00
|
|
|
}
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.entities[i].xp += 400;
|
|
|
|
|
obj.entities[i].lerpoldxp += 400;
|
|
|
|
|
}
|
|
|
|
|
else if (obj.entities[i].xp > 320)
|
|
|
|
|
{
|
|
|
|
|
if (obj.entities[i].isplatform)
|
2020-04-04 21:13:24 +02:00
|
|
|
{
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.moveblockto(obj.entities[i].xp, obj.entities[i].yp, obj.entities[i].xp - 400, obj.entities[i].yp, obj.entities[i].w, obj.entities[i].h);
|
2020-04-04 21:16:16 +02:00
|
|
|
}
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.entities[i].xp -= 400;
|
|
|
|
|
obj.entities[i].lerpoldxp -= 400;
|
2020-04-04 21:16:16 +02:00
|
|
|
}
|
2021-04-23 00:38:29 +02:00
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
if (obj.entities[i].xp <= -10)
|
2020-04-04 21:16:16 +02:00
|
|
|
{
|
2021-04-23 00:38:29 +02:00
|
|
|
if (obj.entities[i].isplatform)
|
2020-04-04 21:16:16 +02:00
|
|
|
{
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.moveblockto(obj.entities[i].xp, obj.entities[i].yp, obj.entities[i].xp + 320, obj.entities[i].yp, obj.entities[i].w, obj.entities[i].h);
|
2020-04-04 21:16:16 +02:00
|
|
|
}
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.entities[i].xp += 320;
|
|
|
|
|
obj.entities[i].lerpoldxp += 320;
|
|
|
|
|
}
|
|
|
|
|
else if (obj.entities[i].xp > 310)
|
|
|
|
|
{
|
|
|
|
|
if (obj.entities[i].isplatform)
|
2020-04-04 21:16:16 +02:00
|
|
|
{
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.moveblockto(obj.entities[i].xp, obj.entities[i].yp, obj.entities[i].xp - 320, obj.entities[i].yp, obj.entities[i].w, obj.entities[i].h);
|
2020-04-04 21:13:24 +02:00
|
|
|
}
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.entities[i].xp -= 320;
|
|
|
|
|
obj.entities[i].lerpoldxp -= 320;
|
2020-04-04 21:13:24 +02:00
|
|
|
}
|
2021-04-23 00:38:29 +02:00
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
De-dupe screen transition / warping logic
I noticed that the code for going to the adjacent room when offscreen
and for warping instead if the room is warping was a bit
copy-and-pasted. To clean up the code a bit, there's now 5 separate
checks in gamelogic():
if (map.warpx)
if (map.warpy)
if (map.warpy && !map.warpx)
if (!map.warpy)
if (!map.warpx)
I made sure to preserve the previous weird horizontal warping behavior
that happens with vertical warping (thus the middle one), and to
preserve the frame ordering just in case there's something dependent on
the frame ordering.
The frame ordering is that first it will warp you horizontally, if
applicable, then it will warp you vertically, if applicable. Then if you
have vertical warping only, that weird horizontal warp. Then it will
screen transition you vertically, if applicable. Then it will screen
transition you horizontally, if applicable.
To explain the weird horizontal warp with the vertical warp: apparently
if an entity is far offscreen enough, and if that entity is not the
player, it will be warped horizontally during a vertical warp. The
points at which it will warp is 30 pixels farther out than normal
horizontal warping.
I think someone ran into this before, but my memory is fuzzy. The best I
can recall is that they were probably createentity()ing a high-speed
horizontally-moving enemy in a vertically warping room, only to discover
that said enemy kept warping horizontally.
2020-04-04 21:39:17 +02:00
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
if (map.warpy && !map.towermode)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2021-04-23 00:31:23 +02:00
|
|
|
size_t i;
|
|
|
|
|
for (i = 0; i < obj.entities.size(); ++i)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
Fix rescuable crewmates not warping
The game excluded every entity whose `type` was 50 or higher. The `type`
of rescuable crewmates is 55.
Could some levels be broken by this behavior? Unlikely; without warping,
the crewmates would end up falling out of the room and would become
unrescuable. So this is more likely to fix than to break.
But more importantly, *no one knows that rescuable crewmates don't
warp*. If anyone would know, it would be me, because I've been in the
custom levels community for over 7 years - and yet, during that time, I
have not seen anyone run into this corner case. If they did, I would
remember! This implies that people simply have never thought about
putting rescuable crewmates in places where they would warp - or they
have, ran into this issue, and worked around it.
With those two reasons, I'm comfortable fixing this inconsistency.
2021-04-23 00:41:09 +02:00
|
|
|
if (obj.entities[i].type >= 51
|
|
|
|
|
&& obj.entities[i].type <= 54) /* Don't warp warp lines */
|
2021-04-23 00:36:16 +02:00
|
|
|
{
|
|
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
|
2021-04-23 00:38:29 +02:00
|
|
|
if (obj.entities[i].yp <= -12)
|
|
|
|
|
{
|
|
|
|
|
if (obj.entities[i].isplatform)
|
2020-04-04 21:13:24 +02:00
|
|
|
{
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.moveblockto(obj.entities[i].xp, obj.entities[i].yp, obj.entities[i].xp, obj.entities[i].yp + 232, obj.entities[i].w, obj.entities[i].h);
|
2020-04-04 21:13:24 +02:00
|
|
|
}
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.entities[i].yp += 232;
|
|
|
|
|
obj.entities[i].lerpoldyp += 232;
|
|
|
|
|
}
|
|
|
|
|
else if (obj.entities[i].yp > 226)
|
|
|
|
|
{
|
|
|
|
|
if (obj.entities[i].isplatform)
|
2020-04-04 21:13:24 +02:00
|
|
|
{
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.moveblockto(obj.entities[i].xp, obj.entities[i].yp, obj.entities[i].xp, obj.entities[i].yp - 232, obj.entities[i].w, obj.entities[i].h);
|
2020-04-04 21:13:24 +02:00
|
|
|
}
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.entities[i].yp -= 232;
|
|
|
|
|
obj.entities[i].lerpoldyp -= 232;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
De-dupe screen transition / warping logic
I noticed that the code for going to the adjacent room when offscreen
and for warping instead if the room is warping was a bit
copy-and-pasted. To clean up the code a bit, there's now 5 separate
checks in gamelogic():
if (map.warpx)
if (map.warpy)
if (map.warpy && !map.warpx)
if (!map.warpy)
if (!map.warpx)
I made sure to preserve the previous weird horizontal warping behavior
that happens with vertical warping (thus the middle one), and to
preserve the frame ordering just in case there's something dependent on
the frame ordering.
The frame ordering is that first it will warp you horizontally, if
applicable, then it will warp you vertically, if applicable. Then if you
have vertical warping only, that weird horizontal warp. Then it will
screen transition you vertically, if applicable. Then it will screen
transition you horizontally, if applicable.
To explain the weird horizontal warp with the vertical warp: apparently
if an entity is far offscreen enough, and if that entity is not the
player, it will be warped horizontally during a vertical warp. The
points at which it will warp is 30 pixels farther out than normal
horizontal warping.
I think someone ran into this before, but my memory is fuzzy. The best I
can recall is that they were probably createentity()ing a high-speed
horizontally-moving enemy in a vertically warping room, only to discover
that said enemy kept warping horizontally.
2020-04-04 21:39:17 +02:00
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
if (map.warpy && !map.warpx && !map.towermode)
|
De-dupe screen transition / warping logic
I noticed that the code for going to the adjacent room when offscreen
and for warping instead if the room is warping was a bit
copy-and-pasted. To clean up the code a bit, there's now 5 separate
checks in gamelogic():
if (map.warpx)
if (map.warpy)
if (map.warpy && !map.warpx)
if (!map.warpy)
if (!map.warpx)
I made sure to preserve the previous weird horizontal warping behavior
that happens with vertical warping (thus the middle one), and to
preserve the frame ordering just in case there's something dependent on
the frame ordering.
The frame ordering is that first it will warp you horizontally, if
applicable, then it will warp you vertically, if applicable. Then if you
have vertical warping only, that weird horizontal warp. Then it will
screen transition you vertically, if applicable. Then it will screen
transition you horizontally, if applicable.
To explain the weird horizontal warp with the vertical warp: apparently
if an entity is far offscreen enough, and if that entity is not the
player, it will be warped horizontally during a vertical warp. The
points at which it will warp is 30 pixels farther out than normal
horizontal warping.
I think someone ran into this before, but my memory is fuzzy. The best I
can recall is that they were probably createentity()ing a high-speed
horizontally-moving enemy in a vertically warping room, only to discover
that said enemy kept warping horizontally.
2020-04-04 21:39:17 +02:00
|
|
|
{
|
2021-04-23 00:31:23 +02:00
|
|
|
size_t i;
|
|
|
|
|
for (i = 0; i < obj.entities.size(); ++i)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
Fix rescuable crewmates not warping
The game excluded every entity whose `type` was 50 or higher. The `type`
of rescuable crewmates is 55.
Could some levels be broken by this behavior? Unlikely; without warping,
the crewmates would end up falling out of the room and would become
unrescuable. So this is more likely to fix than to break.
But more importantly, *no one knows that rescuable crewmates don't
warp*. If anyone would know, it would be me, because I've been in the
custom levels community for over 7 years - and yet, during that time, I
have not seen anyone run into this corner case. If they did, I would
remember! This implies that people simply have never thought about
putting rescuable crewmates in places where they would warp - or they
have, ran into this issue, and worked around it.
With those two reasons, I'm comfortable fixing this inconsistency.
2021-04-23 00:41:09 +02:00
|
|
|
if ((obj.entities[i].type >= 51
|
|
|
|
|
&& obj.entities[i].type <= 54) /* Don't warp warp lines */
|
2021-04-23 00:36:16 +02:00
|
|
|
|| obj.entities[i].rule == 0) /* Don't warp the player */
|
2020-04-04 21:16:16 +02:00
|
|
|
{
|
2021-04-23 00:36:16 +02:00
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
|
2021-04-23 00:38:29 +02:00
|
|
|
if (obj.entities[i].xp <= -30)
|
|
|
|
|
{
|
|
|
|
|
if (obj.entities[i].isplatform)
|
2020-04-04 21:13:24 +02:00
|
|
|
{
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.moveblockto(obj.entities[i].xp, obj.entities[i].yp, obj.entities[i].xp + 350, obj.entities[i].yp, obj.entities[i].w, obj.entities[i].h);
|
2020-04-04 21:16:16 +02:00
|
|
|
}
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.entities[i].xp += 350;
|
|
|
|
|
obj.entities[i].lerpoldxp += 350;
|
|
|
|
|
}
|
|
|
|
|
else if (obj.entities[i].xp > 320)
|
|
|
|
|
{
|
|
|
|
|
if (obj.entities[i].isplatform)
|
2020-04-04 21:16:16 +02:00
|
|
|
{
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.moveblockto(obj.entities[i].xp, obj.entities[i].yp, obj.entities[i].xp - 350, obj.entities[i].yp, obj.entities[i].w, obj.entities[i].h);
|
2020-04-04 21:13:24 +02:00
|
|
|
}
|
2021-04-23 00:38:29 +02:00
|
|
|
obj.entities[i].xp -= 350;
|
|
|
|
|
obj.entities[i].lerpoldxp -= 350;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
De-dupe screen transition / warping logic
I noticed that the code for going to the adjacent room when offscreen
and for warping instead if the room is warping was a bit
copy-and-pasted. To clean up the code a bit, there's now 5 separate
checks in gamelogic():
if (map.warpx)
if (map.warpy)
if (map.warpy && !map.warpx)
if (!map.warpy)
if (!map.warpx)
I made sure to preserve the previous weird horizontal warping behavior
that happens with vertical warping (thus the middle one), and to
preserve the frame ordering just in case there's something dependent on
the frame ordering.
The frame ordering is that first it will warp you horizontally, if
applicable, then it will warp you vertically, if applicable. Then if you
have vertical warping only, that weird horizontal warp. Then it will
screen transition you vertically, if applicable. Then it will screen
transition you horizontally, if applicable.
To explain the weird horizontal warp with the vertical warp: apparently
if an entity is far offscreen enough, and if that entity is not the
player, it will be warped horizontally during a vertical warp. The
points at which it will warp is 30 pixels farther out than normal
horizontal warping.
I think someone ran into this before, but my memory is fuzzy. The best I
can recall is that they were probably createentity()ing a high-speed
horizontally-moving enemy in a vertically warping room, only to discover
that said enemy kept warping horizontally.
2020-04-04 21:39:17 +02:00
|
|
|
|
Fix the two-frame-delay when entering a room with an "init" script
This patch is very kludge-y, but at least it fixes a semi-noticeable
visual issue in custom levels that use internal scripts to spawn
entities when loading a room.
Basically, the problem here is that when the game checks for script
boxes and sets newscript, newscript has already been processed for that
frame, and when the game does load a script, script.run() has already
been processed for that frame.
That issue can be fixed, but it turns out that due to my over-30-FPS
game loop changes, there's now ANOTHER visible frame of delay between
room load and entity creation, because the render function gets called
in between the script being loaded at the end of gamelogic() and the
script actually getting run.
So... I have to temporary move script.run() to the end of gamelogic()
(in map.twoframedelayfix()), and make sure it doesn't get run next
frame, because double-evaluations are bad. To do that, I have to
introduce the kludge variable script.dontrunnextframe, which does
exactly as it says.
And with all that work, the two-frame (now three-frame) delay is fixed.
2020-06-28 02:08:57 +02:00
|
|
|
bool screen_transition = false;
|
|
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
if (!map.warpy && !map.towermode)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
//Normal! Just change room
|
|
|
|
|
int player = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(player, obj.entities) && game.door_down > -2 && obj.entities[player].yp >= 238)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
obj.entities[player].yp -= 240;
|
2020-03-31 10:09:42 +02:00
|
|
|
map.gotoroom(game.roomx, game.roomy + 1);
|
Fix the two-frame-delay when entering a room with an "init" script
This patch is very kludge-y, but at least it fixes a semi-noticeable
visual issue in custom levels that use internal scripts to spawn
entities when loading a room.
Basically, the problem here is that when the game checks for script
boxes and sets newscript, newscript has already been processed for that
frame, and when the game does load a script, script.run() has already
been processed for that frame.
That issue can be fixed, but it turns out that due to my over-30-FPS
game loop changes, there's now ANOTHER visible frame of delay between
room load and entity creation, because the render function gets called
in between the script being loaded at the end of gamelogic() and the
script actually getting run.
So... I have to temporary move script.run() to the end of gamelogic()
(in map.twoframedelayfix()), and make sure it doesn't get run next
frame, because double-evaluations are bad. To do that, I have to
introduce the kludge variable script.dontrunnextframe, which does
exactly as it says.
And with all that work, the two-frame (now three-frame) delay is fixed.
2020-06-28 02:08:57 +02:00
|
|
|
screen_transition = true;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(player, obj.entities) && game.door_up > -2 && obj.entities[player].yp < -2)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
obj.entities[player].yp += 240;
|
2020-03-31 10:09:42 +02:00
|
|
|
map.gotoroom(game.roomx, game.roomy - 1);
|
Fix the two-frame-delay when entering a room with an "init" script
This patch is very kludge-y, but at least it fixes a semi-noticeable
visual issue in custom levels that use internal scripts to spawn
entities when loading a room.
Basically, the problem here is that when the game checks for script
boxes and sets newscript, newscript has already been processed for that
frame, and when the game does load a script, script.run() has already
been processed for that frame.
That issue can be fixed, but it turns out that due to my over-30-FPS
game loop changes, there's now ANOTHER visible frame of delay between
room load and entity creation, because the render function gets called
in between the script being loaded at the end of gamelogic() and the
script actually getting run.
So... I have to temporary move script.run() to the end of gamelogic()
(in map.twoframedelayfix()), and make sure it doesn't get run next
frame, because double-evaluations are bad. To do that, I have to
introduce the kludge variable script.dontrunnextframe, which does
exactly as it says.
And with all that work, the two-frame (now three-frame) delay is fixed.
2020-06-28 02:08:57 +02:00
|
|
|
screen_transition = true;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
De-dupe screen transition / warping logic
I noticed that the code for going to the adjacent room when offscreen
and for warping instead if the room is warping was a bit
copy-and-pasted. To clean up the code a bit, there's now 5 separate
checks in gamelogic():
if (map.warpx)
if (map.warpy)
if (map.warpy && !map.warpx)
if (!map.warpy)
if (!map.warpx)
I made sure to preserve the previous weird horizontal warping behavior
that happens with vertical warping (thus the middle one), and to
preserve the frame ordering just in case there's something dependent on
the frame ordering.
The frame ordering is that first it will warp you horizontally, if
applicable, then it will warp you vertically, if applicable. Then if you
have vertical warping only, that weird horizontal warp. Then it will
screen transition you vertically, if applicable. Then it will screen
transition you horizontally, if applicable.
To explain the weird horizontal warp with the vertical warp: apparently
if an entity is far offscreen enough, and if that entity is not the
player, it will be warped horizontally during a vertical warp. The
points at which it will warp is 30 pixels farther out than normal
horizontal warping.
I think someone ran into this before, but my memory is fuzzy. The best I
can recall is that they were probably createentity()ing a high-speed
horizontally-moving enemy in a vertically warping room, only to discover
that said enemy kept warping horizontally.
2020-04-04 21:39:17 +02:00
|
|
|
}
|
|
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
if (!map.warpx && !map.towermode)
|
De-dupe screen transition / warping logic
I noticed that the code for going to the adjacent room when offscreen
and for warping instead if the room is warping was a bit
copy-and-pasted. To clean up the code a bit, there's now 5 separate
checks in gamelogic():
if (map.warpx)
if (map.warpy)
if (map.warpy && !map.warpx)
if (!map.warpy)
if (!map.warpx)
I made sure to preserve the previous weird horizontal warping behavior
that happens with vertical warping (thus the middle one), and to
preserve the frame ordering just in case there's something dependent on
the frame ordering.
The frame ordering is that first it will warp you horizontally, if
applicable, then it will warp you vertically, if applicable. Then if you
have vertical warping only, that weird horizontal warp. Then it will
screen transition you vertically, if applicable. Then it will screen
transition you horizontally, if applicable.
To explain the weird horizontal warp with the vertical warp: apparently
if an entity is far offscreen enough, and if that entity is not the
player, it will be warped horizontally during a vertical warp. The
points at which it will warp is 30 pixels farther out than normal
horizontal warping.
I think someone ran into this before, but my memory is fuzzy. The best I
can recall is that they were probably createentity()ing a high-speed
horizontally-moving enemy in a vertically warping room, only to discover
that said enemy kept warping horizontally.
2020-04-04 21:39:17 +02:00
|
|
|
{
|
|
|
|
|
//Normal! Just change room
|
|
|
|
|
int player = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(player, obj.entities) && game.door_left > -2 && obj.entities[player].xp < -14)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
obj.entities[player].xp += 320;
|
2020-03-31 10:09:42 +02:00
|
|
|
map.gotoroom(game.roomx - 1, game.roomy);
|
Fix the two-frame-delay when entering a room with an "init" script
This patch is very kludge-y, but at least it fixes a semi-noticeable
visual issue in custom levels that use internal scripts to spawn
entities when loading a room.
Basically, the problem here is that when the game checks for script
boxes and sets newscript, newscript has already been processed for that
frame, and when the game does load a script, script.run() has already
been processed for that frame.
That issue can be fixed, but it turns out that due to my over-30-FPS
game loop changes, there's now ANOTHER visible frame of delay between
room load and entity creation, because the render function gets called
in between the script being loaded at the end of gamelogic() and the
script actually getting run.
So... I have to temporary move script.run() to the end of gamelogic()
(in map.twoframedelayfix()), and make sure it doesn't get run next
frame, because double-evaluations are bad. To do that, I have to
introduce the kludge variable script.dontrunnextframe, which does
exactly as it says.
And with all that work, the two-frame (now three-frame) delay is fixed.
2020-06-28 02:08:57 +02:00
|
|
|
screen_transition = true;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(player, obj.entities) && game.door_right > -2 && obj.entities[player].xp >= 308)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
obj.entities[player].xp -= 320;
|
2020-03-31 10:09:42 +02:00
|
|
|
map.gotoroom(game.roomx + 1, game.roomy);
|
Fix the two-frame-delay when entering a room with an "init" script
This patch is very kludge-y, but at least it fixes a semi-noticeable
visual issue in custom levels that use internal scripts to spawn
entities when loading a room.
Basically, the problem here is that when the game checks for script
boxes and sets newscript, newscript has already been processed for that
frame, and when the game does load a script, script.run() has already
been processed for that frame.
That issue can be fixed, but it turns out that due to my over-30-FPS
game loop changes, there's now ANOTHER visible frame of delay between
room load and entity creation, because the render function gets called
in between the script being loaded at the end of gamelogic() and the
script actually getting run.
So... I have to temporary move script.run() to the end of gamelogic()
(in map.twoframedelayfix()), and make sure it doesn't get run next
frame, because double-evaluations are bad. To do that, I have to
introduce the kludge variable script.dontrunnextframe, which does
exactly as it says.
And with all that work, the two-frame (now three-frame) delay is fixed.
2020-06-28 02:08:57 +02:00
|
|
|
screen_transition = true;
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2020-04-25 06:19:41 +02:00
|
|
|
//Right so! Screenwraping for tower:
|
2020-05-05 20:26:26 +02:00
|
|
|
if (map.towermode && map.minitowermode)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
2020-11-03 00:05:24 +01:00
|
|
|
if (graphics.towerbg.scrolldir == 1)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
|
|
|
|
//This is minitower 1!
|
|
|
|
|
int player = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(player, obj.entities) && game.door_left > -2 && obj.entities[player].xp < -14)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[player].xp += 320;
|
|
|
|
|
map.gotoroom(48, 52);
|
|
|
|
|
}
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(player, obj.entities) && game.door_right > -2 && obj.entities[player].xp >= 308)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[player].xp -= 320;
|
|
|
|
|
obj.entities[player].yp -= (71*8);
|
|
|
|
|
map.gotoroom(game.roomx + 1, game.roomy+1);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
//This is minitower 2!
|
|
|
|
|
int player = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(player, obj.entities) && game.door_left > -2 && obj.entities[player].xp < -14)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
|
|
|
|
if (obj.entities[player].yp > 300)
|
|
|
|
|
{
|
|
|
|
|
obj.entities[player].xp += 320;
|
|
|
|
|
obj.entities[player].yp -= (71 * 8);
|
|
|
|
|
map.gotoroom(50, 54);
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
obj.entities[player].xp += 320;
|
|
|
|
|
map.gotoroom(50, 53);
|
|
|
|
|
}
|
|
|
|
|
}
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(player, obj.entities) && game.door_right > -2 && obj.entities[player].xp >= 308)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[player].xp -= 320;
|
|
|
|
|
map.gotoroom(52, 53);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else if (map.towermode)
|
|
|
|
|
{
|
|
|
|
|
//Always wrap except for the very top and very bottom of the tower
|
|
|
|
|
if(map.ypos>=500 && map.ypos <=5000)
|
|
|
|
|
{
|
|
|
|
|
for (size_t i = 0; i < obj.entities.size(); i++)
|
|
|
|
|
{
|
|
|
|
|
if (obj.entities[i].xp <= -10)
|
|
|
|
|
{
|
|
|
|
|
obj.entities[i].xp += 320;
|
Restore previous oldxp/oldyp variables in favor of lerpoldxp/lerpoldyp
I was investigating a desync in my Nova TAS, and it turns out that
the gravity line collision functions check for the `oldxp` and `oldyp`
of the player, i.e. their position on the previous frame, along with
their position on the current frame. So, if the player either collided
with the gravity line last frame or this frame, then the player collided
with the gravity line this frame.
Except, that's not actually true. It turns out that `oldxp` and `oldyp`
don't necessarily always correspond to the `xp` and `yp` of the player
on the previous frame. It turns out that your `oldyp` will be updated if
you stand on a vertically moving platform, before the gravity line
collision function gets ran. So, if you were colliding with a gravity
line on the previous frame, but you got moved out of there by a
vertically moving platform, then you just don't collide with the gravity
line at all.
However, this behavior changed in 2.3 after my over-30-FPS patch got
merged (#220). That patch took advantage of the existing `oldxp` and
`oldyp` entity attributes, and uses them to interpolate their positions
during rendering to make everything look real smooth.
Previously, `oldxp` and `oldyp` would both be updated in
`entityclass::updateentitylogic()`. However, I moved it in that patch to
update right before `gameinput()` in `main.cpp`.
As a result, `oldyp` no longer gets updated whenever the player stands
on a vertically moving platform. This ends up desyncing my TAS.
As expected, updating `oldyp` in `entityclass::movingplatformfix()` (the
function responsible for moving the player whenever they stand on a
vertically moving platform) makes it so that my TAS syncs, but the
visuals are glitchy when standing on a vertically moving platform. And
as much as I'd like to get rid of gravity lines checking for whether
you've collided with them on the previous frame, doing that desyncs my
TAS, too.
In the end, it seems like I should just leave `oldxp` and `oldyp` alone,
and switch to using dedicated variables that are never used in the
physics of the game. So I'm introducing `lerpoldxp` and `lerpoldyp`, and
replacing all instances of using `oldxp` and `oldyp` that my over-30-FPS
patch added, with `lerpoldxp` and `lerpoldyp` instead.
After doing this, and applying #503 as well, my Nova TAS syncs after
some minor but acceptable fixes with Viridian's walkingframe.
2020-10-10 05:58:58 +02:00
|
|
|
obj.entities[i].lerpoldxp += 320;
|
2020-04-25 06:19:41 +02:00
|
|
|
}
|
2020-04-29 01:45:53 +02:00
|
|
|
else if (obj.entities[i].xp > 310)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
2020-04-29 01:45:53 +02:00
|
|
|
obj.entities[i].xp -= 320;
|
Restore previous oldxp/oldyp variables in favor of lerpoldxp/lerpoldyp
I was investigating a desync in my Nova TAS, and it turns out that
the gravity line collision functions check for the `oldxp` and `oldyp`
of the player, i.e. their position on the previous frame, along with
their position on the current frame. So, if the player either collided
with the gravity line last frame or this frame, then the player collided
with the gravity line this frame.
Except, that's not actually true. It turns out that `oldxp` and `oldyp`
don't necessarily always correspond to the `xp` and `yp` of the player
on the previous frame. It turns out that your `oldyp` will be updated if
you stand on a vertically moving platform, before the gravity line
collision function gets ran. So, if you were colliding with a gravity
line on the previous frame, but you got moved out of there by a
vertically moving platform, then you just don't collide with the gravity
line at all.
However, this behavior changed in 2.3 after my over-30-FPS patch got
merged (#220). That patch took advantage of the existing `oldxp` and
`oldyp` entity attributes, and uses them to interpolate their positions
during rendering to make everything look real smooth.
Previously, `oldxp` and `oldyp` would both be updated in
`entityclass::updateentitylogic()`. However, I moved it in that patch to
update right before `gameinput()` in `main.cpp`.
As a result, `oldyp` no longer gets updated whenever the player stands
on a vertically moving platform. This ends up desyncing my TAS.
As expected, updating `oldyp` in `entityclass::movingplatformfix()` (the
function responsible for moving the player whenever they stand on a
vertically moving platform) makes it so that my TAS syncs, but the
visuals are glitchy when standing on a vertically moving platform. And
as much as I'd like to get rid of gravity lines checking for whether
you've collided with them on the previous frame, doing that desyncs my
TAS, too.
In the end, it seems like I should just leave `oldxp` and `oldyp` alone,
and switch to using dedicated variables that are never used in the
physics of the game. So I'm introducing `lerpoldxp` and `lerpoldyp`, and
replacing all instances of using `oldxp` and `oldyp` that my over-30-FPS
patch added, with `lerpoldxp` and `lerpoldyp` instead.
After doing this, and applying #503 as well, my Nova TAS syncs after
some minor but acceptable fixes with Viridian's walkingframe.
2020-10-10 05:58:58 +02:00
|
|
|
obj.entities[i].lerpoldxp -= 320;
|
2020-04-25 06:19:41 +02:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
//Do not wrap! Instead, go to the correct room
|
|
|
|
|
int player = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(player, obj.entities) && game.door_left > -2 && obj.entities[player].xp < -14)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[player].xp += 320;
|
|
|
|
|
obj.entities[player].yp -= (671 * 8);
|
|
|
|
|
map.gotoroom(108, 109);
|
|
|
|
|
}
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(player, obj.entities) && game.door_right > -2 && obj.entities[player].xp >= 308)
|
2020-04-25 06:19:41 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[player].xp -= 320;
|
|
|
|
|
map.gotoroom(110, 104);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2020-01-01 21:29:24 +01:00
|
|
|
//Warp tokens
|
|
|
|
|
if (map.custommode){
|
2021-04-14 05:20:40 +02:00
|
|
|
if (game.teleport && INBOUNDS_VEC(game.edteleportent, obj.entities))
|
2020-04-04 21:13:24 +02:00
|
|
|
{
|
|
|
|
|
int edi=obj.entities[game.edteleportent].behave;
|
|
|
|
|
int edj=obj.entities[game.edteleportent].para;
|
|
|
|
|
int edi2, edj2;
|
|
|
|
|
edi2 = (edi-(edi%40))/40;
|
|
|
|
|
edj2 = (edj-(edj%30))/30;
|
|
|
|
|
|
|
|
|
|
map.warpto(100+edi2, 100+edj2, obj.getplayer(), edi%40, (edj%30)+2);
|
|
|
|
|
game.teleport = false;
|
|
|
|
|
|
|
|
|
|
if (game.teleport == false)
|
|
|
|
|
{
|
|
|
|
|
game.flashlight = 6;
|
|
|
|
|
game.screenshake = 25;
|
|
|
|
|
}
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}else{
|
2020-04-04 21:13:24 +02:00
|
|
|
if (game.teleport)
|
|
|
|
|
{
|
|
|
|
|
if (game.roomx == 117 && game.roomy == 102)
|
|
|
|
|
{
|
|
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities))
|
2020-06-13 05:36:08 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[i].yp = 225;
|
|
|
|
|
}
|
2020-04-04 21:13:24 +02:00
|
|
|
map.gotoroom(119, 100);
|
|
|
|
|
game.teleport = false;
|
|
|
|
|
}
|
|
|
|
|
else if (game.roomx == 119 && game.roomy == 100)
|
|
|
|
|
{
|
|
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities))
|
2020-06-13 05:36:08 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[i].yp = 225;
|
|
|
|
|
}
|
2020-04-04 21:13:24 +02:00
|
|
|
map.gotoroom(119, 103);
|
|
|
|
|
game.teleport = false;
|
|
|
|
|
}
|
|
|
|
|
else if (game.roomx == 119 && game.roomy == 103)
|
|
|
|
|
{
|
|
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities))
|
2020-06-13 05:36:08 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[i].xp = 0;
|
|
|
|
|
}
|
2020-04-04 21:13:24 +02:00
|
|
|
map.gotoroom(116, 103);
|
|
|
|
|
game.teleport = false;
|
|
|
|
|
}
|
|
|
|
|
else if (game.roomx == 116 && game.roomy == 103)
|
|
|
|
|
{
|
|
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities))
|
2020-06-13 05:36:08 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[i].yp = 225;
|
|
|
|
|
}
|
2020-04-04 21:13:24 +02:00
|
|
|
map.gotoroom(116, 100);
|
|
|
|
|
game.teleport = false;
|
|
|
|
|
}
|
|
|
|
|
else if (game.roomx == 116 && game.roomy == 100)
|
|
|
|
|
{
|
|
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities))
|
2020-06-13 05:36:08 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[i].xp = 0;
|
|
|
|
|
}
|
2020-04-04 21:13:24 +02:00
|
|
|
map.gotoroom(114, 102);
|
|
|
|
|
game.teleport = false;
|
|
|
|
|
}
|
|
|
|
|
else if (game.roomx == 114 && game.roomy == 102)
|
|
|
|
|
{
|
|
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities))
|
2020-06-13 05:36:08 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[i].yp = 225;
|
|
|
|
|
}
|
2020-04-04 21:13:24 +02:00
|
|
|
map.gotoroom(113, 100);
|
|
|
|
|
game.teleport = false;
|
|
|
|
|
}
|
|
|
|
|
else if (game.roomx == 116 && game.roomy == 104)
|
|
|
|
|
{
|
|
|
|
|
//pre warp zone here
|
|
|
|
|
map.warpto(107, 101, obj.getplayer(), 14, 16);
|
|
|
|
|
}
|
|
|
|
|
else if (game.roomx == 107 && game.roomy == 101)
|
|
|
|
|
{
|
|
|
|
|
map.warpto(105, 119, obj.getplayer(), 5, 26);
|
|
|
|
|
}
|
|
|
|
|
else if (game.roomx == 105 && game.roomy == 118)
|
|
|
|
|
{
|
|
|
|
|
map.warpto(101, 111, obj.getplayer(), 34, 6);
|
|
|
|
|
}
|
|
|
|
|
else if (game.roomx == 101 && game.roomy == 111)
|
|
|
|
|
{
|
|
|
|
|
//There are lots of warp tokens in this room, so we have to distinguish!
|
|
|
|
|
switch(game.teleportxpos)
|
|
|
|
|
{
|
|
|
|
|
case 1:
|
|
|
|
|
map.warpto(108, 108, obj.getplayer(), 4, 27);
|
|
|
|
|
break;
|
|
|
|
|
case 2:
|
|
|
|
|
map.warpto(101, 111, obj.getplayer(), 12, 27);
|
|
|
|
|
break;
|
|
|
|
|
case 3:
|
|
|
|
|
map.warpto(119, 111, obj.getplayer(), 31, 7);
|
|
|
|
|
break;
|
|
|
|
|
case 4:
|
|
|
|
|
map.warpto(114, 117, obj.getplayer(), 19, 16);
|
|
|
|
|
break;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else if (game.roomx == 108 && game.roomy == 106)
|
|
|
|
|
{
|
|
|
|
|
map.warpto(119, 111, obj.getplayer(), 4, 27);
|
|
|
|
|
}
|
|
|
|
|
else if (game.roomx == 100 && game.roomy == 111)
|
|
|
|
|
{
|
|
|
|
|
map.warpto(101, 111, obj.getplayer(), 24, 6);
|
|
|
|
|
}
|
|
|
|
|
else if (game.roomx == 119 && game.roomy == 107)
|
|
|
|
|
{
|
|
|
|
|
//Secret lab, to super gravitron
|
|
|
|
|
map.warpto(119, 108, obj.getplayer(), 19, 10);
|
|
|
|
|
}
|
|
|
|
|
if (game.teleport == false)
|
|
|
|
|
{
|
|
|
|
|
game.flashlight = 6;
|
|
|
|
|
game.screenshake = 25;
|
|
|
|
|
}
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
Fix the two-frame-delay when entering a room with an "init" script
This patch is very kludge-y, but at least it fixes a semi-noticeable
visual issue in custom levels that use internal scripts to spawn
entities when loading a room.
Basically, the problem here is that when the game checks for script
boxes and sets newscript, newscript has already been processed for that
frame, and when the game does load a script, script.run() has already
been processed for that frame.
That issue can be fixed, but it turns out that due to my over-30-FPS
game loop changes, there's now ANOTHER visible frame of delay between
room load and entity creation, because the render function gets called
in between the script being loaded at the end of gamelogic() and the
script actually getting run.
So... I have to temporary move script.run() to the end of gamelogic()
(in map.twoframedelayfix()), and make sure it doesn't get run next
frame, because double-evaluations are bad. To do that, I have to
introduce the kludge variable script.dontrunnextframe, which does
exactly as it says.
And with all that work, the two-frame (now three-frame) delay is fixed.
2020-06-28 02:08:57 +02:00
|
|
|
|
|
|
|
|
if (screen_transition)
|
|
|
|
|
{
|
|
|
|
|
map.twoframedelayfix();
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2020-04-30 21:32:53 +02:00
|
|
|
|
Consolidate tower BG bypos and bscroll assignments
Tower backgrounds have a bypos and bscroll. bypos is just the y-position
of the background, and bscroll is the amount of pixels to scroll the
background by on each frame, which is used to scroll it (if it's not
being redrawn) and for linear interpolation.
For the tower background (and not the title background), bypos is
map.ypos / 2, and bscroll is (map.ypos - map.oldypos) / 2. However,
usually bscroll gets assigned at the same time bypos is incremented or
decremented, so you never see that calculation explicitly - except in
the previous commit, where I worked out the calculation because the
change in y-position isn't a known constant.
Having to do all these calculations every time introduces the
possibility of errors where you forget to do it, or you do it wrongly.
But that's not even the worst; you could cause a linear interpolation
glitch if you decide to overwrite bscroll without taking into account
map.oldypos and map.ypos.
So that's why I'm adding a function that automatically updates the tower
background, using the values of map.oldypos and map.ypos, that is used
every time map.ypos is assigned. That way, we have to write less code,
you can be sure that there's no place where we forget to do the
calculations (or at least it will be glaringly obvious) or we do it
wrongly, and it plays nicely with linear interpolation. This also
replaces every instance where the manual calculations are done with the
new function.
2021-04-23 03:47:07 +02:00
|
|
|
if (map.towermode)
|
|
|
|
|
{
|
|
|
|
|
map.setbgobjlerp(graphics.towerbg);
|
|
|
|
|
}
|
|
|
|
|
|
2020-04-30 21:32:53 +02:00
|
|
|
//Update colour cycling for final level
|
|
|
|
|
if (map.finalmode && map.final_colormode)
|
|
|
|
|
{
|
|
|
|
|
map.final_aniframedelay--;
|
|
|
|
|
if(map.final_aniframedelay==0)
|
|
|
|
|
{
|
|
|
|
|
graphics.foregrounddrawn=false;
|
|
|
|
|
}
|
|
|
|
|
if (map.final_aniframedelay <= 0) {
|
|
|
|
|
map.final_aniframedelay = 2;
|
|
|
|
|
map.final_aniframe++;
|
|
|
|
|
if (map.final_aniframe >= 4)
|
|
|
|
|
map.final_aniframe = 0;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2020-01-01 21:29:24 +01:00
|
|
|
if (game.roomchange)
|
|
|
|
|
{
|
|
|
|
|
//We've changed room? Let's bring our companion along!
|
|
|
|
|
game.roomchange = false;
|
2020-06-13 05:36:08 +02:00
|
|
|
int i = obj.getplayer();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (game.companion > 0 && INBOUNDS_VEC(i, obj.entities))
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
//ok, we'll presume our companion has been destroyed in the room change. So:
|
|
|
|
|
switch(game.companion)
|
|
|
|
|
{
|
|
|
|
|
case 6:
|
2020-09-08 23:59:23 +02:00
|
|
|
{
|
2020-03-31 02:46:36 +02:00
|
|
|
obj.createentity(obj.entities[i].xp, 121.0f, 15.0f,1); //Y=121, the floor in that particular place!
|
2020-09-08 23:59:23 +02:00
|
|
|
int j = obj.getcompanion();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(j, obj.entities))
|
2020-06-13 05:43:19 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[j].vx = obj.entities[i].vx;
|
|
|
|
|
obj.entities[j].dir = obj.entities[i].dir;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
2020-09-08 23:59:23 +02:00
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
case 7:
|
|
|
|
|
if (game.roomy <= 105) //don't jump after him!
|
|
|
|
|
{
|
|
|
|
|
if (game.roomx == 110)
|
|
|
|
|
{
|
2020-03-31 02:46:36 +02:00
|
|
|
obj.createentity(320, 86, 16, 1); //Y=86, the ROOF in that particular place!
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
2020-03-31 02:46:36 +02:00
|
|
|
obj.createentity(obj.entities[i].xp, 86.0f, 16.0f, 1); //Y=86, the ROOF in that particular place!
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2020-09-08 23:59:23 +02:00
|
|
|
int j = obj.getcompanion();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(j, obj.entities))
|
2020-06-13 05:43:19 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[j].vx = obj.entities[i].vx;
|
|
|
|
|
obj.entities[j].dir = obj.entities[i].dir;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
break;
|
|
|
|
|
case 8:
|
|
|
|
|
if (game.roomy >= 104) //don't jump after him!
|
|
|
|
|
{
|
|
|
|
|
if (game.roomx == 102)
|
|
|
|
|
{
|
2020-03-31 02:46:36 +02:00
|
|
|
obj.createentity(310, 177, 17, 1);
|
2020-09-08 23:59:23 +02:00
|
|
|
int j = obj.getcompanion();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(j, obj.entities))
|
2020-06-13 05:43:19 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[j].vx = obj.entities[i].vx;
|
|
|
|
|
obj.entities[j].dir = obj.entities[i].dir;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
2020-03-31 02:46:36 +02:00
|
|
|
obj.createentity(obj.entities[i].xp, 177.0f, 17.0f, 1);
|
2020-09-08 23:59:23 +02:00
|
|
|
int j = obj.getcompanion();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(j, obj.entities))
|
2020-06-13 05:43:19 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[j].vx = obj.entities[i].vx;
|
|
|
|
|
obj.entities[j].dir = obj.entities[i].dir;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
break;
|
|
|
|
|
case 9:
|
|
|
|
|
if (!map.towermode) //don't go back into the tower!
|
|
|
|
|
{
|
|
|
|
|
if (game.roomx == 110 && obj.entities[i].xp<20)
|
|
|
|
|
{
|
2020-03-31 02:46:36 +02:00
|
|
|
obj.createentity(100, 185, 18, 15, 0, 1);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
2020-03-31 02:46:36 +02:00
|
|
|
obj.createentity(obj.entities[i].xp, 185.0f, 18.0f, 15, 0, 1);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
2020-09-08 23:59:23 +02:00
|
|
|
int j = obj.getcompanion();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(j, obj.entities))
|
2020-06-13 05:43:19 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[j].vx = obj.entities[i].vx;
|
|
|
|
|
obj.entities[j].dir = obj.entities[i].dir;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
break;
|
|
|
|
|
case 10:
|
|
|
|
|
//intermission 2, choose colour based on lastsaved
|
|
|
|
|
if (game.roomy == 51)
|
|
|
|
|
{
|
2020-04-09 08:34:26 +02:00
|
|
|
if (!obj.flags[59])
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(225.0f, 169.0f, 18, graphics.crewcolour(game.lastsaved), 0, 10);
|
2020-09-08 23:59:23 +02:00
|
|
|
int j = obj.getcompanion();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(j, obj.entities))
|
2020-06-13 05:43:19 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[j].vx = obj.entities[i].vx;
|
|
|
|
|
obj.entities[j].dir = obj.entities[i].dir;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else if (game.roomy >= 52)
|
|
|
|
|
{
|
2020-04-09 08:34:26 +02:00
|
|
|
if (obj.flags[59])
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(160.0f, 177.0f, 18, graphics.crewcolour(game.lastsaved), 0, 18, 1);
|
2020-09-08 23:59:23 +02:00
|
|
|
int j = obj.getcompanion();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(j, obj.entities))
|
2020-06-13 05:43:19 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[j].vx = obj.entities[i].vx;
|
|
|
|
|
obj.entities[j].dir = obj.entities[i].dir;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
2020-04-09 08:34:26 +02:00
|
|
|
obj.flags[59] = true;
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(obj.entities[i].xp, -20.0f, 18.0f, graphics.crewcolour(game.lastsaved), 0, 10, 0);
|
2020-09-08 23:59:23 +02:00
|
|
|
int j = obj.getcompanion();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(j, obj.entities))
|
2020-06-13 05:43:19 +02:00
|
|
|
{
|
|
|
|
|
obj.entities[j].vx = obj.entities[i].vx;
|
|
|
|
|
obj.entities[j].dir = obj.entities[i].dir;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
break;
|
|
|
|
|
case 11:
|
|
|
|
|
//Intermission 1: We're using the SuperCrewMate instead!
|
|
|
|
|
if(game.roomx-41==game.scmprogress)
|
|
|
|
|
{
|
|
|
|
|
switch(game.scmprogress)
|
|
|
|
|
{
|
|
|
|
|
case 0:
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(76, 161, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
|
|
|
|
case 1:
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(10, 169, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
|
|
|
|
case 2:
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(10, 177, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
|
|
|
|
case 3:
|
|
|
|
|
if (game.scmmoveme)
|
|
|
|
|
{
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(obj.entities[obj.getplayer()].xp, 185, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
game.scmmoveme = false;
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(10, 177, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
|
|
|
|
break;
|
|
|
|
|
case 4:
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(10, 185, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
|
|
|
|
case 5:
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(10, 185, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
|
|
|
|
case 6:
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(10, 185, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
|
|
|
|
case 7:
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(10, 41, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
|
|
|
|
case 8:
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(10, 169, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
|
|
|
|
case 9:
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(10, 169, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
|
|
|
|
case 10:
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(10, 129, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
|
|
|
|
case 11:
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(10, 129, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
|
|
|
|
case 12:
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(10, 65, 24, graphics.crewcolour(game.lastsaved), 2);
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
|
|
|
|
case 13:
|
2020-04-01 23:48:11 +02:00
|
|
|
obj.createentity(10, 177, 24, graphics.crewcolour(game.lastsaved));
|
2020-01-01 21:29:24 +01:00
|
|
|
break;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if (game.scmmoveme)
|
|
|
|
|
{
|
Bounds check all entity getters that can return 0
The entity getters I'm referring to are entityclass::getscm(),
entityclass::getlineat(), entityclass::getcrewman(), and
entityclass::getcustomcrewman().
Even though the player should always exist, and the player should always
be indice 0, I wouldn't want to make that assumption. I've been wrong
before.
Also, these functions returning 0 lull you into a false sense of
security. If you assume that commands using these functions are fine,
you'll forget about the fact that `i` in those commands could be
potentially anything, given an invalid argument. In fact, it's possible
to index createactivityzone(), flipgravity(), and customposition()
out-of-bounds by setting `i` to anything! Well, WAS possible. I fixed it
so now they can't.
Furthermore, in the game.scmmoveme block in gamelogic(), obj.getplayer()
wasn't even checked, even though it's been checked in all other places.
I only caught it just now because I wanted to bounds-check all usages of
obj.getscm(), too, and that game.scmmove block also used obj.getscm()
without bounds-checking it as well.
2020-09-10 07:31:09 +02:00
|
|
|
int scm = obj.getscm();
|
|
|
|
|
int player = obj.getplayer();
|
|
|
|
|
if (INBOUNDS_VEC(scm, obj.entities) && INBOUNDS_VEC(player, obj.entities))
|
|
|
|
|
{
|
|
|
|
|
obj.entities[scm].xp = obj.entities[player].xp;
|
|
|
|
|
}
|
2020-01-01 21:29:24 +01:00
|
|
|
game.scmmoveme = false;
|
|
|
|
|
}
|
|
|
|
|
break;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
game.activeactivity = obj.checkactivity();
|
|
|
|
|
|
Fix prompt fade out when activating overlapping zones
If you stood in two activity zones at once, you'll automatically select
the one that got created first. And when you activated it, the activity
zone prompt would switch to fading out the prompt of the OTHER activity
zone, the one you didn't activate.
This wasn't a problem in 2.2 and previous, because the fading animation
was simply bugged and defaulted to being solid black. However, in 2.3,
the fading animation is fixed, so this is possible.
Also, this really only happens in the main game. Since there's only one
type of useful activity zone in custom levels - namely the terminal
activity zone - if two activity zones did happen to overlap, activating
one of them wouldn't result in visibly fading out a different activity
zone (because they both look the same); furthermore custom level makers
are careful to not overlap terminal activity zones, lest this result in
player confusion; furthermore the placed activity zones only cover a
small area, whereas in the main game, crewmates' activity zones are
pretty big.
(Technically, you CAN create main game activity zones in custom levels,
but those are hardcoded to call main game scripts, and basically nobody
uses them.)
So what's the solution? Simply adding game.hascontrol and script.running
checks to the updating of game.activity_last[prompt|r|g|b].
Why not add those checks to the assignment of game.activeactivity, just
above? Because that would introduce a frame ordering issue (that
would NOT be (automatically) fixed by #535) where the eligibility of
pressing Enter on an activity zone now checks if you were standing in an
activity zone LAST frame, and not THIS frame. (I tested this with
libTAS.) Better to fiddle with the rendering code than fiddle with the
actual physics code.
The specific spot I used to test this was standing in Violet's activity
zone and the activity zone of the ship radio terminals (the three
terminals on the ground in her room); the ship radio terminals are
first-placed, so if you're testing this (and you should!), make that the
prompt is of the ship radio activity zone before activation.
2021-03-05 19:43:11 +01:00
|
|
|
if (game.hascontrol && !script.running
|
2021-06-26 21:56:35 +02:00
|
|
|
&& INBOUNDS_VEC(game.activeactivity, obj.blocks))
|
2021-03-05 19:41:35 +01:00
|
|
|
{
|
|
|
|
|
game.activity_lastprompt = obj.blocks[game.activeactivity].prompt;
|
|
|
|
|
game.activity_r = obj.blocks[game.activeactivity].r;
|
|
|
|
|
game.activity_g = obj.blocks[game.activeactivity].g;
|
|
|
|
|
game.activity_b = obj.blocks[game.activeactivity].b;
|
|
|
|
|
}
|
|
|
|
|
|
2020-04-30 02:04:25 +02:00
|
|
|
game.oldreadytotele = game.readytotele;
|
Remove advancetext check from teleporter prompt logic
This fixes being unable to use teleporters while the "- Press ACTION to
advance text -" prompt is up, which is used to perform credits warp.
In 2.2 and 2.0, this advancetext check was only in gamerender() for
rendering the "- Press ENTER to Teleport -" prompt and didn't affect any
logic. In 2.3, I moved the check (and the rest of the conditional it was
in) to gamelogic() - same as the activity zone prompt conditionals - so
if you gained control while being in a prompt zone, the prompt wouldn't
suddenly appear[1].
As a side effect, this ended up aligning rendering and logic together,
so if you couldn't see the teleporter prompt, you weren't able to
teleport - whereas in 2.2 and 2.0, you could still use the teleporter
even though the prompt wasn't up.
So by removing the advancetext check, you are now able to use the
teleporter again, AND the "- Press ENTER to Teleport -" prompt will also
show up as well.
Habeechee reported this regression on the VVVVVV speedrunning Discord
server.
[1]: f07a8d2143479a465c31aea2e08764197a37a2e3, PR #421
2021-04-02 01:21:58 +02:00
|
|
|
if (game.activetele && game.hascontrol && !script.running && !game.intimetrial)
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
int i = obj.getplayer();
|
Move all temporary variables off of entityclass
This is a refactor that simply moves all temporary variables off of
entityclass, and makes it so they are no longer global variables. This
makes the resulting code easier to understand as it is less entangled
with global state.
These attributes were:
- colpoint1
- colpoint2
- tempx
- tempy
- tempw
- temph
- temp
- temp2
- tpx1
- tpy1
- tpx2
- tpy2
- temprect
- temprect2
- x (actually unused)
- dx
- dy
- dr
- px
- py
- linetemp
- activetrigger
- skipblocks
- skipdirblocks
Most of these attributes were assigned before any of the times they were
used, so it's easy to prove that ungloballing them won't change any
behaviors. However, dx, dy, dr, and skipblocks are a bit more tricky to
analyze. They relate to blocks, with dx, dy, and dr more specifically
relating to one-way tiles. So after some testing with the quirks of
one-way tiles, it seems that the jankiness of one-way tiles haven't
changed at all, either.
Unfortunately, the attribute k is clearly used without being assigned
beforehand, so I can't move it off of entityclass. It's the same story
with the attribute k that Graphics has, too.
2020-09-26 21:38:57 +02:00
|
|
|
SDL_Rect temprect = SDL_Rect();
|
Use explicit INBOUNDS_VEC() instead of checking sentinel -1
It's better to do INBOUNDS_VEC(i, obj.entities) instead of 'i > -1'.
'i > -1' is used in cases like obj.getplayer(), which COULD return a
sentinel value of -1 and so correct code will have to check that value.
However, I am now of the opinion that INBOUNDS_VEC() should be used and
isn't unnecessary.
Consider the case of the face() script command: it's not enough to check
i > -1, you should read the routine carefully. Because if you look
closely, you'll see that it's not guaranteed that 'i' will be initialized
at all in that command. Indeed, if you call face() with invalid
arguments, it won't be. And so, 'i' could be something like 215, and
that would index out-of-bounds, and that wouldn't be good. Therefore,
it's better to have the full bounds check instead of checking only one
bounds. Many commands are like this, after some searching I can also
name position(), changemood(), changetile(), changegravity(), etc.
It also makes the code more explicit. Now you don't have to wonder what
-1 means or why it's being checked, you can just read the 'INBOUNDS' and
go "oh, that checks if it's actually inbounds or not".
2020-09-09 13:15:14 +02:00
|
|
|
if (INBOUNDS_VEC(i, obj.entities))
|
2020-06-13 05:36:08 +02:00
|
|
|
{
|
Move all temporary variables off of entityclass
This is a refactor that simply moves all temporary variables off of
entityclass, and makes it so they are no longer global variables. This
makes the resulting code easier to understand as it is less entangled
with global state.
These attributes were:
- colpoint1
- colpoint2
- tempx
- tempy
- tempw
- temph
- temp
- temp2
- tpx1
- tpy1
- tpx2
- tpy2
- temprect
- temprect2
- x (actually unused)
- dx
- dy
- dr
- px
- py
- linetemp
- activetrigger
- skipblocks
- skipdirblocks
Most of these attributes were assigned before any of the times they were
used, so it's easy to prove that ungloballing them won't change any
behaviors. However, dx, dy, dr, and skipblocks are a bit more tricky to
analyze. They relate to blocks, with dx, dy, and dr more specifically
relating to one-way tiles. So after some testing with the quirks of
one-way tiles, it seems that the jankiness of one-way tiles haven't
changed at all, either.
Unfortunately, the attribute k is clearly used without being assigned
beforehand, so I can't move it off of entityclass. It's the same story
with the attribute k that Graphics has, too.
2020-09-26 21:38:57 +02:00
|
|
|
temprect.x = obj.entities[i].xp + obj.entities[i].cx;
|
|
|
|
|
temprect.y = obj.entities[i].yp + obj.entities[i].cy;
|
|
|
|
|
temprect.w = obj.entities[i].w;
|
|
|
|
|
temprect.h = obj.entities[i].h;
|
2020-06-13 05:36:08 +02:00
|
|
|
}
|
Move all temporary variables off of entityclass
This is a refactor that simply moves all temporary variables off of
entityclass, and makes it so they are no longer global variables. This
makes the resulting code easier to understand as it is less entangled
with global state.
These attributes were:
- colpoint1
- colpoint2
- tempx
- tempy
- tempw
- temph
- temp
- temp2
- tpx1
- tpy1
- tpx2
- tpy2
- temprect
- temprect2
- x (actually unused)
- dx
- dy
- dr
- px
- py
- linetemp
- activetrigger
- skipblocks
- skipdirblocks
Most of these attributes were assigned before any of the times they were
used, so it's easy to prove that ungloballing them won't change any
behaviors. However, dx, dy, dr, and skipblocks are a bit more tricky to
analyze. They relate to blocks, with dx, dy, and dr more specifically
relating to one-way tiles. So after some testing with the quirks of
one-way tiles, it seems that the jankiness of one-way tiles haven't
changed at all, either.
Unfortunately, the attribute k is clearly used without being assigned
beforehand, so I can't move it off of entityclass. It's the same story
with the attribute k that Graphics has, too.
2020-09-26 21:38:57 +02:00
|
|
|
if (help.intersects(game.teleblock, temprect))
|
2020-01-01 21:29:24 +01:00
|
|
|
{
|
|
|
|
|
game.readytotele += 25;
|
|
|
|
|
if (game.readytotele >= 255) game.readytotele = 255;
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
game.readytotele -= 50;
|
|
|
|
|
if (game.readytotele < 0) game.readytotele = 0;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
else
|
|
|
|
|
{
|
|
|
|
|
if (game.readytotele > 0)
|
|
|
|
|
{
|
|
|
|
|
game.readytotele -= 50;
|
|
|
|
|
if (game.readytotele < 0) game.readytotele = 0;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if (game.teleport_to_new_area)
|
2020-03-31 21:38:52 +02:00
|
|
|
script.teleport();
|
2020-01-01 21:29:24 +01:00
|
|
|
}
|
