Commit Graph

117 Commits

Author SHA1 Message Date
Misa d271907f8c Fix Secret Lab Time Trial trophies having wrong colors
Ever since 2.0, the colors of some of the Time Trial trophies in the
Secret Lab don't correspond to the crewmate of the given level. The
trophy for the Tower uses Victoria's color, and the Lab trophy uses
Vermilion's color. The Space Station 2 trophy uses Viridian's color, and
the Final Level trophy uses Vitellary's color.

This doesn't appear to be intentional, and it would be odd if it was,
since this game matches the colors everywhere else (each zone on the map
is colored with their respective crewmate in mind, for instance). Also,
the Lab trophy has the sad expression, which is Victoria's trait - it
would be weird if this was intended for Vermilion instead.

But the biggest piece of evidence that this was unintentional is the
corresponding comment for each color in Graphics::setcol(). It mislabels
yellow as cyan, cyan as yellow, blue as red, and red as blue.

To fix this, I simply have to set the correct color for each trophy in
case 25 of entityclass::createentity(). I could fix it in
Graphics::setcol() itself, but custom levels might depend on those
certain colors being the way they are, so it's a safer bet to just fix
it in the trophy creation case itself.

The diff of this might look weird. Even though all I'm doing is changing
some value assignments around, it looks like the "patience" algorithm
thinks I'm moving a whole case of the trophy switch-case around.
2021-01-08 15:17:36 -08:00
Misa ad34e95128 Remove unused function entityclass::fixfriction()
This function was marked as unused by cppcheck.
2021-01-02 09:06:42 -05:00
leo60228 6795856431 Use SDL_fabsf where std::abs was previously used 2021-01-01 22:37:46 -05:00
leo60228 91c3e6cbc5 Revert "Document SDL_abs vs. SDL_fabs"
This reverts commit 8c472ed73d.
2021-01-01 22:37:46 -05:00
leo60228 8c472ed73d Document SDL_abs vs. SDL_fabs 2021-01-01 21:10:16 -05:00
leo60228 c4893a133f Use SDL_abs instead of std::abs
This prevents issues when calling std::abs with a float on some older
compilers. While it would normally be promoted to an int, std::abs is
special due to being overloaded despite being a C function. This can
cause errors due to the compiler being unable to find a float overload.
SDL_abs doesn't have this problem, since it's a normal C function.
2021-01-01 21:10:16 -05:00
Misa 5faa86baae Add x-room = 13 check to entity 1 `behave` 13
This check is clearly meant for destroying the factory clouds in the
room "Level Complete!" in the main game, but it covers all rooms on row
8, instead of only (13,8). Adding an x-room check restricts this
behavior to only (13,8).

Trinket9 reported that this weird behavior of destroying specifically
above y-position 60 was undesirable, since they were creating an enemy
with this `behave` in a room on row 8 and it kept disappearing
instantly.
2020-11-06 15:06:11 -05:00
Misa 3ce442459e Fix conveyor check in collisioncheck() always being true
cppcheck said: "Logical disjunction always evaluates to true".

Yes. Yes it does. Whoops. I learned how to specify ranges like this in
high school math and still screw it up...
2020-11-04 08:38:54 -05:00
Misa 61fc06bc3b Disable hardcoded (10,5) no-follow rule in custom levels
Scripting crewmates apparently have a specific hardcoded rule in their
follow-player code that makes it so if they're in (10,5) and are to the
left of the line x=155, they will refuse to continue following the
player. This was clearly done to make it so Vitellary in the main game
wouldn't overlap the teleporter, and that was clearly done to make it so
it wouldn't look like he would go behind the teleporter, which would
look weird (I also noticed this in #513).

I stumbled across this code and thought that just like other weird
main-game code that shouldn't apply in custom levels (#136, #137, #144),
this should be fixed, too.
2020-11-01 23:15:00 -05:00
Misa f95dbd8426 Disable nocollisionat() for conveyors
While my previous commit fixes the glitchy y-position when you get stuck
inside a conveyor, I noticed that getting inside a conveyor seems to
always push the player out, despite conveyors sharing the same code with
moving platforms, which has code to temporarily disable their own
collision when the player gets stuck inside them, so that the player
DOESN'T get pushed out.

Well, it turns out that the reason this happens is because conveyors in
a room that get placed during mapclass::loadlevel() get tile 1 placed
underneath them. This is mostly so moving platforms will collide with
them, because otherwise platforms don't collide with other platforms,
and a conveyor is considered a platform.

This means that a conveyor without any tiles behind it will simply get
the player stuck if they get inside it, and the player won't be pushed
out. This is bad, because conveyors don't move, so they'll be stuck
there forever until they press R (or save, quit, and load). This
situation doesn't come up in the main game, but it COULD come up in
custom levels that use the internal createentity() command to create
conveyors that don't have any tiles behind them.

It seems good to fix this as well, while we're at it fixing conveyor
physics, so I'm fixing this as well.
2020-10-31 19:07:58 -04:00
Misa b8a4b4dfe7 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-11 16:19:26 -04:00
Misa e8084fe699 Remove now-unused entityclass::hormovingplatformfix()
This function is no longer used after I removed it in favor of using
`entityclass::moveblockto()`.
2020-10-11 16:18:30 -04:00
Misa 99297659ee 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-11 16:18:30 -04:00
Misa c83132f4fa Add entityclass::moveblockto()
This function will restore a block's hitbox after it has been disabled
by `entityclass::nocollisionat()`.
2020-10-11 16:18:30 -04:00
Misa d8cee4866e Add entityclass::nocollisionat()
This function will disable a block instead of removing it entirely,
mimicking the previous `active` system of 2.2.
2020-10-11 16:18:30 -04:00
Misa 7b7c7b2dc7 Fix inconsistencies with y-positions of spawns
It seems like the start point of a custom level and all checkpoints in
the game end up putting your spawn point one pixel away from the surface
it touches, which seems like an oversight. I'm going to enforce some
consistency here and make it so that all spawn points, whenever you
start from a start point or a checkpoint, will always be correctly
positioned flush with the surface they're standing on, and not one pixel
more or less than that.
2020-10-06 02:24:51 -04:00
Misa b4a0acc01d Fix onground/onroof taking 1 frame after landing on vertical plat
So, 77a636509b fixed the fact that you
only got 1 frame of onground/onroof when standing on a vertical moving
platform, but removing those lines entirely means that it takes 1 frame
before the onground/onroof of 2 actually takes effect. This desyncs my
Nova TAS, so it seems important to fix.
2020-10-06 02:16:49 -04:00
Misa 77a636509b Remove duplicate onroof/onground assignment for vertical moving plats
The onroof/onground attributes are used to determine if the player is
standing on a surface and is eligible to flip. Most notably, it is an
integer and not a boolean, and it starts at 2, giving the player 2
frames to edge-flip, i.e. they can still flip 2 frames after walking off
an edge.

However, these attributes are unnecessarily reassigned in
movingplatformfix() (which is the function that deals exclusively with
vertically-moving platforms; horizontal moving platforms get their own
hormovingplatformfix()). Whoever wrote this misunderstood what
onroof/onground meant; they thought that they were booleans, and so set
them to true, instead of the proper value of 2. This ends up setting
onroof/onground to 1 instead of 2, causing a discrepancy with vertical
moving platforms and the rest of the surfaces in the game.

The bigger mistake here is duplicating code that never needed to be
duplicated. The onroof/onground assignment in gamelogic() works
perfectly fine for vertical moving platforms. Indeed, after testing it
with libTAS, I can confirm that removing the duplicate assignments
restores being able to edge-flip off of moving platforms with 2 frames
of leeway, instead of only 1 frame. It also doesn't change how long it
takes for the onroof/onground to get set when the player is recognized
as standing on a vertically-moving platform, either.

And so, it's better to not duplicate this code, because when you
duplicate it you run the risk of making a mistake, as I just
demonstrated.
2020-09-28 15:46:41 -04:00
Misa 733cad723b Remove unused attributes from entclass
These attributes were `jumping` and `jumpframe`. Removal of unused
attributes makes reading the code easier.
2020-09-28 15:45:53 -04:00
Misa cbceeccf78 Clean up and prevent unnecessary qualifiers to self
By "unnecessary qualifiers to self", I mean something like using the
'game.' qualifier for a variable on the Game class when you're inside a
function on the Game class itself. This patch is to enforce consistency
as most of the code doesn't have these unnecessary qualifiers.

To prevent further unnecessary qualifiers to self, I made it so the
extern in each header file can be omitted by using a define. That way,
if someone writes something referring to 'game.' on a Game function,
there will be a compile error.

However, if you really need to have a reference to the global name, and
you're within the same .cpp file as the implementation of that object,
you can just do the extern at the function-level. A good example of this
is editorinput()/editorrender()/editorlogic() in editor.cpp. In my
opinion, they should probably be split off into their own separate file
because editor.cpp is getting way too big, but this will do for now.
2020-09-28 01:34:40 -04:00
Misa 571ad1f7d8 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-27 19:08:37 -04:00
Misa c8f000af02 Don't use bounds check for result of checktrigger(), it's a gamestate
checktrigger() returns a gamestate number, not the index of an entity.

Whoops.
2020-09-25 18:00:18 -04:00
Misa a38faad156 Add bounds checks to indexing of global "temporary" variable `k`
For some reason, the variable `k` is on entityclass and gets mutated in
createentity() and createblock(). Then updateentities() uses it without
checking if it's valid, because either `k` or the size of `entities`
could have changed in the meantime. To fix any potential undefined
behavior, these bounds checks should be added.
2020-09-25 17:16:15 -04:00
Misa c325085ddb Fix up trinket and coin ID bounds checks
Trinket IDs weren't being bounds-checked upon creation, and coin IDs
weren't being bounds-checked upon pickup. But now they both are.
2020-09-25 13:51:47 -04:00
Misa 76d6a3536b 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-25 13:51:47 -04:00
Misa f02dcbfdad Don't manually write out INBOUNDS_ARR() checks
When this is done, there is potential for a mistake to occur when
writing out the bounds check, which is eliminated when using the macro
instead. Luckily, this doesn't seem to have happened, but what's even
worse is I hardcoded 400 instead of using SDL_arraysize(ed.level), so if
the size of ed.level the bounds checks would all be wrong, which
wouldn't be good. But that's fixed now, too.
2020-09-25 13:51:47 -04:00
Misa 7b20d90446 Don't manually write out INBOUNDS_VEC() checks
This is because if they are manually written out, they are more likely
to contain mistakes.

In fact, after further review, there are several functions with
incorrect manually-written bounds checks:
 * entityclass::entitycollide()
 * entityclass::removeentity()
 * entityclass::removeblock()
 * entityclass::copylinecross()
 * entityclass::revertlinecross()

All of those functions forgot to do 'greater than or equal to' instead
of 'greater than' when comparing against the size of the vector. So they
were erroneous. But they are now fixed.
2020-09-25 13:51:47 -04:00
Misa b34be3f1ac 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-25 13:51:47 -04:00
Misa 7ed495c373 Rename INBOUNDS() macro to INBOUNDS_VEC()
Since there's an INBOUNDS_ARR() macro, it's much better to specify the
macro for the vector is a macro for the vector, to avoid confusion.

All usages of this macro have been renamed accordingly.
2020-09-25 13:51:47 -04:00
Misa fae14f4e98 De-duplicate stuck prevention for the player/SCM
Stuck prevention (pushing the player/supercrewmate out if they are
inside a wall) has been factored out into its own function, so it's no
longer copy-pasted but slightly tweaked just for the supercrewmate.
2020-09-25 13:37:38 -04:00
Misa 5e29d676e9 Use case-switch for entityclass::collisioncheck()
This makes each case in the function less verbose.
2020-09-25 13:37:38 -04:00
Misa b5806c8bb0 De-duplicate entity collision checks for player/SCM
The entity collision check routine has been factored out into its own
function, so it no longer needs to be copy-pasted for the supercrewmate.
2020-09-25 13:37:38 -04:00
Misa ceaee392e5 De-duplicate vertical moving platform fix for player/SCM
Instead of having two separate functions to move entities along vertical
moving platforms, one for the player and one for the supercrewmate, they
have been consolidated into one function.
2020-09-25 13:37:38 -04:00
Misa 1c5b72410a De-duplicate spike hitbox checks for player/SCM
The spike hitbox check is now one function for both the player and the
supercrewmate, instead of being two separate functions.
2020-09-25 13:37:38 -04:00
Misa 1d40bbdbc7 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-09 22:05:43 -04:00
Misa 662a658cf6 Optimize entity collision checking to O(n)
I noticed that if I have a large amount of entities in the room, the
game starts to freeze and one frame would take a very long time. I
identified an obvious cause of this, which is that the entity collision
checking in entityclass::entitycollisioncheck() is O(n²), n being the
number of entities in the room.

But it doesn't need to be O(n²). The only entities you need to check
against all other entities are the player and the supercrewmate. You
don't need to "test entity to entity" if 99% of the pairs of entities
you're checking don't involve the player or supercrewmate.

How do we make it O(n)? Well, just hoist the rule 0 and type 14 checks
out of the inner for-loop. That way, the inner for-loop won't be
unconditionally ran, meaning that in most cases it will always be O(n).
However, if you start having large amounts of duplicate player or
supercrewmate entities (I don't know why you would), it would start
approaching O(n²), but I feel like that's fair in that case. But most of
the time, it will be O(n).

So that's how collision checking is now O(n) instead.
2020-09-07 23:03:34 -04:00
Misa 610f4e7782 Move crewmate drawframe fix to entity creation instead of loadlevel
This makes it so the fix for crewmates' drawframes being wrong for
1-frame is fixed for all crewmates regardless of when they get created.
Sure, crewmates created in mapclass::loadlevel() have their drawframes
fixed there, but for crewmates that get created from scripting (such as
Violet when gotorooming to the Ship teleporter room after Space Station
1), this fix doesn't apply to them. But now it does, and Violet will no
longer be facing the wrong way for 1 frame when teleporting to the Ship
teleporter room in the Space Station 1 Level Complete cutscene.
2020-09-07 20:46:01 -04:00
Misa 9de3aba94b Fix sprite of player if they are stuck in a wall
If the player is stuck in a wall (which shouldn't happen in the first
place), their sprite would always default to being flipped, even if they
were unflipped.

Being stuck in a wall is characterized by having both positive onfloor
and onground.
2020-09-06 00:13:16 -04:00
Misa 77b47a3c7e Fix compatibility with levels that rely on gamestate-based script boxes
Some levels rely specifically on the fact that certain script boxes are
loaded using gamestates, instead of directly loading the script and
bypassing the gamestate system. Then weird things could happen. This
restores compatibility with those levels.

mapclass::twoframedelayfix() doesn't need to be updated because the
point of that function is to bypass the gamestate system entirely
anyways.
2020-08-17 15:14:22 -04:00
Misa f07a8d2143 Move tele/activity/trophytext conds to collision detection
This moves the teleporter, activity prompt, and trophy text "don't draw"
conditionals to the part where the game checks collision with them,
instead of whenever the game draws them.

This makes it so that the game smoothly does the fade-in/fade-out
animation instead of suddenly stopping rendering them whenever their
"don't draw" conditions apply. Now, the "Press ENTER to activate
terminal" prompt will no longer suddenly disappear whenever you activate
one, and the "- Press ENTER to Teleport -" prompt will smoothly fade
back in after teleporting, instead of suddenly popping in on screen.
2020-08-03 00:12:15 -04:00
tzann b4bac64361 Also change oldyp so there's no deltaframe issues 2020-07-29 10:04:27 -04:00
tzann e48fef4fe5 Fix the 'Game Complete' trophy stand so it no longer hovers above the ground. Also update comments in Otherlevel.cpp to reflect what entities are actually being created. 2020-07-29 10:04:27 -04:00
Misa b5ff65c84e Remove unnecessary includes from header files
Including a header file inside another header file means a bunch of
files are going to be unnecessarily recompiled whenever that inner
header file is changed. So I minimized the amount of header files
included in a header file, and only included the ones that were
necessary (system includes don't count, I'm only talking about includes
from within this project). Then the includes are only in the .cpp files
themselves.

This also minimizes problems such as a NO_CUSTOM_LEVELS build failing
because some file depended on an include that got included in editor.h,
which is another benefit of removing unnecessary includes from header
files.
2020-07-19 21:37:40 -04:00
Misa df96b2a594 Fix softlock using tele while in its hitbox post-rescue/intermission
There were many different ways I could've fixed it, but one thing that
stood out to me was the fact that touching the teleporter wasn't
guaranteed to set its onentity to 0, even though it should be. So now,
every time Viridian touches the teleporter, the teleporter's onentity
will be set to 0, and thus there's no chance of the teleporter
interrupting its own teleport animation and softlocking the game.

We should still do what I suggested in #391, namely setting
game.hascontrol to true if the game is in gamestate 0 and script.running
is false, and also always allowing Esc/Enter to be pressed regardless of
game.hascontrol. But this softlock is fixed now.

Fixes #391.
2020-07-18 17:41:21 -04:00
Misa 2506127a17 Directly execute scripts if script boxes have a non-empty script field
Instead of using gamestates, just directly use the 'script' attribute of
a script box if it is non-empty.

This is accomplished by having to return the index of the block that the
player collides with, so callers can inspect the 'script' attribute of
the block themselves, and do their logic accordingly.
2020-07-15 11:24:25 -04:00
Misa 8c42f82317 Set script attribute of custom level script boxes
To avoid going through gamestates, we'll need to carry the name of the
script on the script box itself. And to do that, we'll need to set the
'script' attribute of script boxes when translating edentities into real
entities in custom levels.
2020-07-15 11:24:25 -04:00
Misa 7703b2c1c2 Ensure that all member attributes are initialized
I ran the game through cppcheck and it spat out a bunch of member
attributes that weren't being initialized. So I initialized them.

In the previous version of this commit, I added constructors to
GraphicsResources, otherlevelclass, labclass, warpclass, and finalclass,
but flibit says this changes the code flow enough that it's risky to
merge before 2.4, so I got rid of those constructors, too.
2020-07-08 19:14:21 -04:00
Misa 0664eac7fc Turn obj.collect and obj.customcollect into plain arrays
Since they're always fixed-size, there's no need for them to be vectors.

Also added an INBOUNDS_ARR() macro to do bounds checks with plain
arrays.
2020-07-06 11:19:24 -04:00
Misa 62203efb2c Turn obj.flags into an array instead of a vector
Since it's always fixed-size, there's no reason for it to be a vector.
2020-07-06 11:19:24 -04:00
Misa 1258eb7bf4 Turn crew rescued/mood vectors into arrays
Since they're always fixed-size, they don't need to be dynamically-sized
vectors.

entityclass::customcrewmoods is now a proper bool instead of an int now,
and I replaced the hardcoded constant 6 with a static const int Game
attribute to make it easier to change.
2020-07-06 11:19:24 -04:00