Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compass: add total number of pickups and killables #387

Merged
merged 25 commits into from Feb 6, 2022

Conversation

walkawayy
Copy link
Collaborator

@walkawayy walkawayy commented Feb 2, 2022

Resolves #362.

Prepared to get slammed, but as far as I know we have to manually define lists for objects that are killable and pickupable. Not sure if I missed any.

For killables, I had to remove some enemies like the Mummy from Qualopec who can't be killed. But Atlantis has an issue where Lara can kill more than there are killables, but idk what I missed.

For pickups, the different Scions behave differently. I excluded O_SCION_ITEM3 because that is the one you have to shoot.

@walkawayy walkawayy added the Feature New functionality label Feb 2, 2022
@walkawayy walkawayy added this to the 2.6 milestone Feb 2, 2022
Copy link
Collaborator

@rr- rr- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to introduce a bit more intricate solution around O_PODS, which are mutant eggs. If we have initialized items already at this point, we can check if item->data is set to a non-zero value (see InitialisePod). I think O_ABORTION won't be accounted for either and needs to be handled under similar rules as O_BIG_POD.

src/game/level.c Outdated Show resolved Hide resolved
src/game/level.c Outdated Show resolved Hide resolved
@rr-
Copy link
Collaborator

rr- commented Feb 2, 2022

@Richard-L would you prefer this to be an opt-in feature or is it OK to include it as-is?

@Richard-L
Copy link
Collaborator

@rr- I would say do it whichever way is easier to maintain and less workload for you.

The only reason I could think of to make it optional was to maintain a "console perfect" list of settings, where the game's look and behaviour is really perfectly identical to the PS1 version. A blurry line of course.

@rr-
Copy link
Collaborator

rr- commented Feb 2, 2022

I would say it's OK to make it non-configurable.
However, we should add a changelog entry.

@rr-
Copy link
Collaborator

rr- commented Feb 2, 2022

Before we go through with it, I'd like for some professional Tomb Raider player to confirm the expected numbers of reachable items and enemies in every level. Maybe Stella's walkthrough could help.

@walkawayy
Copy link
Collaborator Author

walkawayy commented Feb 2, 2022

Ya I agree. speedrun.com doesn't list kills or pickups.

Thanks to Rapora9 from Discord for compiling this list from this 100% playthrough. I can try and cross check it with another playthrough. Especially since Atlantis can't be seen.

Level:   Kills, Pickups

Caves:      14, 7
Vilcabamba: 29, 13
Valley:     13, 16
Qualopec:   8,  8
Folly:      23, 19
Colosseum:  26, 14
Midas:      43, 23
Cistern:    34, 28
Tihocan:    17, 26
City:       14, 24
Obelisk:    16, 38
Sanctuary:  15, 29
Natla:      3,  30
Atlantis:   (not visible on the recording)
Pyramid:    6,  31

Also I think a few levels have a few unreachable medipacks but that seems more like a level design issue than something we should hardcode?

@walkawayy
Copy link
Collaborator Author

walkawayy commented Feb 2, 2022

Here's a list from speedrunner Footi:

Level Kills Pickups Secrets
Caves 14 7 3
Vilcabamba 29 13 3
Lost Valley 13 16 5
Qualopec 8 8 3
Folly 23 19 4
Colosseum 26 14 3
Midas 43 23 (22) 3
Cistern 34 28 3
Tihocan 17 26 2
City 14 24 3
Obelisk 16 38 3
Scion 15 29 1
Mines 3 30 3
Atlantis 32 50 3
Great Pyramid 6 31 3

Midas has an unreachable medipack.

@rr-
Copy link
Collaborator

rr- commented Feb 2, 2022

It's OK to count that medipack as pickupable IMO.

@Richard-L
Copy link
Collaborator

I agree. We were just talking about it on Discord. I've also asked Footi for a list for the UB levels.

@walkawayy
Copy link
Collaborator Author

walkawayy commented Feb 2, 2022

I agree. We were just talking about it on Discord. I've also asked Footi for a list for the UB levels.

Cool thanks. That number is probably messed up due to #250.

Levels with unobtainable stats:

Tomb Raider 1:

Level Kills Pickups
Palace Midas 23 (+1 unreachable medis)
Atlantis 53 (+2 unreachable medis, +1 unreachable uzi ammo)

Unfinished Business:

Level Kills Pickups
Return to Egypt 42 (+1 unreachable enemy)
Temple of the Cat 64 (+1 unreachable magnum ammo)
Atlanetean Stronghold 32 (+1 unreachable Centaur)

@Richard-L
Copy link
Collaborator

Footi thankfully asked Anopob, the only guy who has done a UB max% run, for this numbers. Without guarantee:

yeah i have, for glitchless (same as in my run):
Return to Egypt: 41 Kills, 53 Pickups
Temple of the Cat: 44 Kills, 63 Pickups
Atlantean Stronghold: 31 Kills, 63 Pickups
The Hive: 41 Kills, 60 Pickups

src/game/level.c Outdated Show resolved Hide resolved
@rr- rr- marked this pull request as draft February 3, 2022 16:22
@walkawayy
Copy link
Collaborator Author

walkawayy commented Feb 3, 2022

We have to introduce a bit more intricate solution around O_PODS, which are mutant eggs. If we have initialized items already at this point, we can check if item->data is set to a non-zero value (see InitialisePod). I think O_ABORTION won't be accounted for either and needs to be handled under similar rules as O_BIG_POD.

So I did some digging on the pods stuff. The reason my code does not currently calculate the number of mutants correctly is because I do not include O_PODS and O_BIG_POD as killables (for good reason). When the level loads (where my code is counting the enemies), the game doesn't load any of the mutants spawned from pods. These enemies are spawned when a pod trigger is stepped on and the pod explodes. I can't just count the pods though because the Atlantis level has the abortion pod that will never spawn an enemy because it has no trigger. Unfinished Business has similar eggs that have no trigger.

The solution might require some floor checking to find all pod triggers while making sure not to count multiple triggers for the same pod. The last case would be the broken UB Hive eggs that have triggers and explode, but they spawn an abortion mutant that isn't in the level files. But that seems like a level design issue like unreachable medipacks.

@rr-
Copy link
Collaborator

rr- commented Feb 3, 2022

The solution might require some floor checking to find all pod triggers while making sure not to count multiple triggers for the same pod. The last case would be the broken UB Hive eggs that have triggers and explode, but they spawn an abortion mutant that isn't in the level files. But that seems like a level design issue like unreachable medipacks.

I'm not sure about UB but yes, that's what I meant in my previous post about the Atlanteans.

We have to introduce a bit more intricate solution around O_PODS, which are mutant eggs. If we have initialized items already at this point, we can check if item->data is set to a non-zero value (see InitialisePod). I think O_ABORTION won't be accounted for either and needs to be handled under similar rules as O_BIG_POD.

Counting the triggers sounds nasty, but it's what we do for calculating the secret count.

Since these functions are getting quite complex, could we move them to a function such as GetSecretCount()?

Offtop: To make the endgame level stats work, we'll probably have to store the results in START_INFO and persist it to the savegame file. I wouldn't like to have to load every level and count everything again before displaying the dialog, it will take forever.

@walkawayy
Copy link
Collaborator Author

Ya. Also not sure what to do about pickups right now. Enemies like Pierre and the mercenaries spawn items on death. Adding to the total mid level would be easy but idk if that's nice. For example, when you kill Pierre in Tihocan, the stats screen could go from 23 of 23 pickups, to 23 of 26.

Pierre's code as reference:


    if (item->hit_points <= 0) {
        if (item->current_anim_state != PIERRE_DEATH) {
            item->current_anim_state = PIERRE_DEATH;
            item->anim_number =
                g_Objects[O_PIERRE].anim_index + PIERRE_DIE_ANIM;
            item->frame_number = g_Anims[item->anim_number].frame_base;
            SpawnItem(item, O_MAGNUM_ITEM);
            SpawnItem(item, O_SCION_ITEM2);
            SpawnItem(item, O_KEY_ITEM1);
        }

@rr-
Copy link
Collaborator

rr- commented Feb 3, 2022

Same for the Skate kid and the Baldie. I think it's OK to hardcode the amount of additional items with a proper comment.

IIRC Baldie drops a Shotgun. I don't remember what happens if Lara already has it in her inventory – whether he drops Shotgun, Shotgun Ammo, or nothing (the worst case).

@walkawayy
Copy link
Collaborator Author

walkawayy commented Feb 3, 2022

Same for the Skate kid and the Baldie. I think it's OK to hardcode the amount of additional items with a proper comment.

IIRC Baldie drops a Shotgun. I don't remember what happens if Lara already has it in her inventory – whether he drops Shotgun, Shotgun Ammo, or nothing (the worst case).

Ya all the mercenaries do. Looks like skate kid dropped the Uzis even though I already had them and it didn't seem to give me any ammo. I checked a PS1 video and it looks like that behavior matches.

I know custom games aren't the main focus, but I also didn't want to hard code extra items per level. And you can't just check if Pierre is in the level or something, since he runs away depending on the one shot trigger or not. Maybe I'll do floor parsing for this too and if those enemies are present and can die based on the trigger, add their items to the total.

@Richard-L
Copy link
Collaborator

Richard-L commented Feb 3, 2022

@walkawayy since Natla's Mines strips you of all weapons, how did you have the UZIs already, via cheat? I don't recall there being a secret where you get them. I think you only get them from skateboard kid. There is however the boulder secret after the lava pit, where you get the shotgun.

You can see here that Footi's killing baldie with the shotgun he has from that secret. And when he goes to pick it up (pulls a switch first), it shows as the shotgun item instead of ammo, interestingly. https://youtu.be/D3qgf84dLDg?t=8353

I know that if you pick up the shotgun in Lost Valley, then the secret with it in Folly will show shotgun ammo. If you haven't picked up the Lost Valley shotgun, then it will be as a weapon instead. So perhaps do weapons only update into ammo if there's a level swap in between? Since Natla's Mines has two shotguns (depending on whether you kill baldie or not), it would seem to indicate so.

src/game/level.c Outdated Show resolved Hide resolved
@walkawayy
Copy link
Collaborator Author

Ok I think this is ready for review, but by new module do you mean moving this to a new .c file with those other functions @rr- ?

@rr-
Copy link
Collaborator

rr- commented Feb 5, 2022

Yes. And reduce the amount of global state. Perhaps introduce a new module-level struct that you pass around between the functions.

src/game/level.c Outdated Show resolved Hide resolved
src/game/level.c Outdated Show resolved Hide resolved
src/game/stats.c Outdated

static void Stats_CheckTriggers(FLOOR_INFO **floor_array);

static void Stats_CheckTriggers(FLOOR_INFO **floor_array)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good but I don't quite don't understand why do we need to create a copy of the floor data for the stats calc?

Copy link
Collaborator Author

@walkawayy walkawayy Feb 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the way doors work. There was a bug where the trigger check was missing triggers in a rare instance. See the screenshot below for a trigger that is part of the bigger room but is behind a closed door. When the doors are initialized, closed doors set the floor index to 0, so this trigger was never getting checked.

image

src/game/stats.c Outdated Show resolved Hide resolved
src/game/stats.h Show resolved Hide resolved
src/game/stats.h Outdated Show resolved Hide resolved
src/game/stats.c Outdated Show resolved Hide resolved
@walkawayy
Copy link
Collaborator Author

I'm checking on the floor_data pointer thing.

The only two issues remaining I think are:

  1. GetSecretCount still exists and used in gameflow.c for g_GameFlow.levels[g_CurrentLevel].secrets = GetSecretCount();. I tried to add secret counting into my Stats_CheckTriggers function. But I don't understand why Great Pyramid secret count fix, so my calculation seems to be missing one secret in that level.

  2. I can't figure out why the order is flipped for the compass and the end of level stats screen. Not a big deal but I'd rather it just be consistent.

image

image

@rr-
Copy link
Collaborator

rr- commented Feb 5, 2022

Let's make the order consistent with the end of the level stats, but in a separate ticket, not this one.

The Great Pyramid secret fix works by changing tiles that trigger a secret. It makes sure that secret triggers placed in different rooms trigger unique secrets. So room 555 and room 666 shouldn't both trigger secret 2. In OG level data, they do. After the fix, room 555 triggers secret 2 and room 666 secret 3. Thus, level stats need to be recalculated after applying this fix.

I'll need to think what to do with the extra allocation, as I don't like the way it's coupled with the stats now. Maybe tomorrow.

@rr- rr- marked this pull request as ready for review February 5, 2022 19:08
@walkawayy
Copy link
Collaborator Author

Found the Atlantean Stronghold culprit. This centaur is triggered in this unreachable room probably for scare tactics.

image

@walkawayy
Copy link
Collaborator Author

Ok I made a new section near the bottom of the README. I was going to add an item to the Q&A, but tables can't be indented in Markdown. So, I put the table and section right under the Q&A.

@rr-
Copy link
Collaborator

rr- commented Feb 6, 2022

Thanks. Can we list all the numbers so that if we make a regression we have a single source of truth?
+ I still would like to de-nest those trigger checks by extracting them to a function.

@rr- rr- merged commit ccdea4f into LostArtefacts:master Feb 6, 2022
@rr-
Copy link
Collaborator

rr- commented Feb 6, 2022

Thank you.

@walkawayy walkawayy deleted the more-stats branch March 16, 2022 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: more robust level stats
3 participants