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

Update coktel_decoder.cpp #1837

Merged
merged 1 commit into from Sep 13, 2019

Conversation

@Dimouse
Copy link
Contributor

commented Sep 9, 2019

fix Woodruff subtitles according to https://bugs.scummvm.org/ticket/10960#ticket

Thank you for contributing to ScummVM. Please read the following carefully before submitting your Pull Request.

Make sure your individual commits follow the guidelines found in the ScummVM Wiki: https://wiki.scummvm.org/index.php?title=Commit_Guidelines. If they're not please edit them before submitting the Pull Request.

Commits and Pull Requests should use the following template:

SUBSYSTEM: Short (50 chars or less) summary of changes

More detailed explanatory text, if necessary.  Wrap it to about 72
characters or so.  In some contexts, the first line is treated as the
subject of an email and the rest of the text as the body.  The blank
line separating the summary from the body is critical (unless you omit
the body entirely); tools like rebase can get confused if you run the
two together.

Write your commit message in the present tense: "Fix bug" and not "Fixed
bug."  This convention matches up with commit messages generated by
commands like git merge and git revert.

Further paragraphs come after blank lines.

- Bullet points are okay, too

- Typically a hyphen or asterisk is used for the bullet, preceded by a
 single space, with blank lines in between, but conventions vary here

- Use a hanging indent
@bluegr

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

Thanks for your work! :)

Has this change been tested with other games that use the Coktel Video Decoder? (e.g. SCI games, other gob games apart from Woodruff)

@Dimouse

This comment has been minimized.

Copy link
Contributor Author

commented Sep 13, 2019

Thanks for your work! :)

Has this change been tested with other games that use the Coktel Video Decoder? (e.g. SCI games, other gob games apart from Woodruff)

Hi! Thank you for the reply:)
We didn't check all the games, but it seems that the only game that uses vmd with external subtitles is Woodruff.
There is the list of games here: https://wiki.multimedia.cx/index.php/VMD#Games_Using_VMD

Betrayal In Antara, The Last Dynasty, Shivers 2: Harvest of Souls and SWAT are not supported.
Gabriel Knight 2: The Beast Within, Lighthouse, Phantasmagoria, RAMA and Shivers doesn't have subtitles. Leisure Suit Larry: Love for Sail! and Torin's Passage store subtitles in msg files, and this is probably the case in other SCI games also.
In the gob engine some games are not supported, some are too old for vmd videos and some have other formats (for example, Urban Runner).

But of course if we need to be save, we can check the game explicitly in the code?

@bluegr

This comment has been minimized.

Copy link
Member

commented Sep 13, 2019

Thanks for the feedback, and the work you did by checking the videos of these games. Thus, this can be merged safely.

Thanks for your work!

@bluegr bluegr merged commit cf17986 into scummvm:master Sep 13, 2019

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.