KEMBAR78
PINK: Handle animations with non standard start frame by antoniou79 · Pull Request #4942 · scummvm/scummvm · GitHub
Skip to content

PINK: Handle animations with non standard start frame #4942

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 29, 2023
Merged

PINK: Handle animations with non standard start frame #4942

merged 2 commits into from
Apr 29, 2023

Conversation

@antoniou79
Copy link
Contributor

There are a few animations with start frame greater than 0 and also greater than (or equal to) framecount

It seems that for such animations, the decoder is supposed to start at 0 and play framecount number of frames. So for these cases we deduct a _startFrame ammount before sending them to decoder. This addresses crash bug #14438 PINK: Crash in Egypt when using the digger on the characters for Passport to Peril

This was tested for Passport to Peril US English version. I am uncertain about potential side-effects on Hocus Pocus.

@antoniou79
PINK: Handle animations with non standard start frame
70e5b3e 
There are a few animations with start frame >= 0 and also >= framecount

It seems that for such animations, the decoder is supposed to start at 0 and play framecount number of frames.
So for these cases we deduct a _startFrame ammount before sending them to decoder.
This addresses crash bug #14438 PINK: Crash in Egypt when using the digger on the characters for Passport to Peril
@antoniou79 antoniou79 requested review from sev- and voltya April 27, 2023 10:29
@voltya
Copy link
Member

voltya commented Apr 27, 2023

I need to check assembly before approving, so it may take some time.

@voltya
Copy link
Member

voltya commented Apr 27, 2023

I looked into assembly, but unfortunately I haven't reversed part where decoder process sprites.
Your approach seem a little bit overengineered.
Maybe set _startFrame to zero if it exceeds frames count?
In such case we don't need any additional logic to handle such actions.

@antoniou79
Copy link
Contributor Author

antoniou79 commented Apr 27, 2023 •
edited
Loading

You are right. Just setting _startFrame to 0 for these non-standard cases should also do the trick (although I don't understand why these cases exist). I guess I was reluctant to change the _startFrame value for some reason.

I quick-tested with your suggestion and it looks ok so I pushed a commit.
If this gets accepted, feel free to squash the commits before merging.

Edit: I should clarify that I only tested on Pink Panther Passport to Peril. Not Hocus Pocus. However, this particular scene (Egypt, village) from start to finish (ie from when PInk arrives up to when he leaves through the air ladder) seems to cover many cases for animations so I believe it should be fine.

voltya
voltya approved these changes Apr 28, 2023
View reviewed changes
@sev-
Copy link
Member

sev- commented Apr 29, 2023

OK, merging and squashing

@sev- sev- merged commit 1645c4b into scummvm:master Apr 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@voltya voltya voltya approved these changes

@sev- sev- Awaiting requested review from sev-

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@antoniou79 @voltya @sev-