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

Fix Jade Dragon disappearance in Bartoli's Hideout #2538

Merged
merged 4 commits into from
Feb 21, 2025

Conversation

rr-
Copy link
Collaborator

@rr- rr- commented Feb 20, 2025

Checklist

  • I have read the coding conventions
  • I have added a changelog entry about what my pull request accomplishes, or it is an internal change
  • I have added a readme entry about my new feature or OG bug fix, or it is a different change

Description

Resolves #2536.

The original game has a unique logic for the Jade Dragon, and only the jade variant alone, where the dragon removes itself immediately upon activation. So in terms of game mechanics, when Lara uses the detonator, it triggers the Dragon pickup, and that makes it disappear.

To me, it's a very bizarre approach, so rather than restoring it, I went with making pickups use the IF_INVISIBLE flags whenever their host room gets flipped, and not allowing Lara to see or interact with them if this flag is set. This method feels more conventional to me.

That said, I find it hard to believe that this is the only pickup in both games that is placed in a flippable room, requiring such intricate trigger-based solution from the OG code, and that this scenario is not already handled in some other way. Also, I'm unsure if my usage of the flag is good, and would appreciate @lahm86's opinion on this. We hardly ever seem to use IF_INVISIBLE anywhere beyond initial item setup.

Similar to the newest changes done in regard to the savegame↔specific object code decoupling, I've seen that the room flip logic also references certain objects, and I introduced a new method to OBJECT called handle_flip_func to address this.

Testing

  • A quick confirmation if the push blocks and sliding pillars work as intended with flipped rooms.
  • Possibly a test level that lets you flip rooms that contain pickups on and off would be good to know for sure it's working as intended.
  • Also it would be good to programmatically verify whether there are any more pickup items in either game that are triggered by something.

@rr- rr- added TRX bug A bug with TRX TR2 labels Feb 20, 2025
@rr- rr- self-assigned this Feb 20, 2025
@rr- rr- requested review from a team as code owners February 20, 2025 15:01
@rr- rr- requested review from lahm86, walkawayy and aredfan and removed request for a team February 20, 2025 15:01
@lahm86
Copy link
Collaborator

lahm86 commented Feb 20, 2025

I've checked throughout OG TR1 and 2 and the only other place items are triggered is in Natla's Mines - the pistols, and one of the fuses.

The current implementation doesn't seem to fix the Bartoli's issue though. I tried inverting the condition, and it does fix it, but it then also causes all other pickup items in a flip room to disappear. I think this is needed (same in TR1):

diff --git a/src/tr2/game/objects/general/pickup.c b/src/tr2/game/objects/general/pickup.c
index 3d4720aa3..1e0bdb8ae 100644
--- a/src/tr2/game/objects/general/pickup.c
+++ b/src/tr2/game/objects/general/pickup.c
@@ -231,8 +231,10 @@ static void M_Setup(OBJECT *const obj)
 
 static void M_HandleFlip(ITEM *const item, const ROOM_FLIP_STATUS flip_status)
 {
-    if (flip_status == RFS_UNFLIPPED) {
-        item->flags |= IF_INVISIBLE;
+    if (flip_status == RFS_FLIPPED) {
+        if (Item_IsTriggerActive(item)) {
+            item->flags |= IF_INVISIBLE;
+        }
     } else {
         item->flags &= ~IF_INVISIBLE;
     }

Test level here with a pushblock, regular items and a secret set up like Bartoli's.
WALL.zip

@rr- rr- force-pushed the issue-2536-jade-dragon-disappearance branch 3 times, most recently from e0f3c30 to 36e68c3 Compare February 20, 2025 21:16
@rr-
Copy link
Collaborator Author

rr- commented Feb 20, 2025

@lahm86 @aredfan updated. Thanks a lot to Lahm for helping me create a good solution.

Testing:

  • TR1: the Fuse pickup in Natla's Mines is not visible until Lara activates the conveyor belt
  • TR1: the Pistols pickup in Natla's Mines is not visible until Lara lowers the porta-cabin
  • TR2: the Jade Dragon secret pickup in Bartoli's Hideout is visible until Lara explodes the building using the detonator box

@rr- rr- force-pushed the issue-2536-jade-dragon-disappearance branch from 36e68c3 to 8646346 Compare February 20, 2025 21:19
@@ -25,8 +25,3 @@ int32_t Room_GetCeiling(const SECTOR *sector, int32_t x, int32_t y, int32_t z);

void Room_TestTriggers(const ITEM *item);
void Room_TestSectorTrigger(const ITEM *item, const SECTOR *sector);

// TODO: eliminate
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for tidying this up, I forgot to come back to this 😄

@aredfan
Copy link
Collaborator

aredfan commented Feb 20, 2025

This might be a separate issue because it's also present in the latest dev snapshot - the jade dragon persists with these steps.

  1. /play 3
  2. /tp sec x2
  3. pick up the jade dragon
  4. /tp det
  5. use the all items cheat and activate the detonator
  6. fly to the room where the secret was
  7. /save 1
  8. /load 1

Edit: The fuse and pistols in Natla's Mines LGTM.

rr- added 4 commits February 20, 2025 23:23
Resolves #2536 - fixes Jade Dragon not disappearing after Lara uses the
detonator box in Bartoli's Hideout. Regression from 9ec44fd.
@rr- rr- force-pushed the issue-2536-jade-dragon-disappearance branch from 8646346 to b3fb30d Compare February 20, 2025 22:23
@rr-
Copy link
Collaborator Author

rr- commented Feb 20, 2025

@aredfan thank you :) Fixed, but it'll require retesting all scenarios from scratch. The logic is rather delicate unfortunately.

Copy link
Collaborator

@aredfan aredfan left a comment

Choose a reason for hiding this comment

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

@aredfan thank you :) Fixed, but it'll require retesting all scenarios from scratch. The logic is rather delicate unfortunately.

Anytime 👍

I tested all the scenarios again including Lahm's test level, and your fix checks out in all cases. LGTM.

@aredfan aredfan added the Unreleased Applies to things that aren't in a stable release. label Feb 20, 2025
@rr- rr- merged commit 018c192 into develop Feb 21, 2025
9 checks passed
@rr- rr- deleted the issue-2536-jade-dragon-disappearance branch February 21, 2025 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TR2 TRX bug A bug with TRX Unreleased Applies to things that aren't in a stable release.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

TR2X bug: jade secret persists after detonation in Bartoli's Hideout
3 participants