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

SCUMM: Label v6 subops #4735

Merged
merged 1 commit into from
Feb 26, 2023
Merged

SCUMM: Label v6 subops #4735

merged 1 commit into from
Feb 26, 2023

Conversation

BLooperZ
Copy link
Contributor

This labels the sub opcodes in switch cases of SCUMM v6 (including HE) games

@AndywinXp
Copy link
Contributor

Been meaning to do that for a while now, thanks 🫶

@AndywinXp
Copy link
Contributor

Everything seems fine to me (I still have to test it though).

Just a curiosity: was there a specific reasoning behind e.g. in SCUMM v8 naming op 103 to SO_ANIMATION_DEFAULT instead of SO_ACTOR_ANIMATION_DEFAULT?

@BLooperZ
Copy link
Contributor Author

Thank you!
Yes, I have tried to match the names with the mode(?) of other engine versions as much as I could,
so it'll easier for us to connect them for comparison.

@BLooperZ
Copy link
Contributor Author

BLooperZ commented Feb 23, 2023

I am undecided about the force-pushed fix (renaming SO_SHADOW to SO_SPECIAL_DRAW)
SO_SHADOW is the common name that match the behaviour in all other engine versions
but it appears it might actually was renamed SO_SPECIAL_DRAW since V7 (shadow %1 -> special-draw %1)
According to the SCUMM tutorial https://web.archive.org/web/20170225190343/http://wilmunder.com/Arics_World/Games_files/The SCUMM Manual.pdf

@AndywinXp
Copy link
Contributor

AndywinXp commented Feb 24, 2023

I thought some more time about it; from my point of view it would be prettier (but not mandatory of course!) to keep the original names when applicable, maybe with a comment next to it, e.g. we could use SO_SPECIAL_DRAW in v7 and keep a comment next to it which says SO_SHADOW.

The reasoning behind this is that IMHO it'd be better have code labeled in a way that it's self explanatory.

This applies to v8 in particular which is almost its own thing, with its high number of features which were reworked from previous versions.

Again, this is not mandatory at all, I already like the changes if you do like them too. 😄

@AndywinXp
Copy link
Contributor

Needless to say, excellent work as always! Merging!

@AndywinXp AndywinXp merged commit e14b232 into scummvm:master Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants