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

DIRECTOR: Add detection for Barbie and her Magical House #5652

Merged
merged 1 commit into from Feb 11, 2024

Conversation

threefins
Copy link
Contributor

No description provided.

Copy link
Member

@sev- sev- left a comment

Choose a reason for hiding this comment

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

Thank you, you mention that this change "adds support". Does it mean that the title is fully playable and compostable?

@@ -8950,6 +8951,8 @@ static const DirectorGameDescription gameDescriptions[] = {
WINGAME1_l("barbswanlake", "", "SwanLake.exe", "310659620631c126edb6943af1f83e38", 4096651, Common::FR_FRA, 900),
WINDEMO1("barbswanlake", "Demo", "SwanLake.exe", "c612aa43e7ef55aa4fd2e1085fb1ef7a", 2385166, 900),

WINGAME1("barbmagichouse", "", "BARBIE.exe", "11d4eb5c3a76fcb30dadca36ad9d2760", 909919, 400),
Copy link
Member

Choose a reason for hiding this comment

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

We require tail md5s for projector files.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, BARBIE.EXE is a quite common filename. Any chance of using an additional file for detection?

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be above "barbnail" to maintain alphabetical order, and in the Director v4 section of the file instead of v9

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK I think I've addressed all of that. I've also bumped up the game version to 404. Let me know if the formatting of the WINGAME2( macro is ok - a few different approaches in that file.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's another issue... you added the game to the "supported" D4 games but it's not, so it should be among the "unsupported" group. After line 3295, that is.

@@ -74,6 +74,7 @@ static const PlainGameDescriptor directorGames[] = {
{ "barbpauper", "Barbie as the Princess and the Pauper" },
{ "barbrapunzel", "Barbie as Rapunzel: A Creative Adventure" },
{ "barbswanlake", "Barbie of Swan Lake: The Enchanted Forest" },
{ "barbmagichouse", "Barbie and her Magical House"},
Copy link
Member

Choose a reason for hiding this comment

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

Is this aligned vertically with the rest of the lines?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also this should be above "barbnail" to maintain alphabetical order

@threefins threefins marked this pull request as draft February 4, 2024 08:10
@threefins
Copy link
Contributor Author

It's only at the beginning of being playable - sorry. I'll close for now but I am not entirely sure what failed in those builds. Assume it's maybe related to the hash but perhaps it's an intermittent thing?

@threefins threefins closed this Feb 4, 2024
@sev-
Copy link
Member

sev- commented Feb 4, 2024

Please do not close the pull requests like that since it wastes the team's resources.

The build failures have nothing to do with your code changes.

@sev- sev- reopened this Feb 4, 2024
@sev-
Copy link
Member

sev- commented Feb 4, 2024

Since it is not fully playable, please rename the commit to "DIRECTOR: Add detection for Barbie and her Magical House", because it is so far the detection only, but not real support.

@threefins threefins force-pushed the barb_magic_house branch 2 times, most recently from 2f233b5 to 9179f0a Compare February 8, 2024 10:53
@threefins threefins changed the title DIRECTOR: Add support for Barbie and her Magical House DIRECTOR: Add detection for Barbie and her Magical House Feb 8, 2024
@threefins threefins marked this pull request as ready for review February 8, 2024 11:11
@sev-
Copy link
Member

sev- commented Feb 11, 2024

Thank you! Merging. I ignore the added empty line :)

@sev- sev- merged commit 4f82bfa into scummvm:master Feb 11, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants