Add support demo levels (try 2) #1378

carlo-bramini · 2024-05-26T12:04:14Z

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

Description

This patch adds support for playing the demo levels of City of Vilcabamba.
It supersedes the patch at #722 and additional description can be found there and at #667.
It has been splitted in two commits:

the first commit improves the code for handling the assignment of level_type and it has been applied also for SEQ type. It does not change the behaviour but it allows to remove much similar copy-paste code, which is useful for implementing the second commit.
the second commit adds support for the demo level. I played it and I have not found particular problems in the game.

Richard-L · 2024-05-26T23:19:50Z

Been hoping for this for a long time 🙏

Since there are several demos, I wonder which (or all?) are supported?

Many thanks for doing this.

rr- · 2024-05-27T07:00:19Z

Hey @carlo-bramini, I appreciate your efforts in refreshing the initial pull request. The proposed refactoring looks great, however, there are a few code style deviations we'd need to clear before giving it a go. I understand you've had challenges with the build process, so I don't want to burden you with these. Would you be okay with us making some stylistic adjustments directly in the PR, or, if it turns out we lack the access to push to your fork, creating a sibling PR with the changes?

@Richard-L @lahm86 @walkawayy to make this feature less esoteric, I think we could create a checkbox in the installer to download also the demo files from our server to make the demos accessible to a wider audience. But if there is more than a single demo floating around, this becomes problematic – kind of like with what version of UB to bundle. What are your thoughts on this subject?

Richard-L · 2024-05-27T08:14:23Z

@rr- thanks for bringing this up. My hope is still that immediate and no-purchase no-nothing accessibility to playing the game (and that isn't confusing UB), will help attract a few more players' interest.

I'm in Paris atm without my main PC, but of the top of my head, for PC, there are these demos:

Vilcabamba Part 1
Vilcabamba Part 2
"Terracide" bundle, where a TR demo or Caves and the full Vilcabamba were included (this is the one I'd probably offer)

lahm86 · 2024-05-27T11:43:16Z

I think if it's a single download then adding it to the installer is a good call. If it becomes more complicated with various alternative downloads, then I'd be inclined to keep the support just for one type.

Would a "launch demo" button also be needed in the config tool, similar to "launch UB", or would that be going too far?

And - this is just another thought - if we were to provide the demo files, could we not just edit them to move the palette into the expected position, therefore reducing the amount of code changes needed in the first place? I appreciate that would affect the playability of independent demo downloads, but thought it would be worth mentioning. It would also mean we could open the files in trview and suchlike.

rr- · 2024-05-27T12:14:15Z

Good call on editing the files especially since there is finite amount of them IMO.
Adding the button to launch the demo might be too much, same with creating a desktop shortcut.

carlo-bramini · 2024-05-27T14:24:20Z

Hey @carlo-bramini, I appreciate your efforts in refreshing the initial pull request. The proposed refactoring looks great, however, there are a few code style deviations we'd need to clear before giving it a go. I understand you've had challenges with the build process, so I don't want to burden you with these. Would you be okay with us making some stylistic adjustments directly in the PR, or, if it turns out we lack the access to push to your fork, creating a sibling PR with the changes?

Ok, no problem!
You can just let me know which code style deviations need to be adjusted and I will squash them into the proper commit.

rr- · 2024-05-27T15:05:04Z

src/game/gameflow.c

@@ -51,6 +58,60 @@ static bool GameFlow_LoadLevelSequence(
 static bool GameFlow_LoadScriptLevels(struct json_object_s *obj);
 static bool GameFlow_LoadFromFileImpl(const char *file_name);

+static const STRING_TO_ENUM_TYPE map_GAMEFLOW_LEVEL_TYPE[] = {


This should be named m_GameflowLevelTypeEnumMap or alike.

rr- · 2024-05-27T15:05:14Z

src/game/gameflow.c

+    { NULL, -1 },
+};
+
+static const STRING_TO_ENUM_TYPE map_GAMEFLOW_SEQ_TYPE[] = {


This should be named m_GameflowSeqTypeEnumMap or alike.

rr- · 2024-05-27T15:06:04Z

src/game/gameflow.c

+};
+
+static int32_t GameFlow_StringToEnumType(
+    const char *str, const STRING_TO_ENUM_TYPE *map)


We can map the pointers as not changing by using const char *const str (I know we're inconsistent about const in our codebase, but it's a noble thing to try and adhere to it.)

rr- · 2024-05-27T15:07:21Z

src/game/gameflow.c

@@ -219,6 +280,10 @@ static bool GameFlow_LoadLevelSequence(
    GAMEFLOW_SEQUENCE *seq = g_GameFlow.levels[level_num].sequence;
    int32_t i = 0;
    while (jseq_elem) {
+        const char *tmp_s;
+        GAMEFLOW_DISPLAY_PICTURE_DATA *data;


Let's keep this particular variable inside the case expression. The compiler will error out, but we can suppress the error by adding curly braces around the case like so:

case foo: { int32_t local_var = 0; break; }

rr- · 2024-05-27T15:11:57Z

src/game/level.c

 static void Level_CompleteSetup(int32_t level_num);

-static bool Level_LoadFromFile(const char *filename, int32_t level_num)
+static bool Level_LoadFromFile(
+    const char *filename, int32_t level_num, bool is_demo)


If there is any way whatsoever to tell the file structure from the file contents rather than external gameflow metadata, I'd appreciate removing this parameter altogether.
If not, I recommend turning it into an enum LEVEL_FORMAT. In the future, it would be beneficial to support Saturn files, possibly PS1 files (I'm unsure about these platforms' data layouts though). But even beyond that, having this enum will help to distinguish between .PHD and .TR2 when we fuse relevant parts of the TR1X and TR2X codebase.

rr- · 2024-05-27T15:12:42Z

src/game/level.c

-        Level_LoadFromFile(g_GameFlow.levels[level_num].level_file, level_num);
+    bool is_demo =
+        (g_GameFlow.levels[level_num].level_type == GFL_TITLE_DEMO_PC)
+        | (g_GameFlow.levels[level_num].level_type == GFL_LEVEL_DEMO_PC);


Rather than a binary op we can use ||. Also, we can mark the variables as const.

rr- · 2024-05-27T15:15:08Z

src/global/types.h

@@ -958,6 +958,8 @@ typedef enum GAMEFLOW_LEVEL_TYPE {
    GFL_RESTART = 7,
    GFL_SELECT = 8,
    GFL_BONUS = 9,
+    GFL_TITLE_DEMO_PC = 10,


I'm not sure if there are more places that check for GFL_TITLE. In that case rather than sprinkling the codebase with additional || level_type == GFL_TITLE_DEMO_PC across, I'd lean towards resetting the level type in the gameflow loader back to GFL_TITLE for the given level. Same for regular levels.

rr- · 2024-05-27T16:07:54Z

Oops I meant to comment rather than approving.

walkawayy · 2024-05-28T22:32:40Z

I'm not sure if I did something wrong but my game crashes like this:

I put the demo config in my cfg folder, made a shortcut with -gold, and put in the title and level2 .phd's in my data folder. I used the demo level linked in the issue here: https://www.allgamestaff.it/download-tomb-raider/tr-1/

rr- · 2024-05-29T05:37:38Z

The gameflow refers to the demo files with data_demo_pc/level2.phd and that's where the files should be put. Otherwise it'll try to load regular Vilcabamba as a demo level and chaos will ensue.

carlo-bramini · 2024-05-29T07:28:32Z

I'm not sure if I did something wrong but my game crashes like this:

I put the demo config in my cfg folder, made a shortcut with -gold, and put in the title and level2 .phd's in my data folder. I used the demo level linked in the issue here: https://www.allgamestaff.it/download-tomb-raider/tr-1/

For running demo files, you must provide -demo_pc instead of -gold.
At the moment, it is needed to do it, otherwise the engine cannot distinguish between the different file format, if you are using this patch.

walkawayy · 2024-05-29T14:37:54Z

I'm not sure if I did something wrong but my game crashes like this:

I put the demo config in my cfg folder, made a shortcut with -gold, and put in the title and level2 .phd's in my data folder. I used the demo level linked in the issue here: https://www.allgamestaff.it/download-tomb-raider/tr-1/

For running demo files, you must provide -demo_pc instead of -gold. At the moment, it is needed to do it, otherwise the engine cannot distinguish between the different file format, if you are using this patch.

Oops I think was using -demo. I'll try again tonight. IMO we could probably drop the _pc part?

walkawayy

LGTM.

walkawayy · 2024-05-29T21:12:25Z

data/ship/cfg/TR1X_gameflow_demo_pc.json5

+                "key1": "Silver Key",
+                "puzzle1": "Gold Idol"
+            },
+            "demo": true,


We should set "unobtainable_pickups": x, and "unobtainable_kills": x, since TR1X analyzes the whole level for pickups and kills, but the demo ends early.

Here were the final stats I got. So I think "unobtainable_pickups": 5 and "unobtainable_kills": 9. We don't have an unobtainable_secrets gameflow option either since normally that doesn't come up. is this even worth worrying about @rr- ?

Made a quick PR #1380 to support this.

rr- · 2024-05-31T19:49:38Z

Thanks. We'll follow-up with subsequent PRs to try and remove the need for the new gameflow option.

carlo-bramini · 2024-06-02T10:38:30Z

@rr- thanks for bringing this up. My hope is still that immediate and no-purchase no-nothing accessibility to playing the game (and that isn't confusing UB), will help attract a few more players' interest.

I'm in Paris atm without my main PC, but of the top of my head, for PC, there are these demos:
* Vilcabamba Part 1

* Vilcabamba Part 2

* "Terracide" bundle, where a TR demo or Caves and the full Vilcabamba were included (this is the one I'd probably offer)

I just made an interesting discover: I downloaded the Terracide demo bundle which includes also a different TR demo.
Inside, there is:

Caves level
Vilcabamba level
Gym level
Several videos non included in the other demos.

Unlike the two demo levels that you can download separately, these ones have the sections like the full game.
In other words, they don't need to use "level_demo_pc" and "title_demo_pc", you must use "normal" and "title" instead.
This is something unexpected.. this patch is required if you want to run the demos each one including a single level, while the demo into the Terracide bundle does not.
I made another json file with only the gym, the first levels and the title and I played it: no problems.

My question after I did this last experiment: since this last demo package includes also some videos and things may start to be a bit confusing, how about adding something like prefix_path on top of the json file?

This may allow to organize data files in this way:

\
 TR1_PC_RETAIL
   data
   fmv
 TR1_PC_DEMO1
   data
 TR1_PC_DEMO2
   data
 TR1_PC_DEMO3
   data
   fmv
 TR1_SATURN
   data
 TR1_PS1
   data
.....

If this prefix is not written in the json file or it is empty, it will work exactly like now.

carlo-bramini force-pushed the add-support-demo-levels-2 branch 2 times, most recently from 66f7919 to 2a7ecb3 Compare May 26, 2024 12:25

rr- requested review from a team, rr-, eikehein and lahm86 and removed request for a team May 27, 2024 06:51

rr- approved these changes May 27, 2024

View reviewed changes

rr- reviewed May 27, 2024

View reviewed changes

carlo-bramini force-pushed the add-support-demo-levels-2 branch 2 times, most recently from fe22246 to e57caf7 Compare May 28, 2024 13:55

carlo-bramini added 2 commits May 28, 2024 16:05

Improve parsing of SEQ and level type.

bc59dd4

Add support for demo levels.

0f1928a

carlo-bramini force-pushed the add-support-demo-levels-2 branch from e57caf7 to 0f1928a Compare May 28, 2024 14:06

walkawayy linked an issue May 28, 2024 that may be closed by this pull request

Demo level support #667

Closed

walkawayy approved these changes May 29, 2024

View reviewed changes

rr- merged commit 3f4b452 into LostArtefacts:develop May 31, 2024
1 check passed

carlo-bramini deleted the add-support-demo-levels-2 branch June 4, 2024 11:26

rr- added the TR1 label Oct 3, 2024

lahm86 mentioned this pull request Nov 18, 2024

Accents in Spanish words #1919

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support demo levels (try 2) #1378

Add support demo levels (try 2) #1378

carlo-bramini commented May 26, 2024

Richard-L commented May 26, 2024

rr- commented May 27, 2024

Richard-L commented May 27, 2024 •

edited

Loading

lahm86 commented May 27, 2024

rr- commented May 27, 2024

carlo-bramini commented May 27, 2024

rr- May 27, 2024

rr- May 27, 2024

rr- May 27, 2024

rr- May 27, 2024

rr- May 27, 2024

rr- May 27, 2024

rr- May 27, 2024

rr- commented May 27, 2024

walkawayy commented May 28, 2024 •

edited

Loading

rr- commented May 29, 2024

carlo-bramini commented May 29, 2024

walkawayy commented May 29, 2024

walkawayy left a comment

walkawayy May 29, 2024 •

edited

Loading

walkawayy May 31, 2024 •

edited

Loading

walkawayy May 31, 2024

rr- commented May 31, 2024

carlo-bramini commented Jun 2, 2024

Add support demo levels (try 2) #1378

Add support demo levels (try 2) #1378

Conversation

carlo-bramini commented May 26, 2024

Checklist

Description

Richard-L commented May 26, 2024

rr- commented May 27, 2024

Richard-L commented May 27, 2024 • edited Loading

lahm86 commented May 27, 2024

rr- commented May 27, 2024

carlo-bramini commented May 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rr- commented May 27, 2024

walkawayy commented May 28, 2024 • edited Loading

rr- commented May 29, 2024

carlo-bramini commented May 29, 2024

walkawayy commented May 29, 2024

walkawayy left a comment

Choose a reason for hiding this comment

walkawayy May 29, 2024 • edited Loading

Choose a reason for hiding this comment

walkawayy May 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rr- commented May 31, 2024

carlo-bramini commented Jun 2, 2024

Richard-L commented May 27, 2024 •

edited

Loading

walkawayy commented May 28, 2024 •

edited

Loading

walkawayy May 29, 2024 •

edited

Loading

walkawayy May 31, 2024 •

edited

Loading